Added default non-endpoint methods by edewit · Pull Request #51327 · keycloak/keycloak

edewit · 2026-07-31T09:46:13Z

fixes: #50221
Signed-off-by: Erik Jan de Wit erikjan.dewit@gmail.com

fixes: keycloak#50221 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

Copilot

Pull request overview

Adds typed convenience methods to Admin API v2 while preserving access to raw JAX-RS responses.

Changes:

Introduces TypedResponse<T> with typed deserialization and resource cleanup.
Adds typed create and upsert convenience methods.
Covers wrapper behavior and endpoint integration.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`rest/admin-v2/api/src/main/java/org/keycloak/admin/api/TypedResponse.java`	Adds the typed response wrapper.
`rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java`	Adds typed client creation.
`rest/admin-v2/api/src/main/java/org/keycloak/admin/api/client/ClientApi.java`	Adds typed client upsert.
`rest/admin-v2/api/src/test/java/org/keycloak/admin/api/TypedResponseTest.java`	Tests wrapper behavior.
`rest/admin-v2/tests/src/test/java/org/keycloak/tests/admin/client/v2/ClientApiV2Test.java`	Tests typed methods end to end.

michalvavrik

I didn't find anything wrong about this PR, except that IMO it is not useful and I would not go there. The linked issue says:

To stick with Resteasy classic compatibility

I don't think we need this compatibility. I am -1 for this PR.

vmuzikar · 2026-08-03T09:13:22Z

@michalvavrik This is mostly for convenience if the API is consumed through the Java Admin Client, i.e. RestEasy proxy. That's the main motivation for this.

vmuzikar

LGTM, thanks @edewit.

@shawkins Do you want to give this a look too?

michalvavrik · 2026-08-03T09:55:55Z

@michalvavrik This is mostly for convenience if the API is consumed through the Java Admin Client, i.e. RestEasy proxy. That's the main motivation for this.

However, it doesn't give you any advantage, you don't get value directly, you still need to get entity and you still get entity of requested type (as Response also returns type var). I think you didn't provide enough arguments. Can you describe what is that advantage specifically?

vmuzikar · 2026-08-03T10:41:37Z

@michalvavrik The advantage is that the response has a typed entity - you don't need to cast the type.

michalvavrik · 2026-08-03T11:19:49Z

@michalvavrik The advantage is that the response has a typed entity - you don't need to cast the type.

casting is not required more than this PR does (see example below)
this PR introduces new methods create that does same thing as createClient which I think doesn't add on clarity, users will need to check actual implementation to understand (PR contains no javadoc either)
if users decide to use these new methods, in fact they have to cast if they need anything protocol-specific which how do we know they don't?

// shared part
ClientsApi clientsApi = getClientsApi(manageClientsAdminClient);

// without this PR
OIDCClientRepresentation oidcClientRepresentation = clientsApi.createClient(baseRep).readEntity(OIDCClientRepresentation.class);

// with this pr
BaseClientRepresentation baseClientRepresentation = clientsApi.create(baseRep).readEntity();

Anyway, I don't mind and I did approve. I just consider all these arguments weak, that is all.

michalvavrik · 2026-08-03T11:39:14Z

Quarkus REST has it better solved, they have org.jboss.resteasy.reactive.RestResponse which would avoid extra methods, but we are using RESTEasy client in client...

vmuzikar

this PR introduces new methods create that does same thing as createClient which I think doesn't add on clarity, users will need to check actual implementation to understand (PR contains no javadoc either)

@michalvavrik Actually, this is a good point I missed in my reviews. The new methods should have the same name, the only difference in the signature should be the return type. @edewit Could you please fix this?

vmuzikar · 2026-08-03T11:42:31Z

if users decide to use these new methods, in fact they have to cast if they need anything protocol-specific which how do we know they don't?

Sure but casting from BaseClientRepresentation is expected and is required also for other methods that use this type directly, and you can't even avoid instanceof check in some cases (which is true with or without this PR). I consider the Typed Response still as more user/API friendly as the consumer doesn't need to completely guess what type is the returned entity - they'll know it's a client.

Quarkus REST has it better solved, they have org.jboss.resteasy.reactive.RestResponse which would avoid extra methods, but we are using RESTEasy client in client...

Right, that's what Steve meant by "Resteasy classic compatibility".

michalvavrik · 2026-08-03T11:46:08Z

..ame name, the only difference in the signature should be the return type. @edewit Could you please fix this?

impossible

Right, that's what Steve meant by "Resteasy classic compatibility".

Feels like I opened too long discussion, but nothing forces us to use this wrapper, we could create similar interface (delegate pattern) as Quarkus REST has. So we would need no new methods.

Anyway. I am OK now. Thanks for the patience.

vmuzikar

impossible

Right, it won't compile because of type ambiguity. :D Sorry, for the noise, never mind.

shawkins · 2026-08-03T12:05:28Z

    })
    Response createClient(@Valid BaseClientRepresentation client);

+    default TypedResponse<BaseClientRepresentation> create(BaseClientRepresentation client) {


Instead of just BaseClientRepresentation, can you update the methods to use something that extends that:

<T extends BaseClientRepresentation> TypedResponse<T> create(T client)

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

shawkins

LGTM, thanks @edewit

* Added default non-endpoint methods fixes: keycloak#50221 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * changed types Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> --------- Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

* Added default non-endpoint methods fixes: keycloak#50221 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> * changed types Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> --------- Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com> Signed-off-by: alehhu <159355663+alehhu@users.noreply.github.com>

Added default non-endpoint methods

50c08cb

fixes: keycloak#50221 Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

edewit requested a review from a team as a code owner July 31, 2026 09:46

Copilot AI review requested due to automatic review settings July 31, 2026 09:46

edewit requested a review from a team as a code owner July 31, 2026 09:46

keycloak-github-bot Bot added the team/cloud-native label Jul 31, 2026

Copilot started reviewing on behalf of edewit July 31, 2026 09:46 View session

Copilot AI reviewed Jul 31, 2026

View reviewed changes

michalvavrik approved these changes Jul 31, 2026

View reviewed changes

vmuzikar approved these changes Aug 3, 2026

View reviewed changes

vmuzikar requested changes Aug 3, 2026

View reviewed changes

vmuzikar previously approved these changes Aug 3, 2026

View reviewed changes

shawkins reviewed Aug 3, 2026

View reviewed changes

changed types

38842d4

Signed-off-by: Erik Jan de Wit <erikjan.dewit@gmail.com>

Copilot AI review requested due to automatic review settings August 3, 2026 17:09

edewit dismissed vmuzikar’s stale review via 38842d4 August 3, 2026 17:09

Copilot started reviewing on behalf of edewit August 3, 2026 17:16 View session

Copilot AI reviewed Aug 3, 2026

View reviewed changes

edewit requested a review from shawkins August 4, 2026 06:36

michalvavrik approved these changes Aug 4, 2026

View reviewed changes

vmuzikar approved these changes Aug 4, 2026

View reviewed changes

shawkins approved these changes Aug 4, 2026

View reviewed changes

shawkins merged commit 3b354c6 into keycloak:main Aug 4, 2026
91 checks passed

edewit deleted the non-default-endpoint branch August 10, 2026 08:34

Uh oh!

Conversation

edewit commented Jul 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

michalvavrik left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vmuzikar commented Aug 3, 2026

Uh oh!

vmuzikar left a comment

Choose a reason for hiding this comment

Uh oh!

michalvavrik commented Aug 3, 2026

Uh oh!

vmuzikar commented Aug 3, 2026

Uh oh!

michalvavrik commented Aug 3, 2026

Uh oh!

michalvavrik commented Aug 3, 2026

Uh oh!

vmuzikar left a comment

Choose a reason for hiding this comment

Uh oh!

vmuzikar commented Aug 3, 2026

Uh oh!

michalvavrik commented Aug 3, 2026

Uh oh!

vmuzikar left a comment

Choose a reason for hiding this comment

Uh oh!

shawkins Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

shawkins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

michalvavrik left a comment •

edited

Loading