upstream/keycloak
1
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-02-03 20:39:33 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Optimize deletion of composite roles

Browse source 
Closes #45065

Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
This commit is contained in:
Alexander Schwartz 2026-01-28 12:05:16 +01:00 committed by GitHub
parent f2f185b367
commit 0ddb355d3d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 207 additions and 69 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RoleAdapter.java vendored
View file

@ -142,7 +142,7 @@ public class RoleAdapter implements RoleModel {
if (composites == null) {
composites = new HashSet<>();
for (String id : cached.getComposites()) {
for (String id : cached.getComposites(session, modelSupplier)) {
RoleModel role = realm.getRoleById(id);
if (role == null) {
// chance that composite role was removed, so invalidate this entry and fallback to delegate
@ -160,7 +160,7 @@ public class RoleAdapter implements RoleModel {
public Stream<RoleModel> getCompositesStream(String search, Integer first, Integer max) {
if (isUpdated()) return updated.getCompositesStream(search, first, max);
return cacheSession.getRoleDelegate().getRolesStream(realm, cached.getComposites().stream(), search, first, max);
return cacheSession.getRoleDelegate().getRolesStream(realm, cached.getComposites(session, modelSupplier).stream(), search, first, max);
}
@Override

21
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedGroup.java vendored
View file

@ -18,6 +18,7 @@
package org.keycloak.models.cache.infinispan.entities;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
@ -43,6 +44,11 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {
private final String parentId;
private final LazyLoader<GroupModel, MultivaluedHashMap<String, String>> attributes;
private final LazyLoader<GroupModel, Set<String>> roleMappings;
/**
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted.
*/
private Set<String> cachedRoleMappings = new HashSet<>();
private final LazyLoader<GroupModel, Set<String>> subGroups;
private final Type type;
@ -68,11 +74,16 @@ public class CachedGroup extends AbstractRevisioned implements InRealm {
}
public Set<String> getRoleMappings(KeycloakSession session, Supplier<GroupModel> group) {
// it may happen that groups were not loaded before so we don't actually need to invalidate entries in the cache
if (group == null) {
return Collections.emptySet();
}
return roleMappings.get(session, group);
cachedRoleMappings = roleMappings.get(session, group);
return cachedRoleMappings;
}
/**
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted. Will return an empty list if it hasn't been cached yet (and then no invalidation is necessary)
*/
public Set<String> getCachedRoleMappings() {
return cachedRoleMappings;
}
public String getName() {

27
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/entities/CachedRole.java vendored
View file

@ -38,8 +38,13 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
final protected String name;
final protected String realm;
final protected String description;
final protected boolean composite;
final protected Set<String> composites = new HashSet<>();
protected boolean composite;
final protected LazyLoader<RoleModel, Set<String>> composites;
/**
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted.
*/
private Set<String> cachedComposites = new HashSet<>();
private final LazyLoader<RoleModel, MultivaluedHashMap<String, String>> attributes;
public CachedRole(Long revision, RoleModel model, RealmModel realm) {
@ -49,7 +54,9 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
name = model.getName();
this.realm = realm.getId();
if (composite) {
composites.addAll(model.getCompositesStream().map(RoleModel::getId).collect(Collectors.toSet()));
composites = new DefaultLazyLoader<>(roleModel -> roleModel.getCompositesStream().map(RoleModel::getId).collect(Collectors.toSet()), HashSet::new);
} else {
composites = new DefaultLazyLoader<>(roleModel -> new HashSet<>(), HashSet::new);
}
attributes = new DefaultLazyLoader<>(roleModel -> new MultivaluedHashMap<>(roleModel.getAttributes()), MultivaluedHashMap::new);
}
@ -70,8 +77,18 @@ public class CachedRole extends AbstractRevisioned implements InRealm {
return composite;
}
public Set<String> getComposites() {
return composites;
public Set<String> getComposites(KeycloakSession session, Supplier<RoleModel> roleModel) {
cachedComposites = composites.get(session, roleModel);
composite = !cachedComposites.isEmpty();
return cachedComposites;
}
/**
* Use this so the cache invalidation can retrieve any previously cached role mappings to determine if this
* items should be evicted. Will return an empty list if it hasn't been cached yet (and then no invalidation is necessary)
*/
public Set<String> getCachedComposites() {
return cachedComposites;
}
public MultivaluedHashMap<String, String> getAttributes(KeycloakSession session, Supplier<RoleModel> roleModel) {

4
model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/stream/HasRolePredicate.java vendored
View file

@ -43,8 +43,8 @@ public class HasRolePredicate implements Predicate<Map.Entry<String, Revisioned>
@Override
public boolean test(Map.Entry<String, Revisioned> entry) {
Object value = entry.getValue();
return (value instanceof CachedRole cachedRole && cachedRole.getComposites().contains(role)) ||
(value instanceof CachedGroup cachedGroup && cachedGroup.getRoleMappings(null, null).contains(role)) ||
return (value instanceof CachedRole cachedRole && cachedRole.getCachedComposites().contains(role)) ||
(value instanceof CachedGroup cachedGroup && cachedGroup.getCachedRoleMappings().contains(role)) ||
(value instanceof RoleQuery roleQuery && roleQuery.getRoles().contains(role)) ||
(value instanceof CachedClient cachedClient && cachedClient.getScope().contains(role)) ||
(value instanceof CachedClientScope cachedClientScope && cachedClientScope.getScope().contains(role));

13
model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java
View file

@ -457,17 +457,8 @@ public class JpaRealmProvider implements RealmProvider, ClientProvider, ClientSc
throw new ModelException("Role not found or trying to remove role from incorrect realm");
}
// Can't use a native query to delete the composite roles mappings because it causes TransientObjectException.
// At the same time, can't use the persist cascade type on the compositeRoles field because in that case
// we could not still use a native query as a different problem would arise - it may happen that a parent role that
// has this role as a composite is present in the persistence context. In that case it, the role would be re-created
// again after deletion through persist cascade type.
// So in any case, native query is not an option. This is not optimal as it executes additional queries but
// the alternative of clearing the persistence context is not either as we don't know if something currently present
// in the context is not needed later.
roleEntity.getCompositeRoles().forEach(childRole -> childRole.getParentRoles().remove(roleEntity));
roleEntity.getParentRoles().forEach(parentRole -> parentRole.getCompositeRoles().remove(roleEntity));
em.createNamedQuery("deleteRoleFromComposites").setParameter("role", roleEntity)
.executeUpdate();
em.createNamedQuery("deleteClientScopeRoleMappingByRole").setParameter("role", roleEntity).executeUpdate();

28
model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java
View file

@ -33,9 +33,11 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleContainerModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.jpa.entities.CompositeRoleEntity;
import org.keycloak.models.jpa.entities.RoleAttributeEntity;
import org.keycloak.models.jpa.entities.RoleEntity;
import org.keycloak.models.utils.KeycloakModelUtils;
import org.keycloak.utils.StreamsUtil;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
@ -90,34 +92,42 @@ public class RoleAdapter implements RoleModel, JpaModel<RoleEntity> {
@Override
public boolean isComposite() {
return getCompositesStream().count() > 0;
return getChildRoles().findAny().isPresent();
}
@Override
public void addCompositeRole(RoleModel role) {
RoleEntity entity = toRoleEntity(role);
for (RoleEntity composite : getEntity().getCompositeRoles()) {
if (composite.equals(entity)) return;
if (em.find(CompositeRoleEntity.class, new CompositeRoleEntity.Key(getEntity(), toRoleEntity(role))) == null) {
CompositeRoleEntity compositeRoleEntity = new CompositeRoleEntity(getEntity(), toRoleEntity(role));
em.persist(compositeRoleEntity);
}
getEntity().getCompositeRoles().add(entity);
}
@Override
public void removeCompositeRole(RoleModel role) {
RoleEntity entity = toRoleEntity(role);
getEntity().getCompositeRoles().remove(entity);
RoleEntity child = toRoleEntity(role);
em.createNamedQuery("deleteSingleCompositeFromRole")
.setParameter("parentRole", getEntity())
.setParameter("childRole", child)
.executeUpdate();
}
@Override
public Stream<RoleModel> getCompositesStream() {
Stream<RoleModel> composites = getEntity().getCompositeRoles().stream().map(c -> new RoleAdapter(session, realm, em, c));
Stream<RoleModel> composites = getChildRoles().map(c -> new RoleAdapter(session, realm, em, c));
return composites.filter(Objects::nonNull);
}
private Stream<RoleEntity> getChildRoles() {
return StreamsUtil.closing(em.createNamedQuery("getChildRoles", RoleEntity.class)
.setParameter("parentRoleId", getId()).getResultStream());
}
@Override
public Stream<RoleModel> getCompositesStream(String search, Integer first, Integer max) {
return session.roles().getRolesStream(realm,
getEntity().getCompositeRoles().stream().map(RoleEntity::getId),
getChildRoles().map(RoleEntity::getId),
search, first, max);
}

134
model/jpa/src/main/java/org/keycloak/models/jpa/entities/CompositeRoleEntity.java Executable file
View file

@ -0,0 +1,134 @@
/*
* Copyright 2026 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.keycloak.models.jpa.entities;
import java.io.Serializable;
import java.util.Objects;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.Id;
import jakarta.persistence.IdClass;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import jakarta.persistence.NamedQueries;
import jakarta.persistence.NamedQuery;
import jakarta.persistence.Table;
/**
* Manage compmosite role relations.
* This used to be a @ManyToMany relation in RoleEntity, and before that there was a native query which lead to stale entities.
* After those attempts, this is now a separate table that avoids iterating over a lot of parents their entries by applying a simple JPA deletion.
*/
@Entity
@Table(name="COMPOSITE_ROLE")
@NamedQueries({
@NamedQuery(name="deleteRoleFromComposites", query="delete CompositeRoleEntity c where c.parentRole = :role or c.childRole = :role"),
@NamedQuery(name="deleteSingleCompositeFromRole", query="delete CompositeRoleEntity c where c.parentRole = :parentRole and c.childRole = :childRole"),
})
@IdClass(CompositeRoleEntity.Key.class)
public class CompositeRoleEntity {
@Id
@ManyToOne(fetch=FetchType.LAZY)
@JoinColumn(name="COMPOSITE")
private RoleEntity parentRole;
@Id
@ManyToOne(fetch=FetchType.LAZY)
@JoinColumn(name="CHILD_ROLE")
private RoleEntity childRole;
public CompositeRoleEntity() {
}
public CompositeRoleEntity(RoleEntity parentRole, RoleEntity childRole) {
// Fields must not be null otherwise the automatic dependency detection of Hibernate will not work
this.parentRole = parentRole;
this.childRole = childRole;
}
public RoleEntity getParentRole() {
return parentRole;
}
public void setParentRole(RoleEntity parentRole) {
this.parentRole = parentRole;
}
public RoleEntity getChildRole() {
return childRole;
}
public void setChildRole(RoleEntity childRole) {
this.childRole = childRole;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null) return false;
if (!(o instanceof CompositeRoleEntity that)) return false;
return parentRole.equals(that.parentRole) && childRole.equals(that.childRole);
}
@Override
public int hashCode() {
return Objects.hash(childRole, parentRole);
}
public static class Key implements Serializable {
private RoleEntity childRole;
private RoleEntity parentRole;
public Key() {
}
public Key(RoleEntity parentRole, RoleEntity childRole) {
this.childRole = childRole;
this.parentRole = parentRole;
}
public RoleEntity getChildRole() {
return childRole;
}
public void setChildRole(RoleEntity childRole) {
this.childRole = childRole;
}
public RoleEntity getParentRole() {
return parentRole;
}
public void setParentRole(RoleEntity parentRole) {
this.parentRole = parentRole;
}
@Override
public boolean equals(Object o) {
if (!(o instanceof Key key)) return false;
return Objects.equals(childRole, key.childRole) && Objects.equals(parentRole, key.parentRole);
}
@Override
public int hashCode() {
return Objects.hash(childRole, parentRole);
}
}
}

36
model/jpa/src/main/java/org/keycloak/models/jpa/entities/RoleEntity.java
View file

@ -17,21 +17,15 @@
package org.keycloak.models.jpa.entities;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
import jakarta.persistence.Access;
import jakarta.persistence.AccessType;
import jakarta.persistence.CascadeType;
import jakarta.persistence.Column;
import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.JoinTable;
import jakarta.persistence.ManyToMany;
import jakarta.persistence.NamedQueries;
import jakarta.persistence.NamedQuery;
import jakarta.persistence.OneToMany;
@ -66,6 +60,7 @@ import org.hibernate.annotations.Nationalized;
@NamedQuery(name="searchForRealmRoles", query="select role from RoleEntity role where role.clientRole = false and role.realmId = :realm and ( lower(role.name) like :search or lower(role.description) like :search ) order by role.name"),
@NamedQuery(name="getRoleIdsFromIdList", query="select role.id from RoleEntity role where role.realmId = :realm and role.id in :ids order by role.name ASC"),
@NamedQuery(name="getRoleIdsByNameContainingFromIdList", query="select role.id from RoleEntity role where role.realmId = :realm and lower(role.name) like lower(concat('%',:search,'%')) and role.id in :ids order by role.name ASC"),
@NamedQuery(name="getChildRoles", query="select r from RoleEntity r join CompositeRoleEntity c on r.id = c.childRole.id where c.parentRole.id = :parentRoleId"),
})
public class RoleEntity {
@ -95,13 +90,6 @@ public class RoleEntity {
@Column(name="CLIENT_REALM_CONSTRAINT", length = 36)
private String clientRealmConstraint;
@ManyToMany(fetch = FetchType.LAZY, cascade = {})
@JoinTable(name = "COMPOSITE_ROLE", joinColumns = @JoinColumn(name = "COMPOSITE"), inverseJoinColumns = @JoinColumn(name = "CHILD_ROLE"))
private Set<RoleEntity> compositeRoles;
@ManyToMany(mappedBy = "compositeRoles", fetch = FetchType.LAZY, cascade = {})
private Set<RoleEntity> parentRoles;
// Explicitly not using OrphanRemoval as we're handling the removal manually through HQL but at the same time we still
// want to remove elements from the entity's collection in a manual way. Without this, Hibernate would do a duplicit
// delete query.
@ -154,28 +142,6 @@ public class RoleEntity {
this.description = description;
}
public Set<RoleEntity> getCompositeRoles() {
if (compositeRoles == null) {
compositeRoles = new HashSet<>();
}
return compositeRoles;
}
public Set<RoleEntity> getParentRoles() {
if (parentRoles == null) {
parentRoles = new HashSet<>();
}
return parentRoles;
}
public void setParentRoles(Set<RoleEntity> parentRoles) {
this.parentRoles = parentRoles;
}
public void setCompositeRoles(Set<RoleEntity> compositeRoles) {
this.compositeRoles = compositeRoles;
}
public boolean isClientRole() {
return clientRole;
}

1
model/jpa/src/main/resources/default-persistence.xml
View file

@ -30,6 +30,7 @@
<class>org.keycloak.models.jpa.entities.ComponentEntity</class>
<class>org.keycloak.models.jpa.entities.UserFederationProviderEntity</class>
<class>org.keycloak.models.jpa.entities.UserFederationMapperEntity</class>
<class>org.keycloak.models.jpa.entities.CompositeRoleEntity</class>
<class>org.keycloak.models.jpa.entities.RoleEntity</class>
<class>org.keycloak.models.jpa.entities.RoleAttributeEntity</class>
<class>org.keycloak.models.jpa.entities.FederatedIdentityEntity</class>

8
tests/base/src/test/java/org/keycloak/tests/admin/realm/RealmRolesCRUDTest.java
View file

@ -130,6 +130,14 @@ public class RealmRolesCRUDTest extends AbstractRealmRolesTest {
assertFalse(managedRealm.admin().roles().get("role-a").toRepresentation().isComposite());
assertEquals(0, managedRealm.admin().roles().get("role-a").getRoleComposites().size());
managedRealm.admin().roles().create(RoleConfigBuilder.create().name("role-z").build());
managedRealm.admin().roles().get("role-z").addComposites(l);
// show that I can delete a role that has composite roles
managedRealm.admin().roles().deleteRole("role-z");
// show that the roles still exist
assertNotNull(managedRealm.admin().roles().get("role-b").toRepresentation().getId());
assertNotNull(managedRealm.admin().clients().get(clientA.getId()).roles().get("role-c").toRepresentation().getId());
}
@Test