From 9d46b45a9cf963fdcbdfe43bd7e2e6d58958f41b Mon Sep 17 00:00:00 2001 From: Alexander Schwartz Date: Mon, 24 Jan 2022 14:59:34 +0100 Subject: [PATCH] Ensure that parent's version ID is incremented when an attribute changes. This is necessary to allow the optimistic locking functionality to work as expected when changing only attributes on an entity. Closes #9874 --- .../map/storage/jpa/JpaChildEntity.java | 28 ++++++++ .../storage/jpa/JpaChildEntityListener.java | 70 +++++++++++++++++++ .../jpa/JpaMapStorageProviderFactory.java | 33 +++++++++ .../entity/JpaClientAttributeEntity.java | 8 ++- .../jpa/client/entity/JpaClientEntity.java | 3 - .../role/entity/JpaRoleAttributeEntity.java | 9 ++- .../jpa/role/entity/JpaRoleEntity.java | 3 - 7 files changed, 146 insertions(+), 8 deletions(-) create mode 100644 model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntity.java create mode 100644 model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntityListener.java diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntity.java new file mode 100644 index 00000000000..e18f45f0eb9 --- /dev/null +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntity.java @@ -0,0 +1,28 @@ +/* + * Copyright 2022. Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.keycloak.models.map.storage.jpa; + +/** + * Interface for all child entities for JPA map storage. + */ +public interface JpaChildEntity { + + /** + * Parent entity that should get its optimistic locking version updated upon changes in the child + */ + R getParent(); +} diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntityListener.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntityListener.java new file mode 100644 index 00000000000..774b8158be9 --- /dev/null +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaChildEntityListener.java @@ -0,0 +1,70 @@ +/* + * Copyright 2022. Red Hat, Inc. and/or its affiliates + * and other contributors as indicated by the @author tags. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.keycloak.models.map.storage.jpa; + +import org.hibernate.HibernateException; +import org.hibernate.Session; +import org.hibernate.event.spi.PreDeleteEvent; +import org.hibernate.event.spi.PreDeleteEventListener; +import org.hibernate.event.spi.PreInsertEvent; +import org.hibernate.event.spi.PreInsertEventListener; +import org.hibernate.event.spi.PreUpdateEvent; +import org.hibernate.event.spi.PreUpdateEventListener; + +import javax.persistence.LockModeType; + +/** + * Listen on changes on child entities and forces an optimistic locking increment on the topmost parent. + * + * This support a multiple level parent-child relationship, where only the upmost parent is locked. + */ +public class JpaChildEntityListener implements PreInsertEventListener, PreDeleteEventListener, PreUpdateEventListener { + + public static final JpaChildEntityListener INSTANCE = new JpaChildEntityListener(); + + /** + * Check if the entity is a child with a parent and force optimistic locking increment on the upmost parent. + */ + public void checkRoot(Session session, Object entity) throws HibernateException { + if(entity instanceof JpaChildEntity) { + Object root = entity; + while (root instanceof JpaChildEntity) { + root = ((JpaChildEntity) entity).getParent(); + } + session.lock(root, LockModeType.OPTIMISTIC_FORCE_INCREMENT); + } + } + + @Override + public boolean onPreInsert(PreInsertEvent event) { + checkRoot(event.getSession(), event.getEntity()); + return false; + } + + @Override + public boolean onPreDelete(PreDeleteEvent event) { + checkRoot(event.getSession(), event.getEntity()); + return false; + } + + @Override + public boolean onPreUpdate(PreUpdateEvent event) { + checkRoot(event.getSession(), event.getEntity()); + return false; + } +} diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaMapStorageProviderFactory.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaMapStorageProviderFactory.java index ff32c491b03..97126c1af1d 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaMapStorageProviderFactory.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/JpaMapStorageProviderFactory.java @@ -20,6 +20,7 @@ import java.sql.Connection; import java.sql.DatabaseMetaData; import java.sql.DriverManager; import java.sql.SQLException; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Map; @@ -30,7 +31,14 @@ import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; import javax.persistence.Persistence; import javax.sql.DataSource; + +import org.hibernate.boot.Metadata; import org.hibernate.cfg.AvailableSettings; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.event.service.spi.EventListenerRegistry; +import org.hibernate.event.spi.EventType; +import org.hibernate.jpa.boot.spi.IntegratorProvider; +import org.hibernate.service.spi.SessionFactoryServiceRegistry; import org.jboss.logging.Logger; import org.keycloak.Config; import org.keycloak.common.Profile; @@ -166,6 +174,31 @@ public class JpaMapStorageProviderFactory implements properties.put("hibernate.format_sql", config.getBoolean("formatSql", true)); properties.put("hibernate.dialect", config.get("driverDialect")); + properties.put( + "hibernate.integrator_provider", + (IntegratorProvider) () -> Collections.singletonList( + new org.hibernate.integrator.spi.Integrator() { + + @Override + public void integrate(Metadata metadata, SessionFactoryImplementor sessionFactoryImplementor, + SessionFactoryServiceRegistry sessionFactoryServiceRegistry) { + final EventListenerRegistry eventListenerRegistry = + sessionFactoryServiceRegistry.getService( EventListenerRegistry.class ); + + eventListenerRegistry.appendListeners(EventType.PRE_INSERT, JpaChildEntityListener.INSTANCE); + eventListenerRegistry.appendListeners(EventType.PRE_UPDATE, JpaChildEntityListener.INSTANCE); + eventListenerRegistry.appendListeners(EventType.PRE_DELETE, JpaChildEntityListener.INSTANCE); + } + + @Override + public void disintegrate(SessionFactoryImplementor sessionFactoryImplementor, + SessionFactoryServiceRegistry sessionFactoryServiceRegistry) { + + } + } + ) + ); + Integer isolation = config.getInt("isolation"); if (isolation != null) { if (isolation < Connection.TRANSACTION_REPEATABLE_READ) { diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientAttributeEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientAttributeEntity.java index 715fcbcfa7a..2c36ab6e264 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientAttributeEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/client/entity/JpaClientAttributeEntity.java @@ -28,10 +28,11 @@ import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; import javax.persistence.Table; import org.hibernate.annotations.Nationalized; +import org.keycloak.models.map.storage.jpa.JpaChildEntity; @Entity @Table(name = "client_attribute") -public class JpaClientAttributeEntity implements Serializable { +public class JpaClientAttributeEntity implements JpaChildEntity, Serializable { @Id @Column @@ -100,4 +101,9 @@ public class JpaClientAttributeEntity implements Serializable { Objects.equals(getName(), that.getName()) && Objects.equals(getValue(), that.getValue()); } + + @Override + public JpaClientEntity getParent() { + return getClient(); + } } 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 4c0a3c584c3..9fc57653e62 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 @@ -587,7 +587,6 @@ public class JpaClientEntity extends AbstractClientEntity implements JpaRootEnti JpaClientAttributeEntity attr = iterator.next(); if (Objects.equals(attr.getName(), name)) { iterator.remove(); - attr.setClient(null); } } } @@ -625,9 +624,7 @@ public class JpaClientEntity extends AbstractClientEntity implements JpaRootEnti public void setAttributes(Map> attributes) { checkEntityVersionForUpdate(); for (Iterator iterator = this.attributes.iterator(); iterator.hasNext();) { - JpaClientAttributeEntity attr = iterator.next(); iterator.remove(); - attr.setClient(null); } if (attributes != null) { for (Map.Entry> attrEntry : attributes.entrySet()) { diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleAttributeEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleAttributeEntity.java index f162bd321cf..efe24957bad 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleAttributeEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleAttributeEntity.java @@ -28,10 +28,12 @@ import javax.persistence.JoinColumn; import javax.persistence.ManyToOne; import javax.persistence.Table; import org.hibernate.annotations.Nationalized; +import org.keycloak.models.map.storage.jpa.JpaChildEntity; +import org.keycloak.models.map.storage.jpa.client.entity.JpaClientEntity; @Entity @Table(name = "role_attribute") -public class JpaRoleAttributeEntity implements Serializable { +public class JpaRoleAttributeEntity implements JpaChildEntity, Serializable { @Id @Column @@ -100,4 +102,9 @@ public class JpaRoleAttributeEntity implements Serializable { Objects.equals(getName(), that.getName()) && Objects.equals(getValue(), that.getValue()); } + + @Override + public JpaRoleEntity getParent() { + return role; + } } diff --git a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java index 9ea1409158a..b4c566e36bf 100644 --- a/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java +++ b/model/map-jpa/src/main/java/org/keycloak/models/map/storage/jpa/role/entity/JpaRoleEntity.java @@ -257,9 +257,7 @@ public class JpaRoleEntity extends AbstractRoleEntity implements JpaRootEntity { public void setAttributes(Map> attributes) { checkEntityVersionForUpdate(); for (Iterator iterator = this.attributes.iterator(); iterator.hasNext();) { - JpaRoleAttributeEntity attr = iterator.next(); iterator.remove(); - attr.setRole(null); } if (attributes != null) { for (Map.Entry> entry : attributes.entrySet()) { @@ -285,7 +283,6 @@ public class JpaRoleEntity extends AbstractRoleEntity implements JpaRootEntity { JpaRoleAttributeEntity attr = iterator.next(); if (Objects.equals(attr.getName(), name)) { iterator.remove(); - attr.setRole(null); } } }