From 783eecf612d43cf6869ce9d1808f195dd94773be Mon Sep 17 00:00:00 2001 From: vramik Date: Tue, 30 Nov 2021 00:29:58 +0100 Subject: [PATCH] Closes #8808 - Convert MapRoleEntity to interface --- .../models/map/role/MapRoleAdapter.java | 19 ++- .../models/map/role/MapRoleEntity.java | 139 +++++------------- .../models/map/role/MapRoleProvider.java | 9 +- ...ncurrentHashMapStorageProviderFactory.java | 5 +- 4 files changed, 60 insertions(+), 112 deletions(-) diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java index 65d003b2de..157680f3d2 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleAdapter.java @@ -21,6 +21,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.stream.Stream; import org.jboss.logging.Logger; import static org.keycloak.common.util.StackUtil.getShortStackTrace; @@ -65,21 +66,23 @@ public class MapRoleAdapter extends AbstractRoleModel implements @Override public boolean isComposite() { - return ! entity.getCompositeRoles().isEmpty(); + return ! (entity.getCompositeRoles() == null || entity.getCompositeRoles().isEmpty()); } @Override public Stream getCompositesStream() { - LOG.tracef("%% %s(%s).getCompositesStream():%d - %s", entity.getName(), entity.getId(), entity.getCompositeRoles().size(), getShortStackTrace()); - return entity.getCompositeRoles().stream() + Set compositeRoles = entity.getCompositeRoles() == null ? Collections.emptySet() : entity.getCompositeRoles(); + LOG.tracef("%% %s(%s).getCompositesStream():%d - %s", entity.getName(), entity.getId(), compositeRoles.size(), getShortStackTrace()); + return compositeRoles.stream() .map(uuid -> session.roles().getRoleById(realm, uuid)) .filter(Objects::nonNull); } @Override public Stream getCompositesStream(String search, Integer first, Integer max) { - LOG.tracef("%% (%s).getCompositesStream(%s, %d, %d):%d - %s", this, search, first, max, entity.getCompositeRoles().size(), getShortStackTrace()); - return session.roles().getRolesStream(realm, entity.getCompositeRoles().stream(), search, first, max); + Set compositeRoles = entity.getCompositeRoles() == null ? Collections.emptySet() : entity.getCompositeRoles(); + LOG.tracef("%% (%s).getCompositesStream(%s, %d, %d):%d - %s", this, search, first, max, compositeRoles.size(), getShortStackTrace()); + return session.roles().getRolesStream(realm, compositeRoles.stream(), search, first, max); } @Override @@ -96,7 +99,8 @@ public class MapRoleAdapter extends AbstractRoleModel implements @Override public boolean isClientRole() { - return entity.isClientRole(); + final Boolean clientRole = entity.isClientRole(); + return clientRole == null ? false : clientRole; } @Override @@ -126,7 +130,8 @@ public class MapRoleAdapter extends AbstractRoleModel implements @Override public Map> getAttributes() { - return entity.getAttributes(); + Map> attributes = entity.getAttributes(); + return attributes == null ? Collections.emptyMap() : attributes; } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java index ab48ab14f3..ba5f7be4be 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleEntity.java @@ -16,131 +16,70 @@ */ package org.keycloak.models.map.role; -import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Set; +import org.keycloak.models.map.annotations.GenerateEntityImplementations; import org.keycloak.models.map.common.AbstractEntity; +import org.keycloak.models.map.common.DeepCloner; import org.keycloak.models.map.common.UpdatableEntity; -public class MapRoleEntity extends UpdatableEntity.Impl implements AbstractEntity { +@GenerateEntityImplementations( + inherits = "org.keycloak.models.map.role.MapRoleEntity.AbstractRoleEntity" +) +@DeepCloner.Root +public interface MapRoleEntity extends AbstractEntity, UpdatableEntity { - private String id; - private String realmId; + public abstract class AbstractRoleEntity extends UpdatableEntity.Impl implements MapRoleEntity { - private String name; - private String description; - private boolean clientRole; - private String clientId; - private Set compositeRoles = new HashSet<>(); - private Map> attributes = new HashMap<>(); + private String id; - /** - * Flag signalizing that any of the setters has been meaningfully used. - */ + @Override + public String getId() { + return this.id; + } - public MapRoleEntity() {} + @Override + public void setId(String id) { + if (this.id != null) throw new IllegalStateException("Id cannot be changed"); + this.id = id; + this.updated |= id != null; + } - public MapRoleEntity(String id, String realmId) { - this.id = id; - this.realmId = realmId; } - @Override - public String getId() { - return this.id; + default Boolean isComposite() { + return ! (getCompositeRoles() == null || getCompositeRoles().isEmpty()); } - @Override - public void setId(String id) { - if (this.id != null) throw new IllegalStateException("Id cannot be changed"); - this.id = id; - this.updated |= id != null; - } + Boolean isClientRole(); - public String getName() { - return name; - } + String getRealmId(); - public void setName(String name) { - this.updated |= ! Objects.equals(this.name, name); - this.name = name; - } + String getClientId(); - public String getDescription() { - return description; - } + String getName(); - public void setDescription(String description) { - this.updated |= ! Objects.equals(this.description, description); - this.description = description; - } + String getDescription(); - public Map> getAttributes() { - return attributes; - } + void setClientRole(Boolean clientRole); - public void setAttributes(Map> attributes) { - this.updated |= ! Objects.equals(this.attributes, attributes); - this.attributes = attributes; - } + void setRealmId(String realmId); - public void setAttribute(String name, List values) { - this.updated |= ! Objects.equals(this.attributes.put(name, values), values); - } + void setClientId(String clientId); - public void removeAttribute(String name) { - this.updated |= this.attributes.remove(name) != null; - } + void setName(String name); - public String getRealmId() { - return realmId; - } + void setDescription(String description); - public void setRealmId(String realmId) { - this.updated |= ! Objects.equals(this.realmId, realmId); - this.realmId = realmId; - } + Set getCompositeRoles(); + void setCompositeRoles(Set compositeRoles); + void addCompositeRole(String roleId); + void removeCompositeRole(String roleId); - public boolean isClientRole() { - return clientRole; - } + Map> getAttributes(); + void setAttributes(Map> attributes); + void setAttribute(String name, List values); + void removeAttribute(String name); - public void setClientRole(boolean clientRole) { - this.updated |= ! Objects.equals(this.clientRole, clientRole); - this.clientRole = clientRole; - } - - public boolean isComposite() { - return ! (compositeRoles == null || compositeRoles.isEmpty()); - } - - public Set getCompositeRoles() { - return compositeRoles; - } - - public String getClientId() { - return clientId; - } - - public void setClientId(String clientId) { - this.updated |= ! Objects.equals(this.clientId, clientId); - this.clientId = clientId; - } - - public void setCompositeRoles(Set compositeRoles) { - this.updated |= ! Objects.equals(this.compositeRoles, compositeRoles); - this.compositeRoles.clear(); - this.compositeRoles.addAll(compositeRoles); - } - - public void addCompositeRole(String roleId) { - this.updated |= this.compositeRoles.add(roleId); - } - - public void removeCompositeRole(String roleId) { - this.updated |= this.compositeRoles.remove(roleId); - } } diff --git a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java index 5d3062f30b..6290241380 100644 --- a/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java +++ b/model/map/src/main/java/org/keycloak/models/map/role/MapRoleProvider.java @@ -66,9 +66,10 @@ public class MapRoleProvider implements RoleProvider { LOG.tracef("addRealmRole(%s, %s, %s)%s", realm, id, name, getShortStackTrace()); - MapRoleEntity entity = new MapRoleEntity(id, realm.getId()); - entity.setName(name); + MapRoleEntity entity = new MapRoleEntityImpl(); + entity.setId(id); entity.setRealmId(realm.getId()); + entity.setName(name); if (tx.read(entity.getId()) != null) { throw new ModelDuplicateException("Role exists: " + id); } @@ -121,7 +122,9 @@ public class MapRoleProvider implements RoleProvider { LOG.tracef("addClientRole(%s, %s, %s)%s", client, id, name, getShortStackTrace()); - MapRoleEntity entity = new MapRoleEntity(id, client.getRealm().getId()); + MapRoleEntity entity = new MapRoleEntityImpl(); + entity.setId(id); + entity.setRealmId(client.getRealm().getId()); entity.setName(name); entity.setClientRole(true); entity.setClientId(client.getId()); diff --git a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java index 89f987d203..c56c179f9a 100644 --- a/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java +++ b/model/map/src/main/java/org/keycloak/models/map/storage/chm/ConcurrentHashMapStorageProviderFactory.java @@ -56,6 +56,7 @@ import org.keycloak.models.map.group.MapGroupEntityImpl; import org.keycloak.models.map.loginFailure.MapUserLoginFailureEntity; import org.keycloak.models.map.realm.MapRealmEntity; import org.keycloak.models.map.role.MapRoleEntity; +import org.keycloak.models.map.role.MapRoleEntityImpl; import com.fasterxml.jackson.databind.JavaType; import java.io.File; import java.io.IOException; @@ -106,6 +107,7 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide .constructorDC(MapClientEntityImpl.class, MapClientEntityImpl::new) .constructor(MapProtocolMapperEntity.class, MapProtocolMapperEntityImpl::new) .constructor(MapGroupEntityImpl.class, MapGroupEntityImpl::new) + .constructor(MapRoleEntityImpl.class, MapRoleEntityImpl::new) .build(); public static final Map, String> MODEL_TO_NAME = new HashMap<>(); @@ -154,10 +156,9 @@ public class ConcurrentHashMapStorageProviderFactory implements AmphibianProvide static { INTERFACE_TO_IMPL.put(MapClientEntity.class, MapClientEntityImpl.class); // INTERFACE_TO_IMPL.put(MapClientScopeEntity.class, MapClientScopeEntityImpl.class); -// INTERFACE_TO_IMPL.put(MapClientEntity.class, MapClientEntityImpl.class); INTERFACE_TO_IMPL.put(MapGroupEntity.class, MapGroupEntityImpl.class); // INTERFACE_TO_IMPL.put(MapRealmEntity.class, MapRealmEntityImpl.class); -// INTERFACE_TO_IMPL.put(MapRoleEntity.class, MapRoleEntityImpl.class); + INTERFACE_TO_IMPL.put(MapRoleEntity.class, MapRoleEntityImpl.class); // INTERFACE_TO_IMPL.put(MapRootAuthenticationSessionEntity.class, MapRootAuthenticationSessionEntityImpl.class); // INTERFACE_TO_IMPL.put(MapUserLoginFailureEntity.class, MapUserLoginFailureEntityImpl.class); // INTERFACE_TO_IMPL.put(MapUserEntity.class, MapUserEntityImpl.class);