diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java index 115d4dd839..adde0e436d 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientAdapter.java @@ -47,7 +47,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class ClientAdapter implements ClientModel { +public class ClientAdapter implements ClientModel, JpaModel { protected KeycloakSession session; protected RealmModel realm; diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientTemplateAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientTemplateAdapter.java index 826ed8448b..e38ab7361f 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/ClientTemplateAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/ClientTemplateAdapter.java @@ -24,6 +24,7 @@ import org.keycloak.models.ProtocolMapperModel; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleContainerModel; import org.keycloak.models.RoleModel; +import org.keycloak.models.jpa.entities.ClientEntity; import org.keycloak.models.jpa.entities.ClientTemplateEntity; import org.keycloak.models.jpa.entities.ProtocolMapperEntity; import org.keycloak.models.jpa.entities.RoleEntity; @@ -42,7 +43,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class ClientTemplateAdapter implements ClientTemplateModel { +public class ClientTemplateAdapter implements ClientTemplateModel , JpaModel { protected KeycloakSession session; protected RealmModel realm; diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java index a2d0fece4b..6ea87c5587 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/GroupAdapter.java @@ -41,7 +41,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class GroupAdapter implements GroupModel { +public class GroupAdapter implements GroupModel , JpaModel { protected GroupEntity group; protected EntityManager em; @@ -53,7 +53,7 @@ public class GroupAdapter implements GroupModel { this.realm = realm; } - public GroupEntity getGroup() { + public GroupEntity getEntity() { return group; } @@ -88,7 +88,7 @@ public class GroupAdapter implements GroupModel { public static GroupEntity toEntity(GroupModel model, EntityManager em) { if (model instanceof GroupAdapter) { - return ((GroupAdapter)model).getGroup(); + return ((GroupAdapter)model).getEntity(); } return em.getReference(GroupEntity.class, model.getId()); } @@ -233,7 +233,7 @@ public class GroupAdapter implements GroupModel { protected TypedQuery getGroupRoleMappingEntityTypedQuery(RoleModel role) { TypedQuery query = em.createNamedQuery("groupHasRole", GroupRoleMappingEntity.class); - query.setParameter("group", getGroup()); + query.setParameter("group", getEntity()); query.setParameter("roleId", role.getId()); return query; } @@ -242,7 +242,7 @@ public class GroupAdapter implements GroupModel { public void grantRole(RoleModel role) { if (hasRole(role)) return; GroupRoleMappingEntity entity = new GroupRoleMappingEntity(); - entity.setGroup(getGroup()); + entity.setGroup(getEntity()); entity.setRoleId(role.getId()); em.persist(entity); em.flush(); @@ -269,7 +269,7 @@ public class GroupAdapter implements GroupModel { // we query ids only as the role might be cached and following the @ManyToOne will result in a load // even if we're getting just the id. TypedQuery query = em.createNamedQuery("groupRoleMappingIds", String.class); - query.setParameter("group", getGroup()); + query.setParameter("group", getEntity()); List ids = query.getResultList(); Set roles = new HashSet(); for (String roleId : ids) { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaModel.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaModel.java new file mode 100755 index 0000000000..7cc915f325 --- /dev/null +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaModel.java @@ -0,0 +1,9 @@ +package org.keycloak.models.jpa; + +/** + * @author Bill Burke + * @version $Revision: 1 $ + */ +public interface JpaModel { + T getEntity(); +} diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java index 635521e157..2040b22ab5 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaRealmProvider.java @@ -39,9 +39,11 @@ import javax.persistence.EntityManager; import javax.persistence.TypedQuery; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -53,6 +55,14 @@ public class JpaRealmProvider implements RealmProvider { private final KeycloakSession session; protected EntityManager em; + // we have a local map of adapter classes for two reasons + // 1. So we don't have to allocate one again + // 2. So that we can do em.refresh() on the entity to make sure the state + // is up to date. If em.find is called a second time without a session, the same + // entity instance is returned. With the caching layer above it, it will cache stale + // entries. + protected Map adapters = new HashMap<>(); + public JpaRealmProvider(KeycloakSession session, EntityManager em) { this.session = session; @@ -76,21 +86,28 @@ public class JpaRealmProvider implements RealmProvider { realm.setId(id); em.persist(realm); em.flush(); - final RealmModel model = new RealmAdapter(session, em, realm); + final RealmModel adapter = new RealmAdapter(session, em, realm); session.getKeycloakSessionFactory().publish(new RealmModel.RealmCreationEvent() { @Override public RealmModel getCreatedRealm() { - return model; + return adapter; } }); - return model; + adapters.put(id, adapter); + return adapter; } @Override public RealmModel getRealm(String id) { + RealmAdapter adapter = (RealmAdapter)findJpaModel(id); + if (adapter != null) { + return adapter; + } RealmEntity realm = em.find(RealmEntity.class, id); if (realm == null) return null; - return new RealmAdapter(session, em, realm); + adapter = new RealmAdapter(session, em, realm); + adapters.put(id, adapter); + return adapter; } @Override @@ -124,8 +141,10 @@ public class JpaRealmProvider implements RealmProvider { if (realm == null) { return false; } + em.refresh(realm); RealmAdapter adapter = new RealmAdapter(session, em, realm); session.users().preRemove(adapter); + adapters.remove(id); int num = em.createNamedQuery("deleteGroupRoleMappingsByRealm") .setParameter("realm", realm).executeUpdate(); num = em.createNamedQuery("deleteGroupAttributesByRealm") @@ -174,7 +193,9 @@ public class JpaRealmProvider implements RealmProvider { entity.setRealmId(realm.getId()); em.persist(entity); em.flush(); - return new RoleAdapter(session, realm, em, entity); + RoleAdapter adapter = new RoleAdapter(session, realm, em, entity); + adapters.put(id, adapter); + return adapter; } @@ -203,7 +224,9 @@ public class JpaRealmProvider implements RealmProvider { roleEntity.setRealmId(realm.getId()); em.persist(roleEntity); em.flush(); - return new RoleAdapter(session, realm, em, roleEntity); + RoleAdapter adapter = new RoleAdapter(session, realm, em, roleEntity); + adapters.put(id, adapter); + return adapter; } @Override @@ -247,11 +270,12 @@ public class JpaRealmProvider implements RealmProvider { @Override public boolean removeRole(RealmModel realm, RoleModel role) { session.users().preRemove(realm, role); - RoleEntity roleEntity = em.getReference(RoleEntity.class, role.getId()); RoleContainerModel container = role.getContainer(); if (container.getDefaultRoles().contains(role.getName())) { container.removeDefaultRoles(role.getName()); } + adapters.remove(role.getId()); + RoleEntity roleEntity = em.getReference(RoleEntity.class, role.getId()); String compositeRoleTable = JpaUtils.getTableNameForNativeQuery("COMPOSITE_ROLE", em); em.createNativeQuery("delete from " + compositeRoleTable + " where CHILD_ROLE = :role").setParameter("role", roleEntity).executeUpdate(); em.createNamedQuery("deleteScopeMappingByRole").setParameter("role", roleEntity).executeUpdate(); @@ -260,25 +284,35 @@ public class JpaRealmProvider implements RealmProvider { em.remove(roleEntity); em.flush(); - return true; } @Override public RoleModel getRoleById(String id, RealmModel realm) { + RoleAdapter adapter = (RoleAdapter)findJpaModel(id); + if (adapter != null) { + if (!realm.getId().equals(adapter.getEntity().getRealmId())) return null; + return adapter; + } RoleEntity entity = em.find(RoleEntity.class, id); if (entity == null) return null; if (!realm.getId().equals(entity.getRealmId())) return null; - return new RoleAdapter(session, realm, em, entity); + adapter = new RoleAdapter(session, realm, em, entity); + adapters.put(id, adapter); + return adapter; } @Override public GroupModel getGroupById(String id, RealmModel realm) { + GroupAdapter adapter = (GroupAdapter)findJpaModel(id); + if (adapter != null) return adapter; GroupEntity groupEntity = em.find(GroupEntity.class, id); if (groupEntity == null) return null; if (!groupEntity.getRealm().getId().equals(realm.getId())) return null; - return new GroupAdapter(realm, em, groupEntity); + adapter = new GroupAdapter(realm, em, groupEntity); + adapters.put(id, adapter); + return adapter; } @Override @@ -326,6 +360,7 @@ public class JpaRealmProvider implements RealmProvider { } session.users().preRemove(realm, group); + adapters.remove(group.getId()); realm.removeDefaultGroup(group); for (GroupModel subGroup : group.getSubGroups()) { @@ -361,7 +396,9 @@ public class JpaRealmProvider implements RealmProvider { groupEntity.setRealm(realmEntity); em.persist(groupEntity); - return new GroupAdapter(realm, em, groupEntity); + GroupAdapter adapter = new GroupAdapter(realm, em, groupEntity); + adapters.put(id, adapter); + return adapter; } @Override @@ -391,6 +428,8 @@ public class JpaRealmProvider implements RealmProvider { em.persist(entity); em.flush(); final ClientModel resource = new ClientAdapter(realm, em, session, entity); + adapters.put(id, resource); + em.flush(); session.getKeycloakSessionFactory().publish(new RealmModel.ClientCreationEvent() { @Override @@ -416,14 +455,39 @@ public class JpaRealmProvider implements RealmProvider { } + protected JpaModel findJpaModel(String id) { + // we have a local map of adapter classes for two reasons + // 1. So we don't have to allocate one again + // 2. So that we can do em.refresh() on the entity to make sure the state + // is up to date. If em.find is called a second time without a session, the same + // entity instance is returned as its already in the first level cache. With the caching layer above it, it will cache stale + // entries. + JpaModel client = (JpaModel)adapters.get(id); + if (client != null) { + if (em.contains(client.getEntity())) { + em.flush(); // have to flush as refresh blows away updates + em.refresh(client.getEntity()); + return client; + } + } + return null; + + } @Override public ClientModel getClientById(String id, RealmModel realm) { + ClientAdapter client = (ClientAdapter)findJpaModel(id); + if (client != null) { + if (!realm.getId().equals(client.getRealm().getId())) return null; + return client; + } ClientEntity app = em.find(ClientEntity.class, id); - // Check if application belongs to this realm if (app == null || !realm.getId().equals(app.getRealm().getId())) return null; - return new ClientAdapter(realm, em, session, app); + client = new ClientAdapter(realm, em, session, app); + adapters.put(id, client); + return client; + } @Override @@ -459,15 +523,23 @@ public class JpaRealmProvider implements RealmProvider { logger.errorv("Unable to delete client entity: {0} from realm {1}", client.getClientId(), realm.getName()); throw e; } + adapters.remove(id); return true; } @Override public ClientTemplateModel getClientTemplateById(String id, RealmModel realm) { + ClientTemplateAdapter adapter = (ClientTemplateAdapter)findJpaModel(id); + if (adapter != null) { + if (!realm.getId().equals(adapter.getRealm().getId())) return null; + return adapter; + } ClientTemplateEntity app = em.find(ClientTemplateEntity.class, id); // Check if application belongs to this realm if (app == null || !realm.getId().equals(app.getRealm().getId())) return null; - return new ClientTemplateAdapter(realm, em, session, app); + adapter = new ClientTemplateAdapter(realm, em, session, app); + adapters.put(id, adapter); + return adapter; } } 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 3dd9de7e0f..f517782360 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 @@ -65,7 +65,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class RealmAdapter implements RealmModel { +public class RealmAdapter implements RealmModel, JpaModel { protected static final Logger logger = Logger.getLogger(RealmAdapter.class); protected RealmEntity realm; protected EntityManager em; diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java index 0ac1be9e8d..61cf70c4fb 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RoleAdapter.java @@ -21,6 +21,7 @@ import org.keycloak.models.KeycloakSession; import org.keycloak.models.RealmModel; import org.keycloak.models.RoleContainerModel; import org.keycloak.models.RoleModel; +import org.keycloak.models.jpa.entities.RealmEntity; import org.keycloak.models.jpa.entities.RoleEntity; import org.keycloak.models.utils.KeycloakModelUtils; @@ -33,7 +34,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class RoleAdapter implements RoleModel { +public class RoleAdapter implements RoleModel, JpaModel { protected RoleEntity role; protected EntityManager em; protected RealmModel realm; @@ -46,7 +47,7 @@ public class RoleAdapter implements RoleModel { this.session = session; } - public RoleEntity getRole() { + public RoleEntity getEntity() { return role; } @@ -97,17 +98,17 @@ public class RoleAdapter implements RoleModel { @Override public void addCompositeRole(RoleModel role) { RoleEntity entity = RoleAdapter.toRoleEntity(role, em); - for (RoleEntity composite : getRole().getCompositeRoles()) { + for (RoleEntity composite : getEntity().getCompositeRoles()) { if (composite.equals(entity)) return; } - getRole().getCompositeRoles().add(entity); + getEntity().getCompositeRoles().add(entity); em.flush(); } @Override public void removeCompositeRole(RoleModel role) { RoleEntity entity = RoleAdapter.toRoleEntity(role, em); - Iterator it = getRole().getCompositeRoles().iterator(); + Iterator it = getEntity().getCompositeRoles().iterator(); while (it.hasNext()) { if (it.next().equals(entity)) it.remove(); } @@ -117,7 +118,7 @@ public class RoleAdapter implements RoleModel { public Set getComposites() { Set set = new HashSet(); - for (RoleEntity composite : getRole().getCompositeRoles()) { + for (RoleEntity composite : getEntity().getCompositeRoles()) { set.add(new RoleAdapter(session, realm, em, composite)); // todo I want to do this, but can't as you get stack overflow @@ -161,7 +162,7 @@ public class RoleAdapter implements RoleModel { public static RoleEntity toRoleEntity(RoleModel model, EntityManager em) { if (model instanceof RoleAdapter) { - return ((RoleAdapter)model).getRole(); + return ((RoleAdapter)model).getEntity(); } return em.getReference(RoleEntity.class, model.getId()); } diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java index 9114af1e69..ef24368113 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/UserAdapter.java @@ -63,7 +63,7 @@ import java.util.Set; * @author Bill Burke * @version $Revision: 1 $ */ -public class UserAdapter implements UserModel { +public class UserAdapter implements UserModel, JpaModel { protected UserEntity user; protected EntityManager em; @@ -77,7 +77,7 @@ public class UserAdapter implements UserModel { this.session = session; } - public UserEntity getUser() { + public UserEntity getEntity() { return user; } @@ -503,7 +503,7 @@ public class UserAdapter implements UserModel { // we query ids only as the group might be cached and following the @ManyToOne will result in a load // even if we're getting just the id. TypedQuery query = em.createNamedQuery("userGroupIds", String.class); - query.setParameter("user", getUser()); + query.setParameter("user", getEntity()); List ids = query.getResultList(); Set groups = new HashSet<>(); for (String groupId : ids) { @@ -518,7 +518,7 @@ public class UserAdapter implements UserModel { public void joinGroup(GroupModel group) { if (isMemberOf(group)) return; UserGroupMembershipEntity entity = new UserGroupMembershipEntity(); - entity.setUser(getUser()); + entity.setUser(getEntity()); entity.setGroupId(group.getId()); em.persist(entity); em.flush(); @@ -548,7 +548,7 @@ public class UserAdapter implements UserModel { protected TypedQuery getUserGroupMappingQuery(GroupModel group) { TypedQuery query = em.createNamedQuery("userMemberOf", UserGroupMembershipEntity.class); - query.setParameter("user", getUser()); + query.setParameter("user", getEntity()); query.setParameter("groupId", group.getId()); return query; } @@ -562,7 +562,7 @@ public class UserAdapter implements UserModel { protected TypedQuery getUserRoleMappingEntityTypedQuery(RoleModel role) { TypedQuery query = em.createNamedQuery("userHasRole", UserRoleMappingEntity.class); - query.setParameter("user", getUser()); + query.setParameter("user", getEntity()); query.setParameter("roleId", role.getId()); return query; } @@ -571,7 +571,7 @@ public class UserAdapter implements UserModel { public void grantRole(RoleModel role) { if (hasRole(role)) return; UserRoleMappingEntity entity = new UserRoleMappingEntity(); - entity.setUser(getUser()); + entity.setUser(getEntity()); entity.setRoleId(role.getId()); em.persist(entity); em.flush(); @@ -598,7 +598,7 @@ public class UserAdapter implements UserModel { // we query ids only as the role might be cached and following the @ManyToOne will result in a load // even if we're getting just the id. TypedQuery query = em.createNamedQuery("userRoleMappingIds", String.class); - query.setParameter("user", getUser()); + query.setParameter("user", getEntity()); List ids = query.getResultList(); Set roles = new HashSet(); for (String roleId : ids) { diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java index fc10217127..efbd154f71 100755 --- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java +++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/ClientEntity.java @@ -50,6 +50,7 @@ import java.util.Set; @Table(name="CLIENT", uniqueConstraints = {@UniqueConstraint(columnNames = {"REALM_ID", "CLIENT_ID"})}) @NamedQueries({ @NamedQuery(name="getClientsByRealm", query="select client from ClientEntity client where client.realm = :realm"), + @NamedQuery(name="getClientById", query="select client from ClientEntity client where client.id = :id and client.realm.id = :realm"), @NamedQuery(name="getClientIdsByRealm", query="select client.id from ClientEntity client where client.realm.id = :realm"), @NamedQuery(name="findClientIdByClientId", query="select client.id from ClientEntity client where client.clientId = :clientId and client.realm.id = :realm"), @NamedQuery(name="findClientByClientId", query="select client from ClientEntity client where client.clientId = :clientId and client.realm.id = :realm"), diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java old mode 100644 new mode 100755 index d2bc3f8aae..93b6458d8d --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java +++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java @@ -36,7 +36,6 @@ import org.keycloak.models.utils.KeycloakModelUtils; public class ConcurrentTransactionsTest extends AbstractModelTest { @Test - @Ignore public void persistClient() throws Exception { RealmModel realm = realmManager.createRealm("original"); KeycloakSession session = realmManager.getSession(); @@ -129,6 +128,8 @@ public class ConcurrentTransactionsTest extends AbstractModelTest { thread1.join(); thread2.join(); + System.out.println("after thread join"); + commit(); session = realmManager.getSession();