+
Skip to content

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

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

wants to merge 1 commit into from

Conversation

cherifyazid
Copy link

@cherifyazid cherifyazid commented Oct 12, 2025 •
edited
Loading

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 yadzinov@gmail.com

@cherifyazid cherifyazid requested review from a team as code owners October 12, 2025 10:38
@cherifyazid 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
shawkins reviewed Oct 12, 2025
View reviewed changes
integration/admin-client/src/main/java/org/keycloak/admin/client/resource/ClientsResource.java Outdated
@GET
@Produces(MediaType.APPLICATION_JSON)
List<ClientRepresentation> findByClientId(@QueryParam("clientId") String clientId);
ClientRepresentation findByClientId(@QueryParam("clientId") String clientId);
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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?

Copy link
Author

Choose a reason for hiding this comment

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

@shawkins Done !

bmscomp
bmscomp reviewed Oct 12, 2025
View reviewed changes
...rk/examples/tests/src/test/java/org/keycloak/test/examples/RealmSpecificAdminClientTest.java Outdated

ClientRepresentation client = clients.get(0);
ClientRepresentation client = realmResource.clients().findByClientId("myclient");
assertNotNull(client);
Copy link

Choose a reason for hiding this comment

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

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);

@cherifyazid 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 shawkins requested a review from mposolda October 13, 2025 16:02
@shawkins shawkins force-pushed the feature/clientResource-return-single-item branch from 086ef7a to fd73a1f Compare October 13, 2025 19:35
@cherifyazid cherifyazid force-pushed the feature/clientResource-return-single-item branch from fd73a1f to 56f9526 Compare October 15, 2025 12:41
@cherifyazid cherifyazid requested review from a team as code owners October 15, 2025 12:41
@cherifyazid cherifyazid force-pushed the feature/clientResource-return-single-item branch from 56f9526 to 40d7aab Compare October 15, 2025 12:56
@yadzi @cherifyazid
Client Resource must return a single item (keycloak#39851)
2d16e23 
 Closes keycloak#39851

Signed-off-by: CHERIF Yazid <yadzinov@gmail.com>

Remove unused imports as part of keycloak#43233

Signed-off-by: stianst <stianst@gmail.com>
@cherifyazid cherifyazid force-pushed the feature/clientResource-return-single-item branch from 40d7aab to 2d16e23 Compare October 15, 2025 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shawkins shawkins shawkins left review comments

@mposolda mposolda Awaiting requested review from mposolda

+1 more reviewer

@bmscomp bmscomp bmscomp left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

team/cloud-native team/core-clients team/core-iam team/sre team/ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ClientsResource.findByClientId(String) should return a single item

4 participants

@cherifyazid @bmscomp @shawkins @yadzi
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载