Fix JAX-RS generator: missing MediaType import + illegal throws widening#101
Closed
Syvlain wants to merge 1 commit into
Closed
Fix JAX-RS generator: missing MediaType import + illegal throws widening#101Syvlain wants to merge 1 commit into
Syvlain wants to merge 1 commit into
Conversation
Two regressions surfaced on mcp-ospita (single @produces, single mapping per HTTP method, non-wildcard paths — i.e. the SimpleService dispatch path, which the test fixtures never exercise since they only use a wildcard resource). 1. Illegal throws on service(...) in the SimpleService path do<Method>/$call are emitted with `throws Exception`. In PatternService that is fine because the dispatch goes through service$<id>, which wraps the call chain in try/catch(Throwable) -> JaxrsContext.sendError. SimpleService had no such wrapper: service(...) (override of HttpServlet.service, limited to `throws ServletException, IOException`) called do<Method> directly, so the `throws Exception` leaked out as "unreported exception java.lang.Exception". Fix: wrap SimpleService's dispatch in the same try/catch(Throwable) -> JaxrsContext.sendError that PatternService already uses. do<Method>/$call keep `throws Exception` (correct: $call invokes the user resource method directly, which may declare arbitrary checked exceptions), and both paths now route errors identically. 2. Missing MediaType import in generated per-servlet CU MediaTypesBuilder.type(..) returns either MediaType.XXX (well-known type) or MediaTypes.XXX (custom static field). JaxRsServletBuilder.buildProduces inlines that expression into the per-servlet CU. For well-known types the simple name MediaType is referenced without a matching import on minimal resources (richer graphs add it incidentally), so compile fails with "cannot find symbol". Fix: register MediaType on the consumer CU only when the inlined expression actually references MediaType (well-known), not for custom MediaTypes.XXX fields — avoiding a spurious unused import. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Syvlain
force-pushed
the
fix-jaxrs-mediatype-import-missing
branch
from
aec3282 to
bc49406
Compare
|
Owner
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two regressions surfaced on mcp-ospita (single @produces, single mapping per HTTP method — minimal resource graph).
Missing import in generated per-servlet CU MediaTypesBuilder builds FieldAccessExpr like MediaType.APPLICATION_JSON_TYPE with
mt = types.getClass(MediaType.class)against the MediaTypes class CU. JaxRsServletBuilder.buildProduces then re-uses those Expressions in the per-servlet CU (line ~386, the second arg of req.getAccepted(...)) without registering MediaType on that CU, so the generated servlet references the simple name with no matching import. Compile fails with "cannot find symbol variable MediaType". In richer resource graphs the import is added incidentally by other code paths; minimal resources don't hit those. Fix: call types.getClass(MediaType.class) on the consumer CU before constructing the call.Illegal override-widening of throws on service(...) do and
$callare emitted withaddThrownException(Exception.class). service(...) overrides HttpServlet.service which is declaredthrows ServletException, IOException; an override cannot widen the throws set, so the build fails with "unreported exception java.lang.Exception". Fix (minimal): narrow do/$call to throws IOException, ServletException too. Works as long as user resource methods don't declare checked throws beyond those — a more robust fix would wrap user-method calls in $call with try/catch(Exception) -> ServletException, but that's a broader change.