Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CUSTCOM-13 A REST management DELETE command returns 415 code instead of 404 #4423

Merged
merged 2 commits into from Jan 14, 2020

Conversation

@MattGill98
Copy link
Member

MattGill98 commented Jan 10, 2020

This is a bug fix. Starting a fresh domain in 5.194 and running the following curl command:

curl -X DELETE -v -H "Content-Type: application/x-www-form-urlencoded" -H "X-Requested-By:Payara" http://localhost:4848/management/domain/servers/server/server/application-ref/non-existing-app-ref

will cause a HTTP 415 to return, where it should be a 404. Note that in 5.193 this worked.

Testing

New tests

Second commit. I've made the testing module fairly generic, as I see it as something that could be useful in the future.

Testing Performed

The new test against 5.193 and the SNAPSHOT on micro, managed and remote profiles.

Test suites executed

  • Payara Samples (only single test)
  • Java EE7 Samples (only JAX-RS tests against remote)
  • Java EE8 Samples (only JAX-RS tests against remote)

Notes for Reviewers

Read the commit messages ;)

I've deliberately ignored the previous fix that regressed. I hope this does the same job but with lower technical debt.

@MattGill98 MattGill98 self-assigned this Jan 10, 2020
@MattGill98

This comment has been minimized.

Copy link
Member Author

MattGill98 commented Jan 10, 2020

jenkins test please

@Cousjava

This comment has been minimized.

Copy link
Member

Cousjava commented Jan 10, 2020

Copyright notices need updating.

@MattGill98 MattGill98 force-pushed the MattGill98:CUSTCOM-13 branch from b1dd9e2 to 5629858 Jan 10, 2020
@MattGill98

This comment has been minimized.

Copy link
Member Author

MattGill98 commented Jan 10, 2020

Good spot, I'm in 2019 mode still 😉

@MattGill98

This comment has been minimized.

Copy link
Member Author

MattGill98 commented Jan 13, 2020

jenkins test please

MattGill98 added 2 commits Jan 10, 2020
When sending null or non-map data to an endpoint with the
x-www-form-urlencoded media type, a generic Map object is created
rather than a specific Map implementation. Because of this no
reader would mark the content as readable and a 415 would be
incorrectly returned.
Created a test to make sure that the REST management API continues
to return a 404 for non-existing resources.
@MattGill98 MattGill98 force-pushed the MattGill98:CUSTCOM-13 branch from 5629858 to 0797619 Jan 13, 2020
@dmatej dmatej self-requested a review Jan 13, 2020
@dmatej

This comment has been minimized.

Copy link
Contributor

dmatej commented Jan 13, 2020

Tested also:
mvn clean verify -pl :payara-samples -amd -Ppayara-samples -fae

  • two tests are failing, but we know about them, they will be fixed in another issue.

./tck.sh jaxrs

real    132m3,698s
user    104m31,735s
sys     6m32,722s
Not passed: 0/2803 
@dmatej

This comment has been minimized.

Copy link
Contributor

dmatej commented Jan 13, 2020

Jenkins test please

@dmatej
dmatej approved these changes Jan 13, 2020
@dmatej
dmatej approved these changes Jan 13, 2020
@jbee
jbee approved these changes Jan 14, 2020
@MattGill98 MattGill98 merged commit 1eb4c09 into payara:master Jan 14, 2020
58 checks passed
58 checks passed
Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details
MarkWareham pushed a commit to MarkWareham/Payara that referenced this pull request Jan 20, 2020
CUSTCOM-13 A REST management DELETE command returns 415 code instead of 404
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.