From c9cc1896027c3e6a2d4e7855871c9a4d9914cbdc Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Fri, 12 Feb 2016 16:28:07 -0500 Subject: [PATCH 1/4] make RealmModel unmodifiable collections --- .../models/cache/infinispan/RealmAdapter.java | 72 ++++------ .../org/keycloak/models/jpa/RealmAdapter.java | 136 +++++++++++------- .../mongo/keycloak/adapters/RealmAdapter.java | 115 +++++++++------ .../models/cache/entities/CachedRealm.java | 68 ++++++--- .../models/utils/ModelToRepresentation.java | 3 + .../testsuite/forms/ResetPasswordTest.java | 14 +- 6 files changed, 250 insertions(+), 158 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index 14e2850752..6a61851683 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -502,11 +502,8 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredCredentials() { - - List copy = new LinkedList(); - if (updated != null) copy.addAll(updated.getRequiredCredentials()); - else copy.addAll(cached.getRequiredCredentials()); - return copy; + if (updated != null) return updated.getRequiredCredentials(); + return cached.getRequiredCredentials(); } @Override @@ -548,11 +545,13 @@ public class RealmAdapter implements RealmModel { @Override public List getDefaultGroups() { + if (updated != null) return updated.getDefaultGroups(); + List defaultGroups = new LinkedList<>(); for (String id : cached.getDefaultGroups()) { defaultGroups.add(cacheSession.getGroupById(id, this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @@ -599,13 +598,13 @@ public class RealmAdapter implements RealmModel { } map.put(model.getClientId(), model); } - return map; + return Collections.unmodifiableMap(map); } @Override public List getClients() { if (updated != null) return updated.getClients(); - List apps = new LinkedList(); + List apps = new LinkedList<>(); for (String id : cached.getClients().values()) { ClientModel model = cacheSession.getClientById(id, this); if (model == null) { @@ -613,7 +612,7 @@ public class RealmAdapter implements RealmModel { } apps.add(model); } - return apps; + return Collections.unmodifiableList(apps); } @@ -691,6 +690,7 @@ public class RealmAdapter implements RealmModel { @Override public IdentityProviderModel getIdentityProviderByAlias(String alias) { + if (updated != null) return updated.getIdentityProviderByAlias(alias); for (IdentityProviderModel identityProviderModel : getIdentityProviders()) { if (identityProviderModel.getAlias().equals(alias)) { return identityProviderModel; @@ -941,7 +941,7 @@ public class RealmAdapter implements RealmModel { if (roleById == null) continue; roles.add(roleById); } - return roles; + return Collections.unmodifiableSet(roles); } @Override @@ -1003,13 +1003,7 @@ public class RealmAdapter implements RealmModel { @Override public Set getIdentityProviderMappers() { if (updated != null) return updated.getIdentityProviderMappers(); - Set mappings = new HashSet<>(); - for (List models : cached.getIdentityProviderMappers().values()) { - for (IdentityProviderMapperModel model : models) { - mappings.add(model); - } - } - return mappings; + return cached.getIdentityProviderMapperSet(); } @Override @@ -1020,7 +1014,7 @@ public class RealmAdapter implements RealmModel { for (IdentityProviderMapperModel entity : list) { mappings.add(entity); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1066,13 +1060,7 @@ public class RealmAdapter implements RealmModel { @Override public Set getUserFederationMappers() { if (updated != null) return updated.getUserFederationMappers(); - Set mappers = new HashSet(); - for (List models : cached.getUserFederationMappers().values()) { - for (UserFederationMapperModel model : models) { - mappers.add(model); - } - } - return mappers; + return cached.getUserFederationMapperSet(); } @Override @@ -1083,7 +1071,7 @@ public class RealmAdapter implements RealmModel { for (UserFederationMapperModel entity : list) { mappers.add(entity); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -1192,9 +1180,7 @@ public class RealmAdapter implements RealmModel { @Override public List getAuthenticationFlows() { if (updated != null) return updated.getAuthenticationFlows(); - List models = new ArrayList<>(); - models.addAll(cached.getAuthenticationFlows().values()); - return models; + return cached.getAuthenticationFlowList(); } @Override @@ -1281,7 +1267,7 @@ public class RealmAdapter implements RealmModel { if (updated != null) return updated.getAuthenticatorConfigs(); List models = new ArrayList<>(); models.addAll(cached.getAuthenticatorConfigs().values()); - return models; + return Collections.unmodifiableList(models); } @Override @@ -1313,9 +1299,7 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredActionProviders() { if (updated != null) return updated.getRequiredActionProviders(); - List models = new ArrayList<>(); - models.addAll(cached.getRequiredActionProviders().values()); - return models; + return cached.getRequiredActionProviderList(); } @Override @@ -1366,20 +1350,20 @@ public class RealmAdapter implements RealmModel { if (group == null) continue; list.add(group); } - return list; + return Collections.unmodifiableList(list); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParent() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -1416,15 +1400,17 @@ public class RealmAdapter implements RealmModel { @Override public List getClientTemplates() { if (updated != null) return updated.getClientTemplates(); + List clientTemplates = cached.getClientTemplates(); + if (clientTemplates.isEmpty()) return Collections.EMPTY_LIST; List apps = new LinkedList(); - for (String id : cached.getClientTemplates()) { + for (String id : clientTemplates) { ClientTemplateModel model = cacheSession.getClientTemplateById(id, this); if (model == null) { throw new IllegalStateException("Cached clientemplate not found: " + id); } apps.add(model); } - return apps; + return Collections.unmodifiableList(apps); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index da66a3ba38..d99dedc28f 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -608,9 +608,9 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredCredentials() { - List requiredCredentialModels = new ArrayList(); Collection entities = realm.getRequiredCredentials(); - if (entities == null) return requiredCredentialModels; + if (entities == null) return Collections.EMPTY_LIST; + List requiredCredentialModels = new LinkedList<>(); for (RequiredCredentialEntity entity : entities) { RequiredCredentialModel model = new RequiredCredentialModel(); model.setFormLabel(entity.getFormLabel()); @@ -619,19 +619,19 @@ public class RealmAdapter implements RealmModel { model.setInput(entity.isInput()); requiredCredentialModels.add(model); } - return requiredCredentialModels; //To change body of implemented methods use File | Settings | File Templates. + return Collections.unmodifiableList(requiredCredentialModels); } @Override public List getDefaultRoles() { Collection entities = realm.getDefaultRoles(); - List roles = new ArrayList(); - if (entities == null) return roles; + if (entities == null || entities.isEmpty()) return Collections.emptyList(); + List roles = new LinkedList<>(); for (RoleEntity entity : entities) { roles.add(entity.getName()); } - return roles; + return Collections.unmodifiableList(roles); } @Override @@ -685,11 +685,12 @@ public class RealmAdapter implements RealmModel { @Override public List getDefaultGroups() { Collection entities = realm.getDefaultGroups(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List defaultGroups = new LinkedList<>(); for (GroupEntity entity : entities) { defaultGroups.add(session.realms().getGroupById(entity.getId(), this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @Override @@ -722,23 +723,26 @@ public class RealmAdapter implements RealmModel { @Override public Map getClientNameMap() { + List clients = getClients(); + if (clients.isEmpty()) return Collections.EMPTY_MAP; Map map = new HashMap(); - for (ClientModel app : getClients()) { + for (ClientModel app : clients) { map.put(app.getClientId(), app); } - return map; //To change body of implemented methods use File | Settings | File Templates. + return Collections.unmodifiableMap(map); } @Override public List getClients() { - List list = new LinkedList<>(); TypedQuery query = em.createNamedQuery("getClientsByRealm", ClientEntity.class); query.setParameter("realm", realm); List clients = query.getResultList(); + if (clients.isEmpty()) return Collections.EMPTY_LIST; + List list = new LinkedList<>(); for (ClientEntity entity : clients) { list.add(new ClientAdapter(this, em, session, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override @@ -794,13 +798,14 @@ public class RealmAdapter implements RealmModel { @Override public Map getBrowserSecurityHeaders() { Map attributes = getAttributes(); + if (attributes.isEmpty()) return Collections.EMPTY_MAP; Map headers = new HashMap(); for (Map.Entry entry : attributes.entrySet()) { if (entry.getKey().startsWith(BROWSER_HEADER_PREFIX)) { headers.put(entry.getKey().substring(BROWSER_HEADER_PREFIX.length()), entry.getValue()); } } - return headers; + return Collections.unmodifiableMap(headers); } @Override @@ -812,7 +817,9 @@ public class RealmAdapter implements RealmModel { @Override public Map getSmtpConfig() { - return realm.getSmtpConfig(); + Map config = new HashMap(); + config.putAll(realm.getSmtpConfig()); + return Collections.unmodifiableMap(config); } @Override @@ -824,6 +831,7 @@ public class RealmAdapter implements RealmModel { @Override public List getUserFederationProviders() { List entities = realm.getUserFederationProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List copy = new ArrayList(); for (UserFederationProviderEntity entity : entities) { copy.add(entity); @@ -843,7 +851,7 @@ public class RealmAdapter implements RealmModel { entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -1062,13 +1070,13 @@ public class RealmAdapter implements RealmModel { @Override public Set getRoles() { - Set list = new HashSet(); Collection roles = realm.getRoles(); - if (roles == null) return list; + if (roles == null) return Collections.EMPTY_SET; + Set list = new HashSet(); for (RoleEntity entity : roles) { list.add(new RoleAdapter(this, em, entity)); } - return list; + return Collections.unmodifiableSet(list); } @Override @@ -1206,7 +1214,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getEventsListeners() { - return realm.getEventsListeners(); + Set eventsListeners = realm.getEventsListeners(); + if (eventsListeners.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(eventsListeners); + return Collections.unmodifiableSet(copy); } @Override @@ -1217,7 +1229,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getEnabledEventTypes() { - return realm.getEnabledEventTypes(); + Set enabledEventTypes = realm.getEnabledEventTypes(); + if (enabledEventTypes.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(enabledEventTypes); + return Collections.unmodifiableSet(copy); } @Override @@ -1268,15 +1284,20 @@ public class RealmAdapter implements RealmModel { @Override public List getIdentityProviders() { + List entities = realm.getIdentityProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List identityProviders = new ArrayList(); - for (IdentityProviderEntity entity: realm.getIdentityProviders()) { + for (IdentityProviderEntity entity: entities) { IdentityProviderModel identityProviderModel = new IdentityProviderModel(); identityProviderModel.setProviderId(entity.getProviderId()); identityProviderModel.setAlias(entity.getAlias()); identityProviderModel.setInternalId(entity.getInternalId()); - identityProviderModel.setConfig(entity.getConfig()); + Map config = entity.getConfig(); + Map copy = new HashMap<>(); + copy.putAll(config); + identityProviderModel.setConfig(copy); identityProviderModel.setEnabled(entity.isEnabled()); identityProviderModel.setTrustEmail(entity.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(entity.isAuthenticateByDefault()); @@ -1288,7 +1309,7 @@ public class RealmAdapter implements RealmModel { identityProviders.add(identityProviderModel); } - return identityProviders; + return Collections.unmodifiableList(identityProviders); } @Override @@ -1373,7 +1394,11 @@ public class RealmAdapter implements RealmModel { @Override public Set getSupportedLocales() { - return realm.getSupportedLocales(); + Set supportedLocales = realm.getSupportedLocales(); + if (supportedLocales == null || supportedLocales.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(supportedLocales); + return Collections.unmodifiableSet(copy); } @Override @@ -1395,12 +1420,14 @@ public class RealmAdapter implements RealmModel { @Override public Set getIdentityProviderMappers() { + Collection entities = this.realm.getIdentityProviderMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappings = new HashSet(); - for (IdentityProviderMapperEntity entity : this.realm.getIdentityProviderMappers()) { + for (IdentityProviderMapperEntity entity : entities) { IdentityProviderMapperModel mapping = entityToModel(entity); mappings.add(mapping); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1508,23 +1535,26 @@ public class RealmAdapter implements RealmModel { @Override public Set getUserFederationMappers() { - Set mappers = new HashSet(); - for (UserFederationMapperEntity entity : this.realm.getUserFederationMappers()) { + Collection entities = this.realm.getUserFederationMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; + Set mappers = new HashSet<>(); + for (UserFederationMapperEntity entity : entities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override public Set getUserFederationMappersByFederationProvider(String federationProviderId) { - Set mappers = new HashSet(); Set mapperEntities = getUserFederationMapperEntitiesByFederationProvider(federationProviderId); + if (mapperEntities.isEmpty()) return Collections.EMPTY_SET; + Set mappers = new HashSet(); for (UserFederationMapperEntity entity : mapperEntities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -1693,13 +1723,13 @@ public class RealmAdapter implements RealmModel { TypedQuery query = em.createNamedQuery("getAuthenticationFlowsByRealm", AuthenticationFlowEntity.class); query.setParameter("realm", realm); List flows = query.getResultList(); - if (flows.size() == 0) return Collections.EMPTY_LIST; + if (flows.isEmpty()) return Collections.EMPTY_LIST; List models = new LinkedList<>(); for (AuthenticationFlowEntity entity : flows) { AuthenticationFlowModel model = entityToModel(entity); models.add(model); } - return models; + return Collections.unmodifiableList(models); } @Override @@ -1785,13 +1815,14 @@ public class RealmAdapter implements RealmModel { query.setParameter("realm", realm); query.setParameter("parentFlow", flow); List queryResult = query.getResultList(); + if (queryResult.isEmpty()) return Collections.EMPTY_LIST; List executions = new LinkedList<>(); for (AuthenticationExecutionEntity entity : queryResult) { AuthenticationExecutionModel model = entityToModel(entity); executions.add(model); } Collections.sort(executions, AuthenticationExecutionModel.ExecutionComparator.SINGLETON); - return executions; + return Collections.unmodifiableList(executions); } public AuthenticationExecutionModel entityToModel(AuthenticationExecutionEntity entity) { @@ -1916,11 +1947,13 @@ public class RealmAdapter implements RealmModel { @Override public List getAuthenticatorConfigs() { + Collection entities = realm.getAuthenticatorConfigs(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List authenticators = new LinkedList<>(); - for (AuthenticatorConfigEntity entity : realm.getAuthenticatorConfigs()) { + for (AuthenticatorConfigEntity entity : entities) { authenticators.add(entityToModel(entity)); } - return authenticators; + return Collections.unmodifiableList(authenticators); } @Override @@ -1993,11 +2026,13 @@ public class RealmAdapter implements RealmModel { @Override public List getRequiredActionProviders() { + Collection entities = realm.getRequiredActionProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List actions = new LinkedList<>(); - for (RequiredActionProviderEntity entity : realm.getRequiredActionProviders()) { + for (RequiredActionProviderEntity entity : entities) { actions.add(entityToModel(entity)); } - return actions; + return Collections.unmodifiableList(actions); } @Override @@ -2028,26 +2063,26 @@ public class RealmAdapter implements RealmModel { @Override public List getGroups() { + List groups = em.createNamedQuery("getAllGroupsByRealm").setParameter("realm", realm).getResultList(); + if (groups == null) return Collections.EMPTY_LIST; List list = new LinkedList<>(); - Collection groups = em.createNamedQuery("getAllGroupsByRealm").setParameter("realm", realm).getResultList(); - if (groups == null) return list; for (GroupEntity entity : groups) { list.add(new GroupAdapter(this, em, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParent() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -2100,12 +2135,13 @@ public class RealmAdapter implements RealmModel { @Override public List getClientTemplates() { + Collection entities = realm.getClientTemplates(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List list = new LinkedList<>(); - if (realm.getClientTemplates() == null) return list; - for (ClientTemplateEntity entity : realm.getClientTemplates()) { + for (ClientTemplateEntity entity : entities) { list.add(new ClientTemplateAdapter(this, em, session, entity)); } - return list; + return Collections.unmodifiableList(list); } @Override diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index 701cced0ea..9ceec417a6 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -645,14 +645,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .get(); List roles = getMongoStore().loadEntities(MongoRoleEntity.class, query, invocationContext); - Set result = new HashSet(); - if (roles == null) return result; + if (roles == null) return Collections.EMPTY_SET; + Set result = new HashSet(); for (MongoRoleEntity role : roles) { result.add(new RoleAdapter(session, this, role, this, invocationContext)); } - return result; + return Collections.unmodifiableSet(result); } @Override @@ -709,6 +709,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .and("realmId").is(getId()) .get(); List groups = getMongoStore().loadEntities(MongoGroupEntity.class, query, invocationContext); + if (groups == null) return Collections.EMPTY_LIST; List result = new LinkedList<>(); @@ -717,20 +718,20 @@ public class RealmAdapter extends AbstractMongoAdapter impleme result.add(model.getGroupById(group.getId(), this)); } - return result; + return Collections.unmodifiableList(result); } @Override public List getTopLevelGroups() { - List all = getGroups(); - Iterator it = all.iterator(); - while (it.hasNext()) { - GroupModel group = it.next(); - if (group.getParentId() != null) { - it.remove(); + List base = getGroups(); + if (base.isEmpty()) return base; + List copy = new LinkedList<>(); + for (GroupModel group : base) { + if (group.getParent() == null) { + copy.add(group); } } - return all; + return Collections.unmodifiableList(copy); } @Override @@ -750,7 +751,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getDefaultRoles() { - return realm.getDefaultRoles(); + return Collections.unmodifiableList(realm.getDefaultRoles()); } @Override @@ -781,11 +782,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getDefaultGroups() { + List entities = realm.getDefaultGroups(); + if (entities == null || entities.isEmpty()) return Collections.EMPTY_LIST; List defaultGroups = new LinkedList<>(); - for (String id : realm.getDefaultGroups()) { + for (String id : entities) { defaultGroups.add(session.realms().getGroupById(id, this)); } - return defaultGroups; + return Collections.unmodifiableList(defaultGroups); } @Override @@ -812,11 +815,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getClientNameMap() { + List clients = getClients(); + if (clients.isEmpty()) return Collections.EMPTY_MAP; Map resourceMap = new HashMap(); - for (ClientModel resource : getClients()) { + for (ClientModel resource : clients) { resourceMap.put(resource.getClientId(), resource); } - return resourceMap; + return Collections.unmodifiableMap(resourceMap); } @Override @@ -826,11 +831,12 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .get(); List clientEntities = getMongoStore().loadEntities(MongoClientEntity.class, query, invocationContext); + if (clientEntities.isEmpty()) return Collections.EMPTY_LIST; List result = new ArrayList(); for (MongoClientEntity clientEntity : clientEntities) { result.add(new ClientAdapter(session, this, clientEntity, invocationContext)); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -922,8 +928,8 @@ public class RealmAdapter extends AbstractMongoAdapter impleme } protected List convertRequiredCredentialEntities(Collection credEntities) { - - List result = new ArrayList(); + if (credEntities == null || credEntities.isEmpty()) return Collections.EMPTY_LIST; + List result = new LinkedList<>(); for (RequiredCredentialEntity entity : credEntities) { RequiredCredentialModel model = new RequiredCredentialModel(); model.setFormLabel(entity.getFormLabel()); @@ -933,7 +939,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme result.add(model); } - return result; + return Collections.unmodifiableList(result); } protected void updateRealm() { @@ -950,7 +956,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getBrowserSecurityHeaders() { - return realm.getBrowserSecurityHeaders(); + return Collections.unmodifiableMap(realm.getBrowserSecurityHeaders()); } @Override @@ -961,7 +967,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Map getSmtpConfig() { - return realm.getSmtpConfig(); + return Collections.unmodifiableMap(realm.getSmtpConfig()); } @Override @@ -973,15 +979,20 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getIdentityProviders() { + List entities = realm.getIdentityProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List identityProviders = new ArrayList(); - for (IdentityProviderEntity entity: realm.getIdentityProviders()) { + for (IdentityProviderEntity entity: entities) { IdentityProviderModel identityProviderModel = new IdentityProviderModel(); identityProviderModel.setProviderId(entity.getProviderId()); identityProviderModel.setAlias(entity.getAlias()); identityProviderModel.setInternalId(entity.getInternalId()); - identityProviderModel.setConfig(entity.getConfig()); + Map config = entity.getConfig(); + Map copy = new HashMap<>(); + copy.putAll(config); + identityProviderModel.setConfig(copy); identityProviderModel.setEnabled(entity.isEnabled()); identityProviderModel.setTrustEmail(entity.isTrustEmail()); identityProviderModel.setAuthenticateByDefault(entity.isAuthenticateByDefault()); @@ -993,7 +1004,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme identityProviders.add(identityProviderModel); } - return identityProviders; + return Collections.unmodifiableList(identityProviders); } @Override @@ -1132,6 +1143,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getUserFederationProviders() { List entities = realm.getUserFederationProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List copy = new LinkedList(); for (UserFederationProviderEntity entity : entities) { copy.add(entity); @@ -1151,7 +1163,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme entity.getFullSyncPeriod(), entity.getChangedSyncPeriod(), entity.getLastSync())); } - return result; + return Collections.unmodifiableList(result); } @Override @@ -1257,7 +1269,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getEventsListeners() { - return new HashSet(realm.getEventsListeners()); + List eventsListeners = realm.getEventsListeners(); + if (eventsListeners.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(eventsListeners); + return Collections.unmodifiableSet(copy); } @Override @@ -1272,7 +1288,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getEnabledEventTypes() { - return new HashSet(realm.getEnabledEventTypes()); + List enabledEventTypes = realm.getEnabledEventTypes(); + if (enabledEventTypes.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(enabledEventTypes); + return Collections.unmodifiableSet(copy); } @Override @@ -1364,7 +1384,11 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getSupportedLocales() { - return new HashSet(realm.getSupportedLocales()); + List supportedLocales = realm.getSupportedLocales(); + if (supportedLocales == null || supportedLocales.isEmpty()) return Collections.EMPTY_SET; + Set copy = new HashSet<>(); + copy.addAll(supportedLocales); + return Collections.unmodifiableSet(copy); } @Override @@ -1390,12 +1414,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getIdentityProviderMappers() { + List entities = getMongoEntity().getIdentityProviderMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappings = new HashSet(); - for (IdentityProviderMapperEntity entity : getMongoEntity().getIdentityProviderMappers()) { + for (IdentityProviderMapperEntity entity : entities) { IdentityProviderMapperModel mapping = entityToModel(entity); mappings.add(mapping); } - return mappings; + return Collections.unmodifiableSet(mappings); } @Override @@ -1568,12 +1594,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getAuthenticationFlows() { List flows = getMongoEntity().getAuthenticationFlows(); + if (flows.isEmpty()) return Collections.EMPTY_LIST; List models = new LinkedList<>(); for (AuthenticationFlowEntity entity : flows) { AuthenticationFlowModel model = entityToModel(entity); models.add(model); } - return models; + return Collections.unmodifiableList(models); } @Override @@ -1662,7 +1689,7 @@ public class RealmAdapter extends AbstractMongoAdapter impleme executions.add(model); } Collections.sort(executions, AuthenticationExecutionModel.ExecutionComparator.SINGLETON); - return executions; + return Collections.unmodifiableList(executions); } public AuthenticationExecutionModel entityToModel(AuthenticationExecutionEntity entity) { @@ -1752,11 +1779,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getAuthenticatorConfigs() { + List entities = getMongoEntity().getAuthenticatorConfigs(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List authenticators = new LinkedList<>(); - for (AuthenticatorConfigEntity entity : getMongoEntity().getAuthenticatorConfigs()) { + for (AuthenticatorConfigEntity entity : entities) { authenticators.add(entityToModel(entity)); } - return authenticators; + return Collections.unmodifiableList(authenticators); } @Override @@ -1898,11 +1927,13 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public List getRequiredActionProviders() { + List entities = realm.getRequiredActionProviders(); + if (entities.isEmpty()) return Collections.EMPTY_LIST; List actions = new LinkedList<>(); - for (RequiredActionProviderEntity entity : realm.getRequiredActionProviders()) { + for (RequiredActionProviderEntity entity : entities) { actions.add(entityToModel(entity)); } - return actions; + return Collections.unmodifiableList(actions); } public RequiredActionProviderEntity getRequiredActionProviderEntity(String id) { @@ -1930,12 +1961,14 @@ public class RealmAdapter extends AbstractMongoAdapter impleme @Override public Set getUserFederationMappers() { + List entities = getMongoEntity().getUserFederationMappers(); + if (entities.isEmpty()) return Collections.EMPTY_SET; Set mappers = new HashSet(); - for (UserFederationMapperEntity entity : getMongoEntity().getUserFederationMappers()) { + for (UserFederationMapperEntity entity : entities) { UserFederationMapperModel mapper = entityToModel(entity); mappers.add(mapper); } - return mappers; + return Collections.unmodifiableSet(mappers); } @Override @@ -2053,12 +2086,12 @@ public class RealmAdapter extends AbstractMongoAdapter impleme .and("realmId").is(getId()) .get(); List clientEntities = getMongoStore().loadEntities(MongoClientTemplateEntity.class, query, invocationContext); - + if (clientEntities.isEmpty()) return Collections.EMPTY_LIST; List result = new LinkedList<>(); for (MongoClientTemplateEntity clientEntity : clientEntities) { result.add(new ClientTemplateAdapter(session, this, clientEntity, invocationContext)); } - return result; + return Collections.unmodifiableList(result); } @Override diff --git a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java index 29b93318bd..a34cd9d29a 100755 --- a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java +++ b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java @@ -44,6 +44,7 @@ import java.security.PublicKey; import java.security.cert.Certificate; import java.security.cert.X509Certificate; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; @@ -107,16 +108,19 @@ public class CachedRealm implements Serializable { protected String emailTheme; protected String masterAdminClient; - protected List requiredCredentials = new ArrayList(); - protected List userFederationProviders = new ArrayList(); + protected List requiredCredentials; + protected List userFederationProviders; protected MultivaluedHashMap userFederationMappers = new MultivaluedHashMap(); - protected List identityProviders = new ArrayList(); + protected Set userFederationMapperSet; + protected List identityProviders; - protected Map browserSecurityHeaders = new HashMap(); - protected Map smtpConfig = new HashMap(); + protected Map browserSecurityHeaders; + protected Map smtpConfig; protected Map authenticationFlows = new HashMap<>(); + protected List authenticationFlowList; protected Map authenticatorConfigs = new HashMap<>(); protected Map requiredActionProviders = new HashMap<>(); + protected List requiredActionProviderList; protected Map requiredActionProvidersByAlias = new HashMap<>(); protected MultivaluedHashMap authenticationExecutions = new MultivaluedHashMap<>(); protected Map executionsById = new HashMap<>(); @@ -129,21 +133,27 @@ public class CachedRealm implements Serializable { protected boolean eventsEnabled; protected long eventsExpiration; - protected Set eventsListeners = new HashSet(); - protected Set enabledEventTypes = new HashSet(); + protected Set eventsListeners; + protected Set enabledEventTypes; protected boolean adminEventsEnabled; protected Set adminEnabledEventOperations = new HashSet(); protected boolean adminEventsDetailsEnabled; - protected List defaultRoles = new LinkedList(); + protected List defaultRoles; + + public Set getIdentityProviderMapperSet() { + return identityProviderMapperSet; + } + protected List defaultGroups = new LinkedList(); protected Set groups = new HashSet(); protected Map realmRoles = new HashMap(); protected Map clients = new HashMap(); protected List clientTemplates= new LinkedList<>(); protected boolean internationalizationEnabled; - protected Set supportedLocales = new HashSet(); + protected Set supportedLocales; protected String defaultLocale; protected MultivaluedHashMap identityProviderMappers = new MultivaluedHashMap<>(); + protected Set identityProviderMapperSet; public CachedRealm() { } @@ -200,8 +210,9 @@ public class CachedRealm implements Serializable { requiredCredentials = model.getRequiredCredentials(); userFederationProviders = model.getUserFederationProviders(); - for (UserFederationMapperModel mapper : model.getUserFederationMappers()) { - userFederationMappers.add(mapper.getFederationProviderId(), mapper); + userFederationMapperSet = model.getUserFederationMappers(); + for (UserFederationMapperModel mapper : userFederationMapperSet) { + this.userFederationMappers.add(mapper.getFederationProviderId(), mapper); } this.identityProviders = new ArrayList<>(); @@ -209,6 +220,7 @@ public class CachedRealm implements Serializable { for (IdentityProviderModel identityProviderModel : model.getIdentityProviders()) { this.identityProviders.add(new IdentityProviderModel(identityProviderModel)); } + this.identityProviders = Collections.unmodifiableList(this.identityProviders); for (IdentityProviderMapperModel mapper : model.getIdentityProviderMappers()) { identityProviderMappers.add(mapper.getIdentityProviderAlias(), mapper); @@ -216,18 +228,18 @@ public class CachedRealm implements Serializable { - smtpConfig.putAll(model.getSmtpConfig()); - browserSecurityHeaders.putAll(model.getBrowserSecurityHeaders()); + smtpConfig = model.getSmtpConfig(); + browserSecurityHeaders = model.getBrowserSecurityHeaders(); eventsEnabled = model.isEventsEnabled(); eventsExpiration = model.getEventsExpiration(); - eventsListeners.addAll(model.getEventsListeners()); - enabledEventTypes.addAll(model.getEnabledEventTypes()); + eventsListeners = model.getEventsListeners(); + enabledEventTypes = model.getEnabledEventTypes(); adminEventsEnabled = model.isAdminEventsEnabled(); adminEventsDetailsEnabled = model.isAdminEventsDetailsEnabled(); - defaultRoles.addAll(model.getDefaultRoles()); + defaultRoles = model.getDefaultRoles(); ClientModel masterAdminClient = model.getMasterAdminClient(); this.masterAdminClient = (masterAdminClient != null) ? masterAdminClient.getId() : null; @@ -238,10 +250,11 @@ public class CachedRealm implements Serializable { cacheClientTemplates(cache, delegate, model); internationalizationEnabled = model.isInternationalizationEnabled(); - supportedLocales.addAll(model.getSupportedLocales()); + supportedLocales = model.getSupportedLocales(); defaultLocale = model.getDefaultLocale(); - for (AuthenticationFlowModel flow : model.getAuthenticationFlows()) { - authenticationFlows.put(flow.getId(), flow); + authenticationFlowList = model.getAuthenticationFlows(); + for (AuthenticationFlowModel flow : authenticationFlowList) { + this.authenticationFlows.put(flow.getId(), flow); authenticationExecutions.put(flow.getId(), new LinkedList()); for (AuthenticationExecutionModel execution : model.getAuthenticationExecutions(flow.getId())) { authenticationExecutions.add(flow.getId(), execution); @@ -254,8 +267,9 @@ public class CachedRealm implements Serializable { for (AuthenticatorConfigModel authenticator : model.getAuthenticatorConfigs()) { authenticatorConfigs.put(authenticator.getId(), authenticator); } - for (RequiredActionProviderModel action : model.getRequiredActionProviders()) { - requiredActionProviders.put(action.getId(), action); + requiredActionProviderList = model.getRequiredActionProviders(); + for (RequiredActionProviderModel action : requiredActionProviderList) { + this.requiredActionProviders.put(action.getId(), action); requiredActionProvidersByAlias.put(action.getAlias(), action); } @@ -606,4 +620,16 @@ public class CachedRealm implements Serializable { public X509Certificate getCertificate() { return certificate; } + + public Set getUserFederationMapperSet() { + return userFederationMapperSet; + } + + public List getAuthenticationFlowList() { + return authenticationFlowList; + } + + public List getRequiredActionProviderList() { + return requiredActionProviderList; + } } diff --git a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java index 8b59053102..bc8b0b6e7c 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/ModelToRepresentation.java @@ -400,6 +400,9 @@ public class ModelToRepresentation { rep.setRequiredActions(new LinkedList()); List requiredActionProviders = realm.getRequiredActionProviders(); + List copy = new LinkedList<>(); + copy.addAll(requiredActionProviders); + requiredActionProviders = copy; //ensure consistent ordering of requiredActionProviders. Collections.sort(requiredActionProviders, new Comparator() { @Override diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java index eea67920d9..921e682e24 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/forms/ResetPasswordTest.java @@ -46,6 +46,8 @@ import javax.mail.internet.MimeMessage; import java.io.IOException; import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.*; @@ -542,8 +544,11 @@ public class ResetPasswordTest { keycloakRule.configure(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - host[0] = appRealm.getSmtpConfig().get("host"); - appRealm.getSmtpConfig().put("host", "invalid_host"); + Map smtpConfig = new HashMap<>(); + smtpConfig.putAll(appRealm.getSmtpConfig()); + host[0] = smtpConfig.get("host"); + smtpConfig.put("host", "invalid_host"); + appRealm.setSmtpConfig(smtpConfig); } }); @@ -568,7 +573,10 @@ public class ResetPasswordTest { keycloakRule.configure(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getSmtpConfig().put("host",host[0]); + Map smtpConfig = new HashMap<>(); + smtpConfig.putAll(appRealm.getSmtpConfig()); + smtpConfig.put("host",host[0]); + appRealm.setSmtpConfig(smtpConfig); } }); } From 3143a2e5001ab15a4e1578cee3514862df96f084 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Wed, 17 Feb 2016 15:43:03 -0500 Subject: [PATCH 2/4] unmodifiable complete --- .../java/org/keycloak/models/cache/entities/CachedRealm.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java index a34cd9d29a..ad2ba6a397 100755 --- a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java +++ b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java @@ -222,7 +222,8 @@ public class CachedRealm implements Serializable { } this.identityProviders = Collections.unmodifiableList(this.identityProviders); - for (IdentityProviderMapperModel mapper : model.getIdentityProviderMappers()) { + this.identityProviderMapperSet = model.getIdentityProviderMappers(); + for (IdentityProviderMapperModel mapper : identityProviderMapperSet) { identityProviderMappers.add(mapper.getIdentityProviderAlias(), mapper); } From 20348e5d0b95be3431c0c4548099cc40e2b65cfd Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Wed, 17 Feb 2016 16:05:25 -0500 Subject: [PATCH 3/4] remove RealmModel.getClientNameMap() --- .../models/cache/infinispan/RealmAdapter.java | 14 ------------ .../org/keycloak/models/jpa/RealmAdapter.java | 11 ---------- .../mongo/keycloak/adapters/RealmAdapter.java | 11 ---------- .../migrators/MigrationTo1_2_0_CR1.java | 15 +++++-------- .../java/org/keycloak/models/RealmModel.java | 3 --- .../models/utils/RepresentationToModel.java | 22 +++++++++---------- .../exportimport/util/ImportUtils.java | 3 +-- .../partialimport/UsersPartialImport.java | 3 +-- .../services/managers/RealmManager.java | 4 ++-- .../services/resources/RealmsResource.java | 2 +- .../testsuite/account/AccountTest.java | 2 +- .../testsuite/model/ClientModelTest.java | 2 +- .../keycloak/testsuite/model/ImportTest.java | 10 ++++----- .../oauth/AuthorizationCodeTest.java | 4 ++-- .../testsuite/oauth/OAuthRedirectUriTest.java | 12 +++++----- 15 files changed, 36 insertions(+), 82 deletions(-) mode change 100644 => 100755 services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index 6a61851683..690d2fe429 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -587,20 +587,6 @@ public class RealmAdapter implements RealmModel { updated.updateDefaultRoles(defaultRoles); } - @Override - public Map getClientNameMap() { - if (updated != null) return updated.getClientNameMap(); - Map map = new HashMap(); - for (String id : cached.getClients().values()) { - ClientModel model = cacheSession.getClientById(id, this); - if (model == null) { - throw new IllegalStateException("Cached application not found: " + id); - } - map.put(model.getClientId(), model); - } - return Collections.unmodifiableMap(map); - } - @Override public List getClients() { if (updated != null) return updated.getClients(); diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java index d99dedc28f..65d3b1b854 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java @@ -721,17 +721,6 @@ public class RealmAdapter implements RealmModel { } - @Override - public Map getClientNameMap() { - List clients = getClients(); - if (clients.isEmpty()) return Collections.EMPTY_MAP; - Map map = new HashMap(); - for (ClientModel app : clients) { - map.put(app.getClientId(), app); - } - return Collections.unmodifiableMap(map); - } - @Override public List getClients() { TypedQuery query = em.createNamedQuery("getClientsByRealm", ClientEntity.class); diff --git a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java index 9ceec417a6..48978ffe12 100755 --- a/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java +++ b/model/mongo/src/main/java/org/keycloak/models/mongo/keycloak/adapters/RealmAdapter.java @@ -813,17 +813,6 @@ public class RealmAdapter extends AbstractMongoAdapter impleme return session.realms().getClientByClientId(clientId, this); } - @Override - public Map getClientNameMap() { - List clients = getClients(); - if (clients.isEmpty()) return Collections.EMPTY_MAP; - Map resourceMap = new HashMap(); - for (ClientModel resource : clients) { - resourceMap.put(resource.getClientId(), resource); - } - return Collections.unmodifiableMap(resourceMap); - } - @Override public List getClients() { DBObject query = new QueryBuilder() diff --git a/server-spi/src/main/java/org/keycloak/migration/migrators/MigrationTo1_2_0_CR1.java b/server-spi/src/main/java/org/keycloak/migration/migrators/MigrationTo1_2_0_CR1.java index be0a9f3fe3..f2a86ce466 100755 --- a/server-spi/src/main/java/org/keycloak/migration/migrators/MigrationTo1_2_0_CR1.java +++ b/server-spi/src/main/java/org/keycloak/migration/migrators/MigrationTo1_2_0_CR1.java @@ -36,7 +36,7 @@ public class MigrationTo1_2_0_CR1 { public static final ModelVersion VERSION = new ModelVersion("1.2.0.CR1"); public void setupBrokerService(RealmModel realm) { - ClientModel client = realm.getClientNameMap().get(Constants.BROKER_SERVICE_CLIENT_ID); + ClientModel client = realm.getClientByClientId(Constants.BROKER_SERVICE_CLIENT_ID); if (client == null) { client = KeycloakModelUtils.createClient(realm, Constants.BROKER_SERVICE_CLIENT_ID); client.setEnabled(true); @@ -52,16 +52,13 @@ public class MigrationTo1_2_0_CR1 { } private void setupClientNames(RealmModel realm) { - Map clients = realm.getClientNameMap(); - - setupClientName(clients, Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); - setupClientName(clients, Constants.ADMIN_CONSOLE_CLIENT_ID); - setupClientName(clients, Constants.REALM_MANAGEMENT_CLIENT_ID); + setupClientName(realm.getClientByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID)); + setupClientName(realm.getClientByClientId(Constants.ADMIN_CONSOLE_CLIENT_ID)); + setupClientName(realm.getClientByClientId(Constants.REALM_MANAGEMENT_CLIENT_ID)); } - private void setupClientName(Map clients, String clientId) { - ClientModel client = clients.get(clientId); - if (client != null && client.getName() == null) client.setName("${client_" + clientId + "}"); + private void setupClientName(ClientModel client) { + if (client != null && client.getName() == null) client.setName("${client_" + client.getClientId() + "}"); } public void migrate(KeycloakSession session) { diff --git a/server-spi/src/main/java/org/keycloak/models/RealmModel.java b/server-spi/src/main/java/org/keycloak/models/RealmModel.java index 87a929990c..bc7dec5960 100755 --- a/server-spi/src/main/java/org/keycloak/models/RealmModel.java +++ b/server-spi/src/main/java/org/keycloak/models/RealmModel.java @@ -193,9 +193,6 @@ public interface RealmModel extends RoleContainerModel { void removeDefaultGroup(GroupModel group); - // Key is clientId - Map getClientNameMap(); - List getClients(); ClientModel addClient(String name); diff --git a/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java b/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java index c9d89b462a..d349f4e02e 100755 --- a/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java +++ b/server-spi/src/main/java/org/keycloak/models/utils/RepresentationToModel.java @@ -234,12 +234,12 @@ public class RepresentationToModel { // Now that all possible roles and clients are created, create scope mappings - Map appMap = newRealm.getClientNameMap(); + //Map appMap = newRealm.getClientNameMap(); if (rep.getClientScopeMappings() != null) { for (Map.Entry> entry : rep.getClientScopeMappings().entrySet()) { - ClientModel app = appMap.get(entry.getKey()); + ClientModel app = newRealm.getClientByClientId(entry.getKey()); if (app == null) { throw new RuntimeException("Unable to find client role mappings for client: " + entry.getKey()); } @@ -320,7 +320,7 @@ public class RepresentationToModel { if (rep.getUsers() != null) { for (UserRepresentation userRep : rep.getUsers()) { - UserModel user = createUser(session, newRealm, userRep, appMap); + UserModel user = createUser(session, newRealm, userRep); } } @@ -383,14 +383,13 @@ public class RepresentationToModel { List groups = rep.getGroups(); if (groups == null) return; - Map clientMap = realm.getClientNameMap(); GroupModel parent = null; for (GroupRepresentation group : groups) { - importGroup(realm, clientMap, parent, group); + importGroup(realm, parent, group); } } - public static void importGroup(RealmModel realm, Map clientMap, GroupModel parent, GroupRepresentation group) { + public static void importGroup(RealmModel realm, GroupModel parent, GroupRepresentation group) { GroupModel newGroup = realm.createGroup(group.getId(), group.getName()); if (group.getAttributes() != null) { for (Map.Entry> attr : group.getAttributes().entrySet()) { @@ -410,7 +409,7 @@ public class RepresentationToModel { } if (group.getClientRoles() != null) { for (Map.Entry> entry : group.getClientRoles().entrySet()) { - ClientModel client = clientMap.get(entry.getKey()); + ClientModel client = realm.getClientByClientId(entry.getKey()); if (client == null) { throw new RuntimeException("Unable to find client role mappings for client: " + entry.getKey()); } @@ -427,7 +426,7 @@ public class RepresentationToModel { } if (group.getSubGroups() != null) { for (GroupRepresentation subGroup : group.getSubGroups()) { - importGroup(realm, clientMap, newGroup, subGroup); + importGroup(realm, newGroup, subGroup); } } } @@ -1180,7 +1179,7 @@ public class RepresentationToModel { // Users - public static UserModel createUser(KeycloakSession session, RealmModel newRealm, UserRepresentation userRep, Map clientMap) { + public static UserModel createUser(KeycloakSession session, RealmModel newRealm, UserRepresentation userRep) { convertDeprecatedSocialProviders(userRep); // Import users just to user storage. Don't federate @@ -1228,7 +1227,7 @@ public class RepresentationToModel { } if (userRep.getServiceAccountClientId() != null) { String clientId = userRep.getServiceAccountClientId(); - ClientModel client = clientMap.get(clientId); + ClientModel client = newRealm.getClientByClientId(clientId); if (client == null) { throw new RuntimeException("Unable to find client specified for service account link. Client: " + clientId); } @@ -1316,9 +1315,8 @@ public class RepresentationToModel { } } if (userRep.getClientRoles() != null) { - Map clientMap = realm.getClientNameMap(); for (Map.Entry> entry : userRep.getClientRoles().entrySet()) { - ClientModel client = clientMap.get(entry.getKey()); + ClientModel client = realm.getClientByClientId(entry.getKey()); if (client == null) { throw new RuntimeException("Unable to find client role mappings for client: " + entry.getKey()); } diff --git a/services/src/main/java/org/keycloak/exportimport/util/ImportUtils.java b/services/src/main/java/org/keycloak/exportimport/util/ImportUtils.java index 0e21f7aec6..06f6e54998 100755 --- a/services/src/main/java/org/keycloak/exportimport/util/ImportUtils.java +++ b/services/src/main/java/org/keycloak/exportimport/util/ImportUtils.java @@ -199,9 +199,8 @@ public class ImportUtils { private static void importUsers(KeycloakSession session, RealmProvider model, String realmName, List userReps) { RealmModel realm = model.getRealmByName(realmName); - Map apps = realm.getClientNameMap(); for (UserRepresentation user : userReps) { - RepresentationToModel.createUser(session, realm, user, apps); + RepresentationToModel.createUser(session, realm, user); } } diff --git a/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java b/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java old mode 100644 new mode 100755 index dbfc8406ac..2dc413626b --- a/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java +++ b/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java @@ -107,9 +107,8 @@ public class UsersPartialImport extends AbstractPartialImport apps = realm.getClientNameMap(); user.setId(KeycloakModelUtils.generateId()); - UserModel userModel = RepresentationToModel.createUser(session, realm, user, apps); + UserModel userModel = RepresentationToModel.createUser(session, realm, user); if (userModel == null) throw new RuntimeException("Unable to create user " + getName(user)); createdIds.put(getName(user), userModel.getId()); } diff --git a/services/src/main/java/org/keycloak/services/managers/RealmManager.java b/services/src/main/java/org/keycloak/services/managers/RealmManager.java index 8b6a732d0e..fd3a009824 100755 --- a/services/src/main/java/org/keycloak/services/managers/RealmManager.java +++ b/services/src/main/java/org/keycloak/services/managers/RealmManager.java @@ -327,7 +327,7 @@ public class RealmManager implements RealmImporter { private void setupAccountManagement(RealmModel realm) { - ClientModel client = realm.getClientNameMap().get(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); + ClientModel client = realm.getClientByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); if (client == null) { client = KeycloakModelUtils.createClient(realm, Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); client.setName("${client_" + Constants.ACCOUNT_MANAGEMENT_CLIENT_ID + "}"); @@ -352,7 +352,7 @@ public class RealmManager implements RealmImporter { } public void setupBrokerService(RealmModel realm) { - ClientModel client = realm.getClientNameMap().get(Constants.BROKER_SERVICE_CLIENT_ID); + ClientModel client = realm.getClientByClientId(Constants.BROKER_SERVICE_CLIENT_ID); if (client == null) { client = KeycloakModelUtils.createClient(realm, Constants.BROKER_SERVICE_CLIENT_ID); client.setEnabled(true); diff --git a/services/src/main/java/org/keycloak/services/resources/RealmsResource.java b/services/src/main/java/org/keycloak/services/resources/RealmsResource.java index 1d39612b7e..1ad6b7b6e3 100755 --- a/services/src/main/java/org/keycloak/services/resources/RealmsResource.java +++ b/services/src/main/java/org/keycloak/services/resources/RealmsResource.java @@ -139,7 +139,7 @@ public class RealmsResource { public AccountService getAccountService(final @PathParam("realm") String name) { RealmModel realm = init(name); - ClientModel client = realm.getClientNameMap().get(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); + ClientModel client = realm.getClientByClientId(Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); if (client == null || !client.isEnabled()) { logger.debug("account management not enabled"); throw new NotFoundException("account management not enabled"); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java index 8d9e85d836..7c5152bde4 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/account/AccountTest.java @@ -73,7 +73,7 @@ public class AccountTest { public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { UserModel user = manager.getSession().users().getUserByUsername("test-user@localhost", appRealm); - ClientModel accountApp = appRealm.getClientNameMap().get(org.keycloak.models.Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); + ClientModel accountApp = appRealm.getClientByClientId(org.keycloak.models.Constants.ACCOUNT_MANAGEMENT_CLIENT_ID); UserModel user2 = manager.getSession().users().addUser(appRealm, "test-user-no-access@localhost"); user2.setEnabled(true); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java index e66aa6087c..24161a2b08 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ClientModelTest.java @@ -75,7 +75,7 @@ public class ClientModelTest extends AbstractModelTest { public void persist() { RealmModel persisted = realmManager.getRealm(realm.getId()); - ClientModel actual = persisted.getClientNameMap().get("app-name"); + ClientModel actual = persisted.getClientByClientId("app-name"); assertEquals(client, actual); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java index a60877e99e..0f02666437 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ImportTest.java @@ -118,12 +118,12 @@ public class ImportTest extends AbstractModelTest { Assert.assertNotNull(application); Assert.assertNotNull(otherApp); Assert.assertNull(nonExisting); - Map clients = realm.getClientNameMap(); + List clients = realm.getClients(); Assert.assertEquals(8, clients.size()); - Assert.assertTrue(clients.values().contains(application)); - Assert.assertTrue(clients.values().contains(otherApp)); - Assert.assertTrue(clients.values().contains(accountApp)); - realm.getClients().containsAll(clients.values()); + Assert.assertTrue(clients.contains(application)); + Assert.assertTrue(clients.contains(otherApp)); + Assert.assertTrue(clients.contains(accountApp)); + realm.getClients().containsAll(clients); Assert.assertEquals("Applicationn", application.getName()); Assert.assertEquals(50, application.getNodeReRegistrationTimeout()); diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java index 2ceca15360..9a71c6497d 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/AuthorizationCodeTest.java @@ -91,7 +91,7 @@ public class AuthorizationCodeTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").addRedirectUri(Constants.INSTALLED_APP_URN); + appRealm.getClientByClientId("test-app").addRedirectUri(Constants.INSTALLED_APP_URN); } }); oauth.redirectUri(Constants.INSTALLED_APP_URN); @@ -110,7 +110,7 @@ public class AuthorizationCodeTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").removeRedirectUri(Constants.INSTALLED_APP_URN); + appRealm.getClientByClientId("test-app").removeRedirectUri(Constants.INSTALLED_APP_URN); } }); } diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java index 5b1274899a..a08ca371dc 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/oauth/OAuthRedirectUriTest.java @@ -113,7 +113,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").addRedirectUri("http://localhost:8081/app2"); + appRealm.getClientByClientId("test-app").addRedirectUri("http://localhost:8081/app2"); } }); @@ -127,7 +127,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app2"); + appRealm.getClientByClientId("test-app").removeRedirectUri("http://localhost:8081/app2"); } }); } @@ -138,7 +138,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app/*"); + appRealm.getClientByClientId("test-app").removeRedirectUri("http://localhost:8081/app/*"); } }); @@ -152,7 +152,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").addRedirectUri("http://localhost:8081/app/*"); + appRealm.getClientByClientId("test-app").addRedirectUri("http://localhost:8081/app/*"); } }); } @@ -163,7 +163,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").removeRedirectUri("http://localhost:8081/app/*"); + appRealm.getClientByClientId("test-app").removeRedirectUri("http://localhost:8081/app/*"); } }); @@ -177,7 +177,7 @@ public class OAuthRedirectUriTest { keycloakRule.update(new KeycloakRule.KeycloakSetup() { @Override public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel appRealm) { - appRealm.getClientNameMap().get("test-app").addRedirectUri("http://localhost:8081/app/*"); + appRealm.getClientByClientId("test-app").addRedirectUri("http://localhost:8081/app/*"); } }); } From acf2d662c2d7356933da092269aa9d6c4dd41213 Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Wed, 17 Feb 2016 16:09:25 -0500 Subject: [PATCH 4/4] remove RealmModel.getClientNameMap() --- .../infinispan/InfinispanCacheRealmProviderFactory.java | 2 +- .../org/keycloak/models/cache/infinispan/RealmAdapter.java | 2 +- .../locking/LockingCacheRealmProviderFactory.java | 2 +- .../infinispan/locking/entities/RevisionedCachedRealm.java | 2 +- .../org/keycloak/models/cache/entities/CachedRealm.java | 6 +++--- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheRealmProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheRealmProviderFactory.java index ac8f3734fd..34aa5ced85 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheRealmProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/InfinispanCacheRealmProviderFactory.java @@ -141,7 +141,7 @@ public class InfinispanCacheRealmProviderFactory implements CacheRealmProviderFa realmCache.evictRoleById(r); } - for (String c : realm.getClients().values()) { + for (String c : realm.getClients()) { realmCache.evictClientById(c); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java index 690d2fe429..2f3048ad31 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/RealmAdapter.java @@ -591,7 +591,7 @@ public class RealmAdapter implements RealmModel { public List getClients() { if (updated != null) return updated.getClients(); List apps = new LinkedList<>(); - for (String id : cached.getClients().values()) { + for (String id : cached.getClients()) { ClientModel model = cacheSession.getClientById(id, this); if (model == null) { throw new IllegalStateException("Cached application not found: " + id); diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/LockingCacheRealmProviderFactory.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/LockingCacheRealmProviderFactory.java index 23f8e53bba..d2bfbf4389 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/LockingCacheRealmProviderFactory.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/LockingCacheRealmProviderFactory.java @@ -138,7 +138,7 @@ public class LockingCacheRealmProviderFactory implements CacheRealmProviderFacto realmCache.evictRoleById(r); } - for (String c : realm.getClients().values()) { + for (String c : realm.getClients()) { realmCache.evictClientById(c); } diff --git a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/entities/RevisionedCachedRealm.java b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/entities/RevisionedCachedRealm.java index 2b3a524386..c4fd4870dd 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/entities/RevisionedCachedRealm.java +++ b/model/infinispan/src/main/java/org/keycloak/models/cache/infinispan/locking/entities/RevisionedCachedRealm.java @@ -42,7 +42,7 @@ public class RevisionedCachedRealm extends CachedRealm implements Revisioned { @Override protected void cacheClients(RealmCache cache, RealmProvider delegate, RealmModel model) { for (ClientModel client : model.getClients()) { - clients.put(client.getClientId(), client.getId()); + clients.add(client.getId()); } } diff --git a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java index ad2ba6a397..a037d0549b 100755 --- a/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java +++ b/server-spi/src/main/java/org/keycloak/models/cache/entities/CachedRealm.java @@ -147,7 +147,7 @@ public class CachedRealm implements Serializable { protected List defaultGroups = new LinkedList(); protected Set groups = new HashSet(); protected Map realmRoles = new HashMap(); - protected Map clients = new HashMap(); + protected List clients = new LinkedList<>(); protected List clientTemplates= new LinkedList<>(); protected boolean internationalizationEnabled; protected Set supportedLocales; @@ -296,7 +296,7 @@ public class CachedRealm implements Serializable { protected void cacheClients(RealmCache cache, RealmProvider delegate, RealmModel model) { for (ClientModel client : model.getClients()) { - clients.put(client.getClientId(), client.getId()); + clients.add(client.getId()); CachedClient cachedClient = new CachedClient(cache, delegate, model, client); cache.addClient(cachedClient); } @@ -339,7 +339,7 @@ public class CachedRealm implements Serializable { return realmRoles; } - public Map getClients() { + public List getClients() { return clients; }