Avoid touching the database layer if no changes are necessary for a user

Closes #43682

Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
This commit is contained in:
Alexander Schwartz 2025-11-05 14:42:40 +01:00 committed by GitHub
parent 9ebab2f017
commit bb9015a1f2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 71 additions and 11 deletions

View File

@ -170,6 +170,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void setEnabled(boolean enabled) {
if (updated == null && cached.isEnabled() == enabled) {
return;
}
getDelegateForUpdate();
updated.setEnabled(enabled);
}
@ -214,10 +217,11 @@ public class UserAdapter implements CachedUserModel {
@Override
public void removeAttribute(String name) {
if (getFirstAttribute(name) != null) {
getDelegateForUpdate();
updated.removeAttribute(name);
if (updated == null && getFirstAttribute(name) == null) {
return;
}
getDelegateForUpdate();
updated.removeAttribute(name);
}
@Override
@ -247,30 +251,42 @@ public class UserAdapter implements CachedUserModel {
@Override
public void addRequiredAction(RequiredAction action) {
if (action == null || updated == null && getCachedRequiredActions().contains(action.name())) {
return;
}
getDelegateForUpdate();
updated.addRequiredAction(action);
}
@Override
public void removeRequiredAction(RequiredAction action) {
if (getRequiredActionsStream().anyMatch(s -> Objects.equals(s, action.name()))) {
getDelegateForUpdate();
updated.removeRequiredAction(action);
if (action == null || updated == null && !getCachedRequiredActions().contains(action.name())) {
return;
}
getDelegateForUpdate();
updated.removeRequiredAction(action);
}
@Override
public void addRequiredAction(String action) {
if (updated == null && getCachedRequiredActions().contains(action)) {
return;
}
getDelegateForUpdate();
updated.addRequiredAction(action);
}
@Override
public void removeRequiredAction(String action) {
if (getRequiredActionsStream().anyMatch(s -> Objects.equals(s, action))) {
getDelegateForUpdate();
updated.removeRequiredAction(action);
if (updated == null && !getCachedRequiredActions().contains(action)) {
return;
}
getDelegateForUpdate();
updated.removeRequiredAction(action);
}
private Set<String> getCachedRequiredActions() {
return cached.getRequiredActions(keycloakSession, modelSupplier);
}
@Override
@ -281,6 +297,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void setEmailVerified(boolean verified) {
if (updated == null && cached.isEmailVerified() == verified) {
return;
}
getDelegateForUpdate();
updated.setEmailVerified(verified);
}
@ -293,6 +312,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void setFederationLink(String link) {
if (updated == null && Objects.equals(cached.getFederationLink(), link)) {
return;
}
getDelegateForUpdate();
updated.setFederationLink(link);
}
@ -305,6 +327,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void setServiceAccountClientLink(String clientInternalId) {
if (updated == null && Objects.equals(cached.getServiceAccountClientLink(), clientInternalId)) {
return;
}
getDelegateForUpdate();
updated.setServiceAccountClientLink(clientInternalId);
}
@ -388,6 +413,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void grantRole(RoleModel role) {
if (updated == null && cached.getRoleMappings(keycloakSession, modelSupplier).contains(role.getId())) {
return;
}
getDelegateForUpdate();
updated.grantRole(role);
}
@ -411,6 +439,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void deleteRoleMapping(RoleModel role) {
if (updated == null && !cached.getRoleMappings(keycloakSession, modelSupplier).contains(role.getId())) {
return;
}
getDelegateForUpdate();
updated.deleteRoleMapping(role);
}
@ -454,6 +485,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void joinGroup(GroupModel group) {
if (group.getType() == Type.REALM && cached.getGroups(keycloakSession, modelSupplier).contains(group.getId())) {
return;
}
getDelegateForUpdate();
updated.joinGroup(group);
@ -461,6 +495,9 @@ public class UserAdapter implements CachedUserModel {
@Override
public void leaveGroup(GroupModel group) {
if (group.getType() == Type.REALM && updated == null && !cached.getGroups(keycloakSession, modelSupplier).contains(group.getId())) {
return;
}
getDelegateForUpdate();
updated.leaveGroup(group);
}

View File

@ -376,7 +376,13 @@ public class UserCacheSession implements UserCache, OnCreateComponent, OnUpdateC
}
}
return new UserAdapter(cached, this, session, realm);
UserAdapter userAdapter = new UserAdapter(cached, this, session, realm);
if (isReadOnlyOrganizationMember(session, userAdapter)) {
return new ReadOnlyUserModelDelegate(userAdapter, false);
}
return userAdapter;
}
protected UserModel cacheUser(RealmModel realm, UserModel delegate, Long revision) {

View File

@ -169,6 +169,7 @@ public class JpaChangesPerformer<K, V extends SessionEntity> {
SessionUpdatesList<V> sessionUpdates = entry.getValue();
SessionEntityWrapper<V> sessionWrapper = sessionUpdates.getEntityWrapper();
RealmModel realm = sessionUpdates.getRealm();
session.getContext().setRealm(realm);
UserSessionPersisterProvider userSessionPersister = session.getProvider(UserSessionPersisterProvider.class);
switch (merged.getOperation()) {
@ -433,6 +434,7 @@ public class JpaChangesPerformer<K, V extends SessionEntity> {
SessionUpdatesList<V> sessionUpdates = entry.getValue();
SessionEntityWrapper<V> sessionWrapper = sessionUpdates.getEntityWrapper();
RealmModel realm = sessionUpdates.getRealm();
session.getContext().setRealm(realm);
UserSessionPersisterProvider userSessionPersister = session.getProvider(UserSessionPersisterProvider.class);
UserSessionEntity entity = (UserSessionEntity) sessionWrapper.getEntity();

View File

@ -259,6 +259,7 @@ public class LDAPAdminRestApiTest extends AbstractLDAPTest {
UserResource userResource = testRealm().users().get(newUserId1);
try {
user1.setFirstName(user1.getFirstName() + " updated");
userResource.update(user1);
Assert.fail("Not expected to successfully update user");
} catch (WebApplicationException expected) {

View File

@ -1658,6 +1658,7 @@ public class LDAPProvidersIntegrationTest extends AbstractLDAPTest {
RealmModel testRealm = ctx.getRealm();
UserModel importedUser = UserStoragePrivateUtil.userLocalStorage(session).getUserByUsername(testRealm, "beckybecks");
Assert.assertNotNull(importedUser);
// Update user 'beckybecks' in LDAP
LDAPObject becky = ctx.getLdapProvider().loadLDAPUserByUsername(testRealm, importedUser.getUsername());

View File

@ -146,7 +146,7 @@ public class UserSessionProviderTest extends AbstractTestRealmKeycloakTest {
Time.setOffset(100);
try {
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), kcSession -> {
kcSession.getContext().setRealm(realm);
kcSession.getContext().setRealm(session.realms().getRealm(realm.getId()));
UserSessionModel userSession = kcSession.sessions().getUserSession(realm, sessions[1].getId());
assertSession(userSession, kcSession.users().getUserByUsername(realm, "user1"), "127.0.0.2", started, started, "test-app");
AuthenticatedClientSessionModel clientSession = userSession.getAuthenticatedClientSessionByClient(realm.getClientByClientId("test-app").getId());
@ -166,6 +166,7 @@ public class UserSessionProviderTest extends AbstractTestRealmKeycloakTest {
});
KeycloakModelUtils.runJobInTransaction(session.getKeycloakSessionFactory(), kcSession -> {
kcSession.getContext().setRealm(session.realms().getRealm(realm.getId()));
UserSessionModel userSession = kcSession.sessions().getUserSession(realm, sessions[1].getId());
assertThat(userSession.getNotes(), Matchers.anEmptyMap());

View File

@ -166,12 +166,14 @@ public class UserSessionPersisterProviderTest extends KeycloakModelTest {
inComittedTransaction(session -> {
// Persist 1 online session
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
userSession[0] = session.sessions().getUserSession(realm, origSessions[0].getId());
persistUserSession(session, userSession[0], false);
});
inComittedTransaction(session -> { // Assert online session
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
List<UserSessionModel> loadedSessions = loadPersistedSessionsPaginated(session, false, 1, 1, 1);
assertSession(loadedSessions.get(0), session.users().getUserByUsername(realm, "user1"), "127.0.0.1", started, started, "test-app", "third-party");
});
@ -180,6 +182,7 @@ public class UserSessionPersisterProviderTest extends KeycloakModelTest {
inComittedTransaction(session -> {
// Assert offline sessions
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
List<UserSessionModel> loadedSessions = loadPersistedSessionsPaginated(session, true, 2, 2, 3);
assertSessions(loadedSessions, new String[] { origSessions[0].getId(), origSessions[1].getId(), origSessions[2].getId() });
@ -216,6 +219,7 @@ public class UserSessionPersisterProviderTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class);
@ -245,6 +249,7 @@ public class UserSessionPersisterProviderTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class);
@ -649,6 +654,7 @@ public class UserSessionPersisterProviderTest extends KeycloakModelTest {
// Update one of the sessions with lastSessionRefresh of 20 days ahead
int lastSessionRefresh = Time.currentTime() + 1728000;
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
UserSessionPersisterProvider persister = session.getProvider(UserSessionPersisterProvider.class);
persister.updateLastSessionRefreshes(realm, lastSessionRefresh, Collections.singleton(userSession1[0].getId()), true);

View File

@ -169,6 +169,7 @@ public class UserSessionProviderModelTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
// assert the user session is still there
UserSessionModel userSession = session.sessions().getUserSession(realm, origSessions[0].getId());

View File

@ -167,6 +167,7 @@ public class UserSessionProviderOfflineModelTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
persister = session.getProvider(UserSessionPersisterProvider.class);
UserSessionModel session0 = session.sessions().getOfflineUserSession(realm, origSessions[0].getId());
@ -193,6 +194,7 @@ public class UserSessionProviderOfflineModelTest extends KeycloakModelTest {
int timeOffset = 1728000 + (i * 86400);
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
setTimeOffset(timeOffset);
log.infof("Set time offset to %d. Time is: %d", timeOffset, Time.currentTime());
@ -216,6 +218,7 @@ public class UserSessionProviderOfflineModelTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
persister = session.getProvider(UserSessionPersisterProvider.class);
// assert session0 is the only session found
@ -298,6 +301,7 @@ public class UserSessionProviderOfflineModelTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
persister = session.getProvider(UserSessionPersisterProvider.class);
// Expire everything except offline client sessions
@ -308,6 +312,7 @@ public class UserSessionProviderOfflineModelTest extends KeycloakModelTest {
inComittedTransaction(session -> {
RealmModel realm = session.realms().getRealm(realmId);
session.getContext().setRealm(realm);
sessionManager = new UserSessionManager(session);
persister = session.getProvider(UserSessionPersisterProvider.class);