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.

Sign up for GitHub

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

Jump to bottom

Fix use of ServletHandler.MappedServlet in ResourceFileServlet #98

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Fix use of ServletHandler.MappedServlet in ResourceFileServlet #98

merged 1 commit into from
Mar 8, 2024

Conversation

lachlan-roberts
Copy link
Collaborator

MatchedResource is not comparing the internal resource as it doesn't override equals, so this Objects.equals comparison will not work.

@lachlan-roberts
Fix use of ServletHandler.MappedServlet in ResourceFileServlet
4630fcb 
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
@lachlan-roberts lachlan-roberts self-assigned this Mar 8, 2024
@ludoch
Copy link
Collaborator

ludoch commented Mar 8, 2024

Accepting.
We still need to find the way to emit internal errors to the user logs so it would give a better understanding of the root cause for such error.

gregw
gregw approved these changes Mar 8, 2024
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

Probably worthwhile fixing MatchedResource so it does have a good equals as well.

@copybara-service copybara-service bot merged commit 87fcf04 into main Mar 8, 2024
7 checks passed
@jianglai jianglai mentioned this pull request Mar 11, 2024
Merged
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 11, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
aca3e93 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
@lachlan-roberts lachlan-roberts deleted the EE10-ResourceFileServlet branch March 12, 2024 03:27
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 12, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
c078032 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 13, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
e9bac06 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 13, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
5137c7b 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 14, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
db2a5d7 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 14, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
0c9c113 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 15, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
4cd3ede 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
jianglai added a commit to jianglai/nomulus that referenced this pull request Mar 18, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet
fc51348 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
github-merge-queue bot pushed a commit to google/nomulus that referenced this pull request Mar 18, 2024
@jianglai
Upgrade to Jakarta EE 10 Servlet (#2362)
e7bf74d 
Upgrade to using Jakarta EE 10 from Java EE 8 by mostly following the upgrade instructions. Only the servlet package is upgrade. Other Jakarta EE components (like the persistence package that Hibernate depends on) need to be upgraded separately.

TESTED=deployed and successfully communicated with the pubapi endpoint for web WHOIS.

Note that this currently requires packaing the App Engine runtime per instructions here due to GoogleCloudPlatform/appengine-java-standard#98. This PR will only be merged until the fix is deployed to production (https://rapid.corp.google.com/#/release/serverless_runtimes_run_java/java21_20240310_21_0).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@ludoch ludoch Awaiting requested review from ludoch

Assignees

@lachlan-roberts lachlan-roberts

Labels
ready to pull
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lachlan-roberts @ludoch @gregw