Merge pull request #3017 from mposolda/rhit
KEYCLOAK-3296 same user logging twice at the same time causes lock is…
This commit is contained in:
commit
64ad222a28
4 changed files with 102 additions and 18 deletions
|
@ -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
|
||||
|
|
|
@ -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")
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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());
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue