Skip to content

Fix compilation error#30416

Merged
pedroigor merged 1 commit intokeycloak:mainfrom
pruivo:t_fix_compilation_error
Jun 13, 2024
Merged

Fix compilation error#30416
pedroigor merged 1 commit intokeycloak:mainfrom
pruivo:t_fix_compilation_error

Conversation

@pruivo
Copy link
Member

@pruivo pruivo commented Jun 13, 2024

No description provided.

@pruivo pruivo requested a review from a team as a code owner June 13, 2024 17:33
@pruivo
Copy link
Member Author

pruivo commented Jun 13, 2024

Let's wait for the tests to run.

@pruivo
Copy link
Member Author

pruivo commented Jun 13, 2024

The Protostream pull request caused a conflict with #30381 and the main branch is not compiling anymore.

I'm surprised the test suite didn't fail with #30381 because the events are cast to RealmCacheInvalidationEvent. See:

((RealmCacheInvalidationEvent) event).addInvalidations(this, invalidations);

I've created new events. Can @vramik @pedroigor review this PR, please? Thanks!

@ahus1
Copy link
Member

ahus1 commented Jun 13, 2024

@pruivo - I talked to @pedroigor and he would be ok to merge this now to fix the broken build and then discuss later. He'll be in contact with you.

void registerRoleInvalidation(String id, String roleName, String roleContainerId);

void registerGroupInvalidation(String id);
void registerInvalidation(String id);
Copy link
Contributor

@pedroigor pedroigor Jun 13, 2024

Choose a reason for hiding this comment

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

One of the reasons we added this method is to allow register invalidations on the realm cache for whatever entry we want. Therefore, avoid having methods like those below that are specific to organizations.

For instance, other similar methods like registerGroup|role|etc do not make much sense if you can have a single method like that one. We are not changing this in other places but for orgs, we would like to avoid adding methods in this interface.

Can we keep the previous contract?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I can add a "generic" event and keep the old contract 👍

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@pruivo pruivo force-pushed the t_fix_compilation_error branch from f44c5b6 to b36232a Compare June 13, 2024 19:02
@pruivo
Copy link
Member Author

pruivo commented Jun 13, 2024

Thanks @ahus1 @pedroigor. I've updated the PR.

Copy link
Contributor

@pedroigor pedroigor left a comment

Choose a reason for hiding this comment

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

Thanks!

@pruivo
Copy link
Member Author

pruivo commented Jun 13, 2024

ps. didn't run the test suite locally, better wait for GHA report 👍

@pedroigor pedroigor enabled auto-merge (rebase) June 13, 2024 19:37
@pedroigor pedroigor merged commit 1b342e4 into keycloak:main Jun 13, 2024
@pruivo pruivo deleted the t_fix_compilation_error branch June 13, 2024 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants