Client Resource must return a single item (#39851) #43382

cherifyazid · 2025-10-12T10:38:26Z

We have such a method org.keycloak.admin.client.resource.ClientsResource.findByClientId(String) that should return a single item, but it returns a List of ClientRepresentation.

for more details please check the issue description

Closes #39851

Signed-off-by: cherifyazid [email protected]

shawkins · 2025-10-12T15:09:17Z

integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientsResource.java

    @GET
    @Produces(MediaType.APPLICATION_JSON)
-    List<ClientRepresentation> findByClientId(@QueryParam("clientId") String clientId);
+    ClientRepresentation findByClientId(@QueryParam("clientId") String clientId);


Please see keycloak/keycloak-client#155 (comment) and use the method name findClientByClientId, then deprecate findByClientId.

@shawkins Thanks for the thorough review and helpful suggestions! I’ve addressed the feedback and pushed updates relating to #39851 and #155 :

Renamed the API to findClientByClientId(String) and deprecated findByClientId(String)

Kept the old method as a compat shim delegating to the new one (returns empty/singleton list) ClientTest.findByClientListResult()

Migrated tests to the new API and standardized assertions (assertNotNull, assertNull, etc.);retained one minimal compat test with @SuppressWarnings("deprecation")

This is my first PR to Keycloak—happy to take any suggestions or guidance to align with project requirements.

This is my first PR to Keycloak—happy to take any suggestions or guidance to align with project requirements.

@cherifyazid thank you for this pr. Everything looks good - can you also add a DCO sign off to your commit?

@shawkins Done !

bmscomp · 2025-10-12T18:54:31Z

...rk/examples/tests/src/test/java/org/keycloak/test/examples/RealmSpecificAdminClientTest.java

-
-        ClientRepresentation client = clients.get(0);
+        ClientRepresentation client = realmResource.clients().findByClientId("myclient");
+        assertNotNull(client);


Keep the same style of assertion, use static method of Assertions class instead of static import, just to unify the way asserts are used

Prefer Assertions.assertNotNull(client);

cobotrifork · 2025-12-02T13:02:57Z

I might be wrong here, but just stumbled upon this and saw this kind of snippets in the PR:

ClientRepresentation client = realm.admin().clients().findClientByClientId(attachTo);
if (client != null) {
    throw new IllegalStateException("No client found with client id: " + attachTo);
}

Shouldn't the condition check that client is null instead? i.e. if (client == null)

shawkins · 2025-12-04T14:40:37Z

Shouldn't the condition check that client is null instead? i.e. if (client == null)

Absolutely those two instances need to be corrected.

Also @cherifyazid are you able to update / rebase this pr?

cherifyazid · 2025-12-04T15:15:52Z

@cobotrifork nice catch, correction pushed.
@shawkins

shawkins · 2025-12-05T19:11:44Z

@cherifyazid @cobotrifork took a pass over the changes and made several refinements.

There are now two new default methods on ClientsResource:

findClientByClientId returning an Optional ClientRepresentation
and getByClientId that returns the ClientResource - I changed some obvious locations to using this, but there are probably still more places where this could get rid of some boiler-plate.

To all other reviewers if need this pr can be split apart - just the changes to ClientRepresentation could go in by itself, and the changes to the legacy test classes could be omitted.

mabartos · 2025-12-15T10:18:15Z

Nice work!

Could you please resolve the conflicts?

Closes keycloak#39851 Signed-off-by: CHERIF Yazid <[email protected]> Signed-off-by: Steve Hawkins <[email protected]> # Conflicts: # testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oid4vc/issuance/signing/OID4VCIssuerEndpointTest.java

shawkins · 2025-12-15T15:56:11Z

Could you please resolve the conflicts?

It's resolved, but unfortunately this will keep coming up as too many files are touched. If we don't see a path to get it in soon whole, then it will be best to break it up.

shawkins · 2025-12-16T18:44:29Z

@mposolda @lhanusov @vmuzikar @cherifyazid based upon more feedback from @keycloak/cloud-native I'll stop updating this PR. If core-clients would entertain these some of these changes, we'll assign the PR to someone in that team. If not, then this PR and the keycloak / keycloak-client issues can be closed.

vmuzikar · 2025-12-17T16:08:04Z

Just to add a bit more context to what @shawkins stated.

The current Admin REST API suffers from numerous issues and inconsistencies. Instead of trying to navigate the possible fixes while managing breaking changes, we decided to take a step back and redesign the API as a version 2, currently focused and scoped to Client management. Hence we're not prioritizing enhancements to the current (v1) API as those will come in v2.

cherifyazid · 2025-12-17T20:58:38Z

@shawkins @vmuzikar @mposolda

Thanks for the additional context — I fully understand the challenges behind evolving the API and the rationale for stepping back to redesign it as v2.
I’m available to help or contribute in any way that could be useful as this work progresses.

vmuzikar · 2025-12-18T15:32:35Z

@cherifyazid Thanks for understanding. Can we close this PR then?

shawkins · 2025-12-18T18:34:35Z

@cherifyazid Thanks for understanding. Can we close this PR then?

I'd let @mposolda make that call since he was last to triage the keycloak-client issue. If he or those heavily working on the tests like @lhanusov don't want these changes, then definitely this pr and keycloak and keycloak-client issues can be closed.

cherifyazid · 2025-12-18T18:52:12Z

@shawkins yes you can close the PR @vmuzikar

cherifyazid requested review from a team as code owners October 12, 2025 10:38

keycloak-github-bot bot added team/cloud-native team/core-clients team/core-iam labels Oct 12, 2025

cherifyazid force-pushed the feature/clientResource-return-single-item branch 2 times, most recently from af04a23 to cec612a Compare October 12, 2025 11:12

shawkins reviewed Oct 12, 2025

View reviewed changes

bmscomp reviewed Oct 12, 2025

View reviewed changes

cherifyazid force-pushed the feature/clientResource-return-single-item branch 3 times, most recently from 85f3411 to 086ef7a Compare October 13, 2025 14:55

shawkins requested a review from mposolda October 13, 2025 16:02

shawkins force-pushed the feature/clientResource-return-single-item branch from 086ef7a to fd73a1f Compare October 13, 2025 19:35

cherifyazid force-pushed the feature/clientResource-return-single-item branch from fd73a1f to 56f9526 Compare October 15, 2025 12:41

cherifyazid requested review from a team as code owners October 15, 2025 12:41

keycloak-github-bot bot added team/ui team/sre labels Oct 15, 2025

cherifyazid force-pushed the feature/clientResource-return-single-item branch 2 times, most recently from 40d7aab to 2d16e23 Compare October 15, 2025 12:58

shawkins added this to the 26.5.0 milestone Oct 20, 2025

cherifyazid force-pushed the feature/clientResource-return-single-item branch from 2d16e23 to 76ddcf4 Compare December 4, 2025 15:00

shawkins force-pushed the feature/clientResource-return-single-item branch from 76ddcf4 to 273c7b8 Compare December 5, 2025 19:00

shawkins requested a review from lhanusov December 5, 2025 19:06

shawkins force-pushed the feature/clientResource-return-single-item branch from 273c7b8 to deaa197 Compare December 5, 2025 20:24

shawkins force-pushed the feature/clientResource-return-single-item branch from 4c9f7f7 to 8165a39 Compare December 15, 2025 15:55

Client Resource must return a single item (#39851) #43382

Are you sure you want to change the base?

Client Resource must return a single item (#39851) #43382

Uh oh!

Conversation

cherifyazid commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shawkins Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

cherifyazid Oct 13, 2025

Choose a reason for hiding this comment

Uh oh!

shawkins Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

cherifyazid Oct 15, 2025

Choose a reason for hiding this comment

Uh oh!

bmscomp Oct 12, 2025

Choose a reason for hiding this comment

Uh oh!

cobotrifork commented Dec 2, 2025

Uh oh!

shawkins commented Dec 4, 2025

Uh oh!

cherifyazid commented Dec 4, 2025

Uh oh!

shawkins commented Dec 5, 2025

Uh oh!

mabartos commented Dec 15, 2025

Uh oh!

shawkins commented Dec 15, 2025

Uh oh!

shawkins commented Dec 16, 2025

Uh oh!

vmuzikar commented Dec 17, 2025

Uh oh!

cherifyazid commented Dec 17, 2025

Uh oh!

vmuzikar commented Dec 18, 2025

Uh oh!

shawkins commented Dec 18, 2025

Uh oh!

cherifyazid commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cherifyazid commented Oct 12, 2025 •

edited

Loading

cherifyazid commented Dec 18, 2025 •

edited

Loading