Details
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java
index 263757f..2b5d35c 100755
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/entities/UserAttributeEntity.java
@@ -42,6 +42,7 @@ import java.util.Set;
@NamedQueries({
@NamedQuery(name="getAttributesByNameAndValue", query="select attr from UserAttributeEntity attr where attr.name = :name and attr.value = :value"),
@NamedQuery(name="deleteUserAttributesByRealm", query="delete from UserAttributeEntity attr where attr.user IN (select u from UserEntity u where u.realmId=:realmId)"),
+ @NamedQuery(name="deleteUserAttributesByNameAndUser", query="delete from UserAttributeEntity attr where attr.user.id = :userId and attr.name = :name"),
@NamedQuery(name="deleteUserAttributesByRealmAndLink", query="delete from UserAttributeEntity attr where attr.user IN (select u from UserEntity u where u.realmId=:realmId and u.federationLink=:link)")
})
@Table(name="USER_ATTRIBUTE")
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 5d51fc9..2ea12fd 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
@@ -47,6 +47,7 @@ import org.keycloak.common.util.MultivaluedHashMap;
import org.keycloak.common.util.Time;
import javax.persistence.EntityManager;
+import javax.persistence.Query;
import javax.persistence.TypedQuery;
import java.util.ArrayList;
@@ -172,14 +173,11 @@ public class UserAdapter implements UserModel, JpaModel<UserEntity> {
@Override
public void removeAttribute(String name) {
- Iterator<UserAttributeEntity> it = user.getAttributes().iterator();
- while (it.hasNext()) {
- UserAttributeEntity attr = it.next();
- if (attr.getName().equals(name)) {
- it.remove();
- em.remove(attr);
- }
- }
+ // KEYCLOAK-3296 : Remove attribute through HQL to avoid StaleUpdateException
+ Query query = em.createNamedQuery("deleteUserAttributesByNameAndUser");
+ query.setParameter("name", name);
+ query.setParameter("userId", user.getId());
+ int numUpdated = query.executeUpdate();
}
@Override
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPMultipleAttributesTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPMultipleAttributesTest.java
index 2e6a10a..fb23b5d 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPMultipleAttributesTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/federation/ldap/base/LDAPMultipleAttributesTest.java
@@ -166,6 +166,14 @@ public class LDAPMultipleAttributesTest {
postalCodes.add("77332");
user.setAttribute("postal_code", postalCodes);
+ } finally {
+ keycloakRule.stopSession(session, true);
+ }
+
+ session = keycloakRule.startSession();
+ try {
+ RealmModel appRealm = session.realms().getRealmByName("test");
+ UserModel user = session.users().getUserByUsername("bwilson", appRealm);
assertPostalCodes(user.getAttribute("postal_code"), "88441", "77332");
} finally {
keycloakRule.stopSession(session, true);
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
index 93b6458..b3ad0c4 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/model/ConcurrentTransactionsTest.java
@@ -18,7 +18,9 @@
package org.keycloak.testsuite.model;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicReference;
+import org.jboss.logging.Logger;
import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
@@ -28,6 +30,7 @@ import org.keycloak.models.KeycloakSessionFactory;
import org.keycloak.models.KeycloakSessionTask;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RealmProvider;
+import org.keycloak.models.UserModel;
import org.keycloak.models.utils.KeycloakModelUtils;
/**
@@ -35,6 +38,8 @@ import org.keycloak.models.utils.KeycloakModelUtils;
*/
public class ConcurrentTransactionsTest extends AbstractModelTest {
+ private static final Logger logger = Logger.getLogger(ConcurrentTransactionsTest.class);
+
@Test
public void persistClient() throws Exception {
RealmModel realm = realmManager.createRealm("original");
@@ -63,21 +68,21 @@ public class ConcurrentTransactionsTest extends AbstractModelTest {
try {
// Wait until transaction in both threads started
transactionsCounter.countDown();
- System.out.println("transaction1 started");
+ logger.info("transaction1 started");
transactionsCounter.await();
// Read client
RealmModel realm = session.realms().getRealmByName("original");
ClientModel client = session.realms().getClientByClientId("client", realm);
- System.out.println("transaction1: Read client finished");
+ logger.info("transaction1: Read client finished");
readLatch.countDown();
// Wait until thread2 updates client and commits
updateLatch.await();
- System.out.println("transaction1: Going to read client again");
+ logger.info("transaction1: Going to read client again");
client = session.realms().getClientByClientId("client", realm);
- System.out.println("transaction1: secret: " + client.getSecret());
+ logger.info("transaction1: secret: " + client.getSecret());
} catch (Exception e) {
throw new RuntimeException(e);
}
@@ -99,12 +104,12 @@ public class ConcurrentTransactionsTest extends AbstractModelTest {
try {
// Wait until transaction in both threads started
transactionsCounter.countDown();
- System.out.println("transaction2 started");
+ logger.info("transaction2 started");
transactionsCounter.await();
readLatch.await();
- System.out.println("transaction2: Going to update client secret");
+ logger.info("transaction2: Going to update client secret");
RealmModel realm = session.realms().getRealmByName("original");
ClientModel client = session.realms().getClientByClientId("client", realm);
@@ -116,7 +121,7 @@ public class ConcurrentTransactionsTest extends AbstractModelTest {
});
- System.out.println("transaction2: commited");
+ logger.info("transaction2: commited");
updateLatch.countDown();
}
@@ -128,7 +133,7 @@ public class ConcurrentTransactionsTest extends AbstractModelTest {
thread1.join();
thread2.join();
- System.out.println("after thread join");
+ logger.info("after thread join");
commit();
@@ -138,11 +143,83 @@ public class ConcurrentTransactionsTest extends AbstractModelTest {
ClientModel clientFromCache = session.realms().getClientById(clientDBId, realm);
ClientModel clientFromDB = session.getProvider(RealmProvider.class).getClientById(clientDBId, realm);
- System.out.println("SECRET FROM DB : " + clientFromDB.getSecret());
- System.out.println("SECRET FROM CACHE : " + clientFromCache.getSecret());
+ logger.info("SECRET FROM DB : " + clientFromDB.getSecret());
+ logger.info("SECRET FROM CACHE : " + clientFromCache.getSecret());
Assert.assertEquals("new", clientFromDB.getSecret());
Assert.assertEquals("new", clientFromCache.getSecret());
}
+
+ // KEYCLOAK-3296
+ @Test
+ public void removeUserAttribute() throws Exception {
+ RealmModel realm = realmManager.createRealm("original");
+ KeycloakSession session = realmManager.getSession();
+
+ UserModel user = session.users().addUser(realm, "john");
+ user.setSingleAttribute("foo", "val1");
+
+ final KeycloakSessionFactory sessionFactory = session.getKeycloakSessionFactory();
+ commit();
+
+ AtomicReference<Exception> reference = new AtomicReference<>();
+
+ final CountDownLatch readAttrLatch = new CountDownLatch(2);
+
+ Runnable runnable = new Runnable() {
+
+ @Override
+ public void run() {
+ try {
+ KeycloakModelUtils.runJobInTransaction(sessionFactory, new KeycloakSessionTask() {
+
+ @Override
+ public void run(KeycloakSession session) {
+ try {
+ // Read user attribute
+ RealmModel realm = session.realms().getRealmByName("original");
+ UserModel john = session.users().getUserByUsername("john", realm);
+ String attrVal = john.getFirstAttribute("foo");
+
+ // Wait until it's read in both threads
+ readAttrLatch.countDown();
+ readAttrLatch.await();
+
+ // Remove user attribute in both threads
+ john.removeAttribute("foo");
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }
+
+ });
+ } catch (Exception e) {
+ reference.set(e);
+ throw new RuntimeException(e);
+ } finally {
+ readAttrLatch.countDown();
+ }
+ }
+
+ };
+
+ Thread thread1 = new Thread(runnable);
+ Thread thread2 = new Thread(runnable);
+
+ thread1.start();
+ thread2.start();
+
+ thread1.join();
+ thread2.join();
+
+ logger.info("removeUserAttribute: after thread join");
+
+ commit();
+
+ if (reference.get() != null) {
+ Assert.fail("Exception happened in some of threads. Details: " + reference.get().getMessage());
+ }
+ }
+
}
