Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The patch contains a non-compiling ObjectReader call and unconditional Jackson overrides that prevent explicit Quarkus configuration from taking effect.
1 open issue
What changed in this PR
This PR consolidates REST-layer JSON handling around Quarkus’s managed ObjectMapper, reducing duplicate mapper instances while retaining Keycloak-specific configuration.
Changes:
- Replaces the Quarkus mapper resolver with an
ObjectMapperCustomizer. - Adds a shared mapper accessor to session factories, with a fallback for non-Quarkus runtimes.
- Updates admin-v2 merge-patch handling and RESTEasy test setup to use the revised mapper APIs.
| File | Description |
|---|---|
services/src/test/java/org/keycloak/services/resteasy/ResteasyKeycloakApplication.java |
Supplies an explicit mapper to the test resolver. |
services/src/main/java/org/keycloak/services/util/ObjectMapperResolver.java |
Extracts reusable configuration and accepts an injected mapper. |
services/src/main/java/org/keycloak/services/DefaultKeycloakSessionFactory.java |
Adds lazy access to a shared fallback mapper. |
rest/admin-v2/services/src/main/java/org/keycloak/rest/admin/api/client/DefaultClientApi.java |
Uses the shared mapper for merge-patch processing. |
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/QuarkusKeycloakSessionFactory.java |
Retrieves the Quarkus-managed mapper. |
quarkus/runtime/src/main/java/org/keycloak/quarkus/runtime/integration/jaxrs/KeycloakObjectMapperCustomizer.java |
Applies Keycloak configuration through a Quarkus customizer. |
Review effort: Balanced
f87d17b to
2a7d0f8
Compare
b9ed14e to
9b76874
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Regression coverage is missing for how the mapper customizations interact with quarkus.jackson.* overrides, including default unknown-field rejection, an explicit override, and continued trailing-token rejection in PATCH requests.
No open issues 🎉
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The shared Quarkus mapper changes serializer selection and customizer precedence across REST and single-file exports, requiring runtime validation of configuration overrides and export/import compatibility unavailable in this source-only review.
No open issues 🎉
Review effort: Balanced
|
Updated to be more aggressive in consolidation. |
also consolidating the creation of other object mappers closes: keycloak#44159 Signed-off-by: Steve Hawkins <shawkins@redhat.com>
that doesn't need to be spelled out Signed-off-by: Steve Hawkins <shawkins@redhat.com>
Inverts our creation of the rest layer objectmapper, so that we are reusing the quarkus one. We could move our customizations to the application.properties.
This reduces our memory footprint a little and let's and users or support issues use quarkus properties to workaround serialization / deserialization issues if needed.
closes: #44159