From 91335ebaadae60e89e1a7eaa7b91b5ec4d38f419 Mon Sep 17 00:00:00 2001 From: vramik Date: Fri, 24 Jun 2022 08:10:21 +0200 Subject: [PATCH] Change returning type to Set in MapClientEntity when obtaining protocol mappers Closes #11136 --- .../hotRod/client/HotRodClientEntity.java | 16 +++++++++++++ .../jpa/client/entity/JpaClientEntity.java | 16 +++---------- .../models/map/client/MapClientAdapter.java | 19 ++++++++------- .../models/map/client/MapClientEntity.java | 24 +++++++++++++++---- .../map/client/MapClientEntityClonerTest.java | 24 +++++++++++-------- .../oidc/OIDCClientRegistrationProvider.java | 2 +- 6 files changed, 63 insertions(+), 38 deletions(-) diff --git a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/client/HotRodClientEntity.java b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/client/HotRodClientEntity.java index db9be0ab58..d68fa85105 100644 --- a/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/client/HotRodClientEntity.java +++ b/model/map-hot-rod/src/main/java/org/keycloak/models/map/storage/hotRod/client/HotRodClientEntity.java @@ -24,12 +24,14 @@ import org.keycloak.models.map.storage.hotRod.common.AbstractHotRodEntity; import org.keycloak.models.map.storage.hotRod.common.HotRodAttributeEntity; import org.keycloak.models.map.storage.hotRod.common.HotRodPair; import org.keycloak.models.map.client.MapClientEntity; +import org.keycloak.models.map.client.MapProtocolMapperEntity; import org.keycloak.models.map.storage.hotRod.common.UpdatableHotRodEntityDelegateImpl; import java.util.Collection; import java.util.LinkedList; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.stream.Stream; @@ -190,6 +192,20 @@ public class HotRodClientEntity extends AbstractHotRodEntity { .filter(me -> Objects.equals(me.getValue(), defaultScope)) .map(Map.Entry::getKey); } + + @Override + public Optional getProtocolMapper(String id) { + Set mappers = getProtocolMappers(); + if (mappers == null || mappers.isEmpty()) return Optional.empty(); + + return mappers.stream().filter(m -> Objects.equals(m.getId(), id)).findFirst(); + } + + @Override + public void removeProtocolMapper(String id) { + HotRodClientEntity entity = getHotRodEntity(); + entity.updated |= entity.protocolMappers != null && entity.protocolMappers.removeIf(m -> Objects.equals(m.id, id)); + } } @Override diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java index 3b669e1daf..f8e09ae400 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientEntity.java @@ -211,23 +211,13 @@ public class JpaClientEntity extends AbstractClientEntity implements JpaRootVers } @Override - public MapProtocolMapperEntity getProtocolMapper(String id) { - return metadata.getProtocolMapper(id); - } - - @Override - public Map getProtocolMappers() { + public Set getProtocolMappers() { return metadata.getProtocolMappers(); } @Override - public void removeProtocolMapper(String id) { - metadata.removeProtocolMapper(id); - } - - @Override - public void setProtocolMapper(String id, MapProtocolMapperEntity mapping) { - metadata.setProtocolMapper(id, mapping); + public void addProtocolMapper(MapProtocolMapperEntity mapping) { + metadata.addProtocolMapper(mapping); } @Override diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java index 227ee9ee71..ef762a1f59 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientAdapter.java @@ -524,9 +524,8 @@ public abstract class MapClientAdapter extends AbstractClientModel getProtocolMappersStream() { - final Map protocolMappers = entity.getProtocolMappers(); - return protocolMappers == null ? Stream.empty() : protocolMappers.values().stream().distinct() - .map(pmUtils::toModel); + final Set protocolMappers = entity.getProtocolMappers(); + return protocolMappers == null ? Stream.empty() : protocolMappers.stream().distinct().map(pmUtils::toModel); } @Override @@ -544,7 +543,7 @@ public abstract class MapClientAdapter extends AbstractClientModel()); } - entity.setProtocolMapper(pm.getId(), pm); + entity.addProtocolMapper(pm); return pmUtils.toModel(pm); } @@ -560,23 +559,25 @@ public abstract class MapClientAdapter extends AbstractClientModel { + entity.removeProtocolMapper(id); + addProtocolMapper(mapping); + }); } } @Override public ProtocolMapperModel getProtocolMapperById(String id) { - MapProtocolMapperEntity protocolMapper = entity.getProtocolMapper(id); - return protocolMapper == null ? null : pmUtils.toModel(protocolMapper); + return entity.getProtocolMapper(id).map(pmUtils::toModel).orElse(null); } @Override public ProtocolMapperModel getProtocolMapperByName(String protocol, String name) { - final Map protocolMappers = entity.getProtocolMappers(); + final Set protocolMappers = entity.getProtocolMappers(); if (! Objects.equals(protocol, safeGetProtocol())) { return null; } - return protocolMappers == null ? null : protocolMappers.values().stream() + return protocolMappers == null ? null : protocolMappers.stream() .filter(pm -> Objects.equals(pm.getName(), name)) .map(pmUtils::toModel) .findAny() diff --git a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java index 904f758016..d4eb97613d 100644 --- a/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java +++ b/model/map/src/main/java/org/keycloak/models/map/client/MapClientEntity.java @@ -59,13 +59,13 @@ public interface MapClientEntity extends AbstractEntity, UpdatableEntity, Entity @Override public boolean isUpdated() { return this.updated - || Optional.ofNullable(getProtocolMappers()).orElseGet(Collections::emptyMap).values().stream().anyMatch(MapProtocolMapperEntity::isUpdated); + || Optional.ofNullable(getProtocolMappers()).orElseGet(Collections::emptySet).stream().anyMatch(MapProtocolMapperEntity::isUpdated); } @Override public void clearUpdatedFlag() { this.updated = false; - Optional.ofNullable(getProtocolMappers()).orElseGet(Collections::emptyMap).values().forEach(UpdatableEntity::clearUpdatedFlag); + Optional.ofNullable(getProtocolMappers()).orElseGet(Collections::emptySet).forEach(UpdatableEntity::clearUpdatedFlag); } @Override @@ -75,6 +75,20 @@ public interface MapClientEntity extends AbstractEntity, UpdatableEntity, Entity .filter(me -> Objects.equals(me.getValue(), defaultScope)) .map(Entry::getKey); } + + @Override + public Optional getProtocolMapper(String id) { + Set mappers = getProtocolMappers(); + if (mappers == null || mappers.isEmpty()) return Optional.empty(); + + return mappers.stream().filter(mapper -> Objects.equals(mapper.getId(), id)).findFirst(); + } + + @Override + public void removeProtocolMapper(String id) { + Set mappers = getProtocolMappers(); + this.updated |= mappers != null && mappers.removeIf(mapper -> Objects.equals(mapper.getId(), id)); + } } Map getClientScopes(); @@ -82,9 +96,9 @@ public interface MapClientEntity extends AbstractEntity, UpdatableEntity, Entity void setClientScope(String id, Boolean defaultScope); void removeClientScope(String id); - MapProtocolMapperEntity getProtocolMapper(String id); - Map getProtocolMappers(); - void setProtocolMapper(String id, MapProtocolMapperEntity mapping); + Optional getProtocolMapper(String id); + Set getProtocolMappers(); + void addProtocolMapper(MapProtocolMapperEntity mapping); void removeProtocolMapper(String id); void addRedirectUri(String redirectUri); diff --git a/model/map/src/test/java/org/keycloak/models/map/client/MapClientEntityClonerTest.java b/model/map/src/test/java/org/keycloak/models/map/client/MapClientEntityClonerTest.java index a764b1b5cb..0b714d4e8f 100644 --- a/model/map/src/test/java/org/keycloak/models/map/client/MapClientEntityClonerTest.java +++ b/model/map/src/test/java/org/keycloak/models/map/client/MapClientEntityClonerTest.java @@ -92,7 +92,7 @@ public class MapClientEntityClonerTest { config.put("key1", "value1"); config.put("key2", "value2"); pmm.setConfig(config); - newInstance.setProtocolMapper("pmm-id", pmm); + newInstance.addProtocolMapper(pmm); newInstance.setAttribute("attr", Arrays.asList("aa", "bb", "cc")); MapClientEntity clonedInstance = CLONER.newInstance(MapClientEntity.class); @@ -108,10 +108,12 @@ public class MapClientEntityClonerTest { assertThat(clonedInstance.getAttributes().get("attr"), not(sameInstance(newInstance.getAttributes().get("attr")))); assertThat(clonedInstance.getProtocolMappers(), not(sameInstance(newInstance.getProtocolMappers()))); - assertThat(clonedInstance.getProtocolMapper("pmm-id"), not(sameInstance(newInstance.getProtocolMapper("pmm-id")))); - assertThat(clonedInstance.getProtocolMapper("pmm-id"), equalTo(newInstance.getProtocolMapper("pmm-id"))); - assertThat(clonedInstance.getProtocolMapper("pmm-id").getConfig(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").getConfig()))); - assertThat(clonedInstance.getProtocolMapper("pmm-id").getConfig(), equalTo(newInstance.getProtocolMapper("pmm-id").getConfig())); + assertThat(clonedInstance.getProtocolMapper("pmm-id").isPresent(), is(true)); + assertThat(newInstance.getProtocolMapper("pmm-id").isPresent(), is(true)); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").get()))); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get(), equalTo(newInstance.getProtocolMapper("pmm-id").get())); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get().getConfig(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").get().getConfig()))); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get().getConfig(), equalTo(newInstance.getProtocolMapper("pmm-id").get().getConfig())); assertThat(clonedInstance.getAuthenticationFlowBindingOverrides(), nullValue()); assertThat(clonedInstance.getRegistrationToken(), nullValue()); @@ -130,7 +132,7 @@ public class MapClientEntityClonerTest { config.put("key2", "value2"); pmm.setConfig(config); - newInstance.setProtocolMapper("pmm-id", pmm); + newInstance.addProtocolMapper(pmm); newInstance.setAttribute("attr", Arrays.asList("aa", "bb", "cc")); MapClientEntity clonedInstance = CLONER.newInstance(MapClientEntity.class); @@ -146,10 +148,12 @@ public class MapClientEntityClonerTest { assertThat(clonedInstance.getAttributes().get("attr"), not(sameInstance(newInstance.getAttributes().get("attr")))); assertThat(clonedInstance.getProtocolMappers(), not(sameInstance(newInstance.getProtocolMappers()))); - assertThat(clonedInstance.getProtocolMapper("pmm-id"), not(sameInstance(newInstance.getProtocolMapper("pmm-id")))); - assertThat(clonedInstance.getProtocolMapper("pmm-id"), equalTo(newInstance.getProtocolMapper("pmm-id"))); - assertThat(clonedInstance.getProtocolMapper("pmm-id").getConfig(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").getConfig()))); - assertThat(clonedInstance.getProtocolMapper("pmm-id").getConfig(), equalTo(newInstance.getProtocolMapper("pmm-id").getConfig())); + assertThat(clonedInstance.getProtocolMapper("pmm-id").isPresent(), is(true)); + assertThat(newInstance.getProtocolMapper("pmm-id").isPresent(), is(true)); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").get()))); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get(), equalTo(newInstance.getProtocolMapper("pmm-id").get())); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get().getConfig(), not(sameInstance(newInstance.getProtocolMapper("pmm-id").get().getConfig()))); + assertThat(clonedInstance.getProtocolMapper("pmm-id").get().getConfig(), equalTo(newInstance.getProtocolMapper("pmm-id").get().getConfig())); assertThat(clonedInstance.getAuthenticationFlowBindingOverrides(), nullValue()); assertThat(clonedInstance.getRegistrationToken(), nullValue()); diff --git a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java b/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java index 6cc2d6edff..c9d3f43bcc 100755 --- a/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java +++ b/services/src/main/java/org/keycloak/services/clientregistration/oidc/OIDCClientRegistrationProvider.java @@ -164,7 +164,7 @@ public class OIDCClientRegistrationProvider extends AbstractClientRegistrationPr } else { return false; } - }).forEach((ProtocolMapperModel mapping) -> { + }).collect(Collectors.toList()).forEach((ProtocolMapperModel mapping) -> { PairwiseSubMapperHelper.setSectorIdentifierUri(mapping, sectorIdentifierUri); clientModel.updateProtocolMapper(mapping); });