+
Skip to content

Fixed the IDs comparison logic #40667

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Anchels
Copy link
Contributor

@Anchels Anchels commented Jun 23, 2025

Closes #40666

Closes keycloak#40666

Signed-off-by: Anchels <mishtitov@gmail.com>
@Anchels Anchels requested a review from a team as a code owner June 23, 2025 13:04
@shawkins
Copy link
Contributor

@Anchels thank you for the PR, are you able to add a test for this case as well?

Co-authored-by: Steven Hawkins <shawkins@redhat.com>
Signed-off-by: Anchels <42744001+Anchels@users.noreply.github.com>
@Anchels
Copy link
Contributor Author

Anchels commented Jun 24, 2025

@shawkins sorry, I don't have experience with writing tests

@shawkins
Copy link
Contributor

@shawkins sorry, I don't have experience with writing tests

No problem.

@rmartinc @sguilhen @ahus1 can you review and indicate whether you want a test case?

@sguilhen
Copy link
Contributor

Not sure if this fix is enough - see https://github.com/keycloak/keycloak/blob/main/model/jpa/src/main/java/org/keycloak/connections/jpa/updater/liquibase/lock/CustomLockService.java#L130-L139

The CustomInitializeDatabaseChangeLogLockTableStatement in line 139 should probably be using the ids from the Namespace enum. As it stands now, it is updating the table with the same values it has read in line 130. So, to me, besides fixing the wrong type comparison we should also fix the initialization to set the ids from the enum

…IDs from enum

Signed-off-by: Anchels <mishtitov@gmail.com>
@Anchels
Copy link
Contributor Author

Anchels commented Jun 26, 2025

@sguilhen, could you please see if the latest changes will help fix the problem? Or should we combine currentIds with enum IDs for the initializatoin?

@sguilhen
Copy link
Contributor

sguilhen commented Jul 7, 2025

To be on the safe side I would combine them.

Closes keycloak#40666

Signed-off-by: Anchels <mishtitov@gmail.com>
@Anchels Anchels requested a review from shawkins July 17, 2025 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect types of collection arguments in CustomLockService. SAST
3 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载