From f32c2536b74bdbbf1b4fe07959d938621a634343 Mon Sep 17 00:00:00 2001
From: srosse <none@none>
Date: Tue, 3 Sep 2013 11:15:54 +0200
Subject: [PATCH] OO-753: remove the optimistic locking from
SecurityGroupMemebership and make almost all field immutable
---
.../basesecurity/BaseSecurityManager.java | 6 +-
.../SecurityGroupMembershipImpl.hbm.xml | 14 +-
.../SecurityGroupMembershipImpl.java | 12 +-
.../basesecurity/BaseSecurityManagerTest.java | 45 +++++
.../test/BusinessGroupConcurrentTest.java | 167 ++++++++++++++++++
.../java/org/olat/test/AllTestsJunit4.java | 1 +
6 files changed, 234 insertions(+), 11 deletions(-)
create mode 100644 src/test/java/org/olat/group/test/BusinessGroupConcurrentTest.java
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
index 5846f911334..16f0d0c8894 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
@@ -530,15 +530,17 @@ public class BaseSecurityManager extends BasicManager implements BaseSecurity {
}
/**
+ *
+ *
* @see org.olat.basesecurity.Manager#addIdentityToSecurityGroup(org.olat.core.id.Identity, org.olat.basesecurity.SecurityGroup)
*/
+ @Override
public void addIdentityToSecurityGroup(Identity identity, SecurityGroup secGroup) {
SecurityGroupMembershipImpl sgmsi = new SecurityGroupMembershipImpl();
sgmsi.setIdentity(identity);
sgmsi.setSecurityGroup(secGroup);
sgmsi.setLastModified(new Date());
- DBFactory.getInstance().saveObject(sgmsi);
- //TODO: tracing
+ dbInstance.getCurrentEntityManager().persist(sgmsi);
}
/**
diff --git a/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.hbm.xml b/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.hbm.xml
index e3099279059..3953895507f 100644
--- a/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.hbm.xml
+++ b/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.hbm.xml
@@ -8,16 +8,16 @@
<generator class="hilo"/>
</id>
- <version name="version" access="field" column="version" type="int"/>
- <property name="lastModified" column="lastmodified" type="timestamp" />
- <property name="creationDate" column="creationdate" type="timestamp" />
+ <property name="version" access="field" column="version" insert="true" update="false" type="int"/>
+ <property name="lastModified" column="lastmodified" type="timestamp" />
+ <property name="creationDate" column="creationdate" insert="true" update="false" type="timestamp" />
- <many-to-one name="securityGroup" class="org.olat.basesecurity.SecurityGroupImpl" fetch="join" cascade="none">
- <column name="secgroup_id" not-null="true" unique-key="groupmembership_unique" />
+ <many-to-one name="securityGroup" class="org.olat.basesecurity.SecurityGroupImpl" fetch="join" insert="true" update="false" cascade="none">
+ <column name="secgroup_id" not-null="true" unique-key="groupmembership_unique" />
</many-to-one>
- <many-to-one name="identity" class="org.olat.basesecurity.IdentityImpl" fetch="join" cascade="none">
- <column name="identity_id" not-null="true" unique-key="groupmembership_unique" />
+ <many-to-one name="identity" class="org.olat.basesecurity.IdentityImpl" fetch="join" insert="true" update="false" cascade="none">
+ <column name="identity_id" not-null="true" unique-key="groupmembership_unique" />
</many-to-one>
</class>
diff --git a/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.java b/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.java
index 42540917b57..1f58ae5aa7a 100644
--- a/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.java
+++ b/src/main/java/org/olat/basesecurity/SecurityGroupMembershipImpl.java
@@ -33,10 +33,16 @@ import org.olat.core.id.ModifiedInfo;
/**
* Description: <br>
+ * Relation between the security group and identity. The object is almost immutable
+ * on the hibernate mapping.
+ *
*
* @author Felix Jost
*/
public class SecurityGroupMembershipImpl extends PersistentObject implements ModifiedInfo {
+
+ private static final long serialVersionUID = 2466302280763907357L;
+
private Identity identity;
private SecurityGroup securityGroup;
private Date lastModified;
@@ -63,7 +69,8 @@ public class SecurityGroupMembershipImpl extends PersistentObject implements Mod
}
/**
- * Sets the identity.
+ * Sets the identity. The identity cannot be changed and updated.
+ * The identity is only inserted to the database but never updated
*
* @param identity The identity to set
*/
@@ -72,7 +79,8 @@ public class SecurityGroupMembershipImpl extends PersistentObject implements Mod
}
/**
- * Sets the securityGroup.
+ * Sets the securityGroup. The security group cannot be changed and
+ * updated. It is only inserted to the dabatase but never updated.
*
* @param securityGroup The securityGroup to set
*/
diff --git a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
index 3b57ecd6ab7..f20dc906bb9 100644
--- a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
+++ b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
@@ -24,7 +24,9 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.util.ArrayList;
+import java.util.Calendar;
import java.util.Collections;
+import java.util.Date;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
@@ -845,4 +847,47 @@ public class BaseSecurityManagerTest extends OlatTestCase {
String credential = securityManager.findCredentials(id, BaseSecurityModule.getDefaultAuthProviderIdentifier());
Assert.assertNotNull(credential);
}
+
+ /**
+ * Dummy test to make sure all works as wanted
+ */
+ @Test
+ public void createSecurityGroupMembership() {
+ //create a user with the default provider
+ Identity identity = JunitTestHelper.createAndPersistIdentityAsUser("update-membership-" + UUID.randomUUID().toString());
+ SecurityGroup secGroup = securityManager.createAndPersistSecurityGroup();
+ securityManager.addIdentityToSecurityGroup(identity, secGroup);
+ dbInstance.commitAndCloseSession();
+
+ boolean member = securityManager.isIdentityInSecurityGroup(identity, secGroup);
+ Assert.assertTrue(member);
+ }
+
+ /**
+ * We remove the optimistic locking from SecurityGroupMembershipImpl mapping
+ */
+ @Test
+ public void createAndUpdateSecurityGroupMembership_lastCommitWin() {
+ //create a user with the default provider
+ Identity identity = JunitTestHelper.createAndPersistIdentityAsUser("update-membership-" + UUID.randomUUID().toString());
+ SecurityGroup secGroup = securityManager.createAndPersistSecurityGroup();
+
+ SecurityGroupMembershipImpl sgmsi = new SecurityGroupMembershipImpl();
+ sgmsi.setIdentity(identity);
+ sgmsi.setSecurityGroup(secGroup);
+ sgmsi.setLastModified(new Date());
+ dbInstance.getCurrentEntityManager().persist(sgmsi);
+ dbInstance.commitAndCloseSession();
+
+ Calendar cal = Calendar.getInstance();
+ cal.add(Calendar.DATE, -1);
+ sgmsi.setLastModified(cal.getTime());
+ dbInstance.getCurrentEntityManager().merge(sgmsi);
+ dbInstance.commitAndCloseSession();
+
+ cal.add(Calendar.DATE, -1);
+ sgmsi.setLastModified(cal.getTime());
+ dbInstance.getCurrentEntityManager().merge(sgmsi);
+ dbInstance.commitAndCloseSession();
+ }
}
diff --git a/src/test/java/org/olat/group/test/BusinessGroupConcurrentTest.java b/src/test/java/org/olat/group/test/BusinessGroupConcurrentTest.java
new file mode 100644
index 00000000000..19681067237
--- /dev/null
+++ b/src/test/java/org/olat/group/test/BusinessGroupConcurrentTest.java
@@ -0,0 +1,167 @@
+/**
+ * <a href="http://www.openolat.org">
+ * OpenOLAT - Online Learning and Training</a><br>
+ * <p>
+ * Licensed under the Apache License, Version 2.0 (the "License"); <br>
+ * you may not use this file except in compliance with the License.<br>
+ * You may obtain a copy of the License at the
+ * <a href="http://www.apache.org/licenses/LICENSE-2.0">Apache homepage</a>
+ * <p>
+ * Unless required by applicable law or agreed to in writing,<br>
+ * software distributed under the License is distributed on an "AS IS" BASIS, <br>
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. <br>
+ * See the License for the specific language governing permissions and <br>
+ * limitations under the License.
+ * <p>
+ * Initial code contributed and copyrighted by<br>
+ * frentix GmbH, http://www.frentix.com
+ * <p>
+ */
+package org.olat.group.test;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.UUID;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import junit.framework.Assert;
+
+import org.junit.Test;
+import org.olat.basesecurity.BaseSecurity;
+import org.olat.core.CoreSpringFactory;
+import org.olat.core.commons.persistence.DB;
+import org.olat.core.id.Identity;
+import org.olat.group.BusinessGroup;
+import org.olat.group.BusinessGroupService;
+import org.olat.group.manager.BusinessGroupDAO;
+import org.olat.test.JunitTestHelper;
+import org.olat.test.OlatTestCase;
+import org.springframework.beans.factory.annotation.Autowired;
+
+/**
+ *
+ * Initial date: 28.08.2013<br>
+ * @author srosse, stephane.rosse@frentix.com, http://www.frentix.com
+ *
+ */
+public class BusinessGroupConcurrentTest extends OlatTestCase {
+
+ @Autowired
+ private DB dbInstance;
+ @Autowired
+ private BaseSecurity securityManager;
+ @Autowired
+ private BusinessGroupDAO businessGroupDao;
+
+
+
+ @Test
+ public void concurrentSetLastUsageFor_multipleUser() {
+ BusinessGroup group = businessGroupDao.createAndPersist(null, "gdao", "gdao-desc", -1, -1, false, false, false, false, false);
+ dbInstance.commit();
+
+ int numOfThreads = 25;
+ final CountDownLatch doneSignal = new CountDownLatch(numOfThreads);
+
+ SetLastUsageThread[] threads = new SetLastUsageThread[numOfThreads];
+ for(int i=numOfThreads; i-->0; ) {
+ Identity id = JunitTestHelper.createAndPersistIdentityAsUser("group-concurent-" + i + "-" + UUID.randomUUID().toString());
+ securityManager.addIdentityToSecurityGroup(id, group.getPartipiciantGroup());
+ threads[i] = new SetLastUsageThread(group.getKey(), id, doneSignal);
+ }
+ dbInstance.commitAndCloseSession();;
+
+ for(int i=numOfThreads; i-->0; ) {
+ threads[i].start();
+ }
+
+ try {
+ boolean interrupt = doneSignal.await(240, TimeUnit.SECONDS);
+ assertTrue("Test takes too long (more than 10s)", interrupt);
+ } catch (InterruptedException e) {
+ fail("" + e.getMessage());
+ }
+
+ int errorCount = 0;
+ for(int i=numOfThreads; i-->0; ) {
+ errorCount += threads[i].getErrorCount();
+ }
+ Assert.assertEquals(0, errorCount);
+ }
+
+ @Test
+ public void concurrentSetLastUsageFor_singleUser() {
+ Identity id = JunitTestHelper.createAndPersistIdentityAsUser("group-cc-single-" + UUID.randomUUID().toString());
+ BusinessGroup group = businessGroupDao.createAndPersist(id, "gdao", "gdao-desc", -1, -1, false, false, false, false, false);
+ dbInstance.commitAndCloseSession();
+
+ int numOfThreads = 25;
+ final CountDownLatch doneSignal = new CountDownLatch(numOfThreads);
+
+ SetLastUsageThread[] threads = new SetLastUsageThread[numOfThreads];
+ for(int i=numOfThreads; i-->0; ) {
+ threads[i] = new SetLastUsageThread(group.getKey(), id, doneSignal);
+ }
+
+ for(int i=numOfThreads; i-->0; ) {
+ threads[i].start();
+ }
+
+ try {
+ boolean interrupt = doneSignal.await(240, TimeUnit.SECONDS);
+ assertTrue("Test takes too long (more than 10s)", interrupt);
+ } catch (InterruptedException e) {
+ fail("" + e.getMessage());
+ }
+
+ int errorCount = 0;
+ for(int i=numOfThreads; i-->0; ) {
+ errorCount += threads[i].getErrorCount();
+ }
+ Assert.assertEquals(0, errorCount);
+ }
+
+
+ private class SetLastUsageThread extends Thread {
+
+ private AtomicInteger errorCount = new AtomicInteger();
+
+ private final Long key;
+ private final Identity identity;
+ private final CountDownLatch doneSignal;
+
+ public SetLastUsageThread(Long key, Identity identity, CountDownLatch doneSignal) {
+ this.key = key;
+ this.identity = identity;
+ this.doneSignal = doneSignal;
+ }
+
+ public int getErrorCount() {
+ return errorCount.get();
+ }
+
+ @Override
+ public void run() {
+ try {
+ Thread.sleep(10);
+ } catch (InterruptedException e) {
+ e.printStackTrace();
+ }
+ BusinessGroupService service = CoreSpringFactory.getImpl(BusinessGroupService.class);
+ try {
+ BusinessGroup group = service.loadBusinessGroup(key);
+ for(int i=50; i-->0; ) {
+ group = service.setLastUsageFor(identity, group);
+ }
+ } catch (Exception e) {
+ e.printStackTrace();
+ errorCount.incrementAndGet();
+ } finally {
+ doneSignal.countDown();
+ }
+ }
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/org/olat/test/AllTestsJunit4.java b/src/test/java/org/olat/test/AllTestsJunit4.java
index a20ceb906b2..308bb1b94d3 100644
--- a/src/test/java/org/olat/test/AllTestsJunit4.java
+++ b/src/test/java/org/olat/test/AllTestsJunit4.java
@@ -83,6 +83,7 @@ import org.junit.runners.Suite;
org.olat.group.test.BusinessGroupServiceTest.class,//ok
org.olat.group.test.BusinessGroupDAOTest.class,//ok
org.olat.group.test.BusinessGroupRelationDAOTest.class,//ok
+ org.olat.group.test.BusinessGroupConcurrentTest.class,//ok
org.olat.group.test.ContactDAOTest.class,//ok
org.olat.resource.lock.pessimistic.PLockTest.class,//ok
org.olat.resource.references.ReferenceManagerTest.class,//ok
--
GitLab