Skip to content

Conversation

piu130
Copy link
Contributor

@piu130 piu130 commented Jul 4, 2025

When FileItem.isInMemory is true, value.getFileItem().getFile() throws a NullPointerException. This MR fixes the problem by using the independent FileItem.getInputStream-Method which works for in memory and not in memory FileItems.

Copy link

quarkus-bot bot commented Jul 4, 2025

Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

  • title should preferably start with an uppercase character (if it makes sense!)
  • title should not start with chore/docs/feat/fix/refactor but be a proper sentence

This message is automatically generated by a bot.

@piu130 piu130 changed the title fix(restasy-reactive): add support for reading in memory file in multipart Add support for reading in memory file to string in multipart Jul 4, 2025
@piu130
Copy link
Contributor Author

piu130 commented Jul 7, 2025

Current workaround is to use ByteArray instead of String, because the isInMemory check is made there.

@geoand
Copy link
Contributor

geoand commented Jul 7, 2025

Thanks for the contribution!

Can you please show an example where this change is needed vs what the current code does? Also, having tests that handle this case is also necessary.

@piu130
Copy link
Contributor Author

piu130 commented Jul 7, 2025

Hi @geoand, thanks for checking. I've created a DTO:

class FileUploadBody{
    @RestForm("file")
    lateinit var file: FileUpload

    @RestForm("annotations")
    @PartType("application/xfdf")
    lateinit var annotations: String
}

and a test:

Given {
    contentType(MediaType.MULTIPART_FORM_DATA)
    multiPart(
        "file",
        File("src/test/resources/files/Test.pdf"),
        CustomMediaType.APPLICATION_PDF,
    )
    multiPart(
        "annotations",
        "bla.xfdf",
        "my-annots",
        "application/xfdf",
    )
} When {
    post("upload")
} Then {
    statusCode(Response.Status.CREATED.statusCode)
}

This code throws a NullPointer at the line I've fixed in this MR.

I don't know what to test. I did not change any logic and the current tests still works. A unit test for this fix does not make sense, since my unit is not interested in isInMemory. Neither would a service test (the code above), since the next commit could change the behavior of isInMemory and the code would be skipped.

Please give me some directions for what to test and where to put these test(s) if necessary.

Also note the comment of FileItem.getFile():

This should only be used when {@code isInMemory} is {@code false}

@geoand
Copy link
Contributor

geoand commented Jul 7, 2025

Please give me some directions for what to test and where to put these test(s) if necessary.

We would need a test(or tests) like the ones we have here

@quarkus-bot quarkus-bot bot added the area/rest label Jul 7, 2025
@piu130
Copy link
Contributor Author

piu130 commented Jul 7, 2025

@geoand Thanks for the link :) I've adjusted a test throwing a NullPointer without the fix.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@geoand geoand added triage/backport triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jul 8, 2025
Copy link

quarkus-bot bot commented Jul 8, 2025

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 578df3b.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/kafka-devservices

io.quarkus.it.kafka.continuoustesting.DevServicesDevModeTest.testDevModeServiceDoesNotRestartContainersOnCodeChange - History

  • 1 expectation failed. Expected status code <200> but was <500>. - java.lang.AssertionError
java.lang.AssertionError: 
1 expectation failed.
Expected status code <200> but was <500>.

	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
	at java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:77)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:500)

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/kafka-devservices

io.quarkus.it.kafka.continuoustesting.DevServicesDevModeTest.testDevModeServiceDoesNotRestartContainersOnCodeChange - History

  • New containers: [Container(command=sh -c 'while [ ! -f /work/run.sh ]; do sleep 0.1; done; sleep 0.1; /work/run.sh', created=1751957580, id=fb003fe53588eb0a484b2c084df09e87870c4897ec87a88d11dd761dd03296de, image=quay.io/ogunalp/kafka-native:latest, imageId=sha256:99fa71388449ac7ff46bda7f6d645fb4435d9a21dce3d4dbcaedfc8a738afd6b, names=[/tender_lichterman], ports=[ContainerPort(ip=0.0.0.0, privatePort=9092, publicPort=32847, type=tcp), ContainerPort(ip=::, privatePort=9092, publicPort=32847, type=tcp)], labels={architecture=x86_64, build-date=2025-03-25T21:45:00Z, com.redhat.component=ubi9-micro-container, com.redhat.license_terms=https://www.redhat.com/en/about/red-hat-end-user-license-agreements\#UBI, description=Very small image which doesn't install the package manager., distribution-scope=public, io.buildah.version=1.39.0-dev, io.k8s.description=Very small image which doesn't install the package manager., io.k8s.display-name=Red Hat Universal Base Image 9 Micro, io.openshift.expos... - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: 
New containers: [Container(command=sh -c 'while [ ! -f /work/run.sh ]; do sleep 0.1; done; sleep 0.1; /work/run.sh', created=1751957580, id=fb003fe53588eb0a484b2c084df09e87870c4897ec87a88d11dd761dd03296de, image=quay.io/ogunalp/kafka-native:latest, imageId=sha256:99fa71388449ac7ff46bda7f6d645fb4435d9a21dce3d4dbcaedfc8a738afd6b, names=[/tender_lichterman], ports=[ContainerPort(ip=0.0.0.0, privatePort=9092, publicPort=32847, type=tcp), ContainerPort(ip=::, privatePort=9092, publicPort=32847, type=tcp)], labels={architecture=x86_64, build-date=2025-03-25T21:45:00Z, com.redhat.component=ubi9-micro-container, com.redhat.license_terms=https://www.redhat.com/en/about/red-hat-end-user-license-agreements#UBI, description=Very small image which doesn't install the package manager., distribution-scope=public, io.buildah.version=1.39.0-dev, io.k8s.description=Very small image which doesn't install the package manager., io.k8s.display-name=Red Hat Universal ...

@geoand geoand merged commit 1a247eb into quarkusio:main Jul 8, 2025
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.25 - main milestone Jul 8, 2025
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jul 8, 2025
@gsmet gsmet modified the milestones: 3.25 - main, 3.24.3 Jul 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants