From 43175adbcd5538e87d903d04355fbed1045d914f Mon Sep 17 00:00:00 2001
From: jakubs <jakubs>
Date: Thu, 27 Jun 2013 07:39:05 +0000
Subject: [PATCH] SP-727 BIS-462 move update sessions when role assignments
 change to hibernate interceptor

SVN: 29470
---
 .../openbis/generic/server/CommonServer.java  |   4 -
 .../server/ServiceForDataStoreServer.java     |   2 -
 .../server/SessionsUpdateInterceptor.java     |  85 +++++
 .../HibernateInterceptorsWrapper.java         |  23 +-
 .../OpenBISHibernateTransactionManager.java   |  19 +-
 .../source/java/genericApplicationContext.xml |   3 +-
 .../generic/server/CommonServerTest.java      |   4 -
 .../generic/server/ETLServiceTest.java        |   2 -
 .../openbis/systemtest/CommonServerTest.java  |  75 -----
 .../systemtest/EntityOperationBuilder.java    | 317 ++++++++++++++++++
 .../systemtest/EntityOperationTest.java       | 273 ---------------
 .../openbis/systemtest/SessionUpdateTest.java | 136 ++++++++
 12 files changed, 569 insertions(+), 374 deletions(-)
 create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/server/SessionsUpdateInterceptor.java
 create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationBuilder.java
 create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/SessionUpdateTest.java

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java
index d51c6aeecdb..3c243d829a4 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/CommonServer.java
@@ -558,7 +558,6 @@ public final class CommonServer extends AbstractCommonServer<ICommonServerForInt
         table.add(newRoleAssignment);
         table.save();
 
-        sessionManager.updateAllSessions(getDAOFactory());
     }
 
     @Override
@@ -597,7 +596,6 @@ public final class CommonServer extends AbstractCommonServer<ICommonServerForInt
             }
         }
         getDAOFactory().getRoleAssignmentDAO().deleteRoleAssignment(roleAssignment);
-        sessionManager.updateAllSessions(getDAOFactory());
     }
 
     @Override
@@ -622,7 +620,6 @@ public final class CommonServer extends AbstractCommonServer<ICommonServerForInt
                             + "Ask another instance admin to do that for you.");
         }
         roleAssignmentDAO.deleteRoleAssignment(roleAssignment);
-        sessionManager.updateAllSessions(getDAOFactory());
     }
 
     @Override
@@ -2315,7 +2312,6 @@ public final class CommonServer extends AbstractCommonServer<ICommonServerForInt
         {
             spaceBO.deleteByTechId(id, reason);
         }
-        sessionManager.updateAllSessions(getDAOFactory());
     }
 
     @Override
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ServiceForDataStoreServer.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ServiceForDataStoreServer.java
index b86ba07b22f..7ef9e5938f5 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ServiceForDataStoreServer.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/ServiceForDataStoreServer.java
@@ -2052,8 +2052,6 @@ public class ServiceForDataStoreServer extends AbstractCommonServer<IServiceForD
             assignment.setGrantee(grantee);
             roleTable.add(assignment);
             roleTable.save();
-
-            sessionManager.updateAllSessions(getDAOFactory());
         }
         return space;
 
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/SessionsUpdateInterceptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/SessionsUpdateInterceptor.java
new file mode 100644
index 00000000000..45f5c325210
--- /dev/null
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/SessionsUpdateInterceptor.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright 2013 ETH Zuerich, CISD
+ *
+ * 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 ch.systemsx.cisd.openbis.generic.server;
+
+import java.io.Serializable;
+
+import org.hibernate.EmptyInterceptor;
+import org.hibernate.Transaction;
+import org.hibernate.type.Type;
+
+import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ServiceVersionHolder;
+import ch.systemsx.cisd.openbis.generic.shared.dto.RoleAssignmentPE;
+
+/**
+ * @author Jakub Straszewski
+ */
+public class SessionsUpdateInterceptor extends EmptyInterceptor
+{
+    private static final long serialVersionUID = ServiceVersionHolder.VERSION;
+
+    private OpenBisSessionManager openBisSessionManager;
+
+    private IDAOFactory daoFactory;
+
+    private boolean rolesAssignmentsChanged;
+
+    public SessionsUpdateInterceptor(OpenBisSessionManager openBisSessionManager, IDAOFactory daoFactory)
+    {
+        this.openBisSessionManager = openBisSessionManager;
+        this.daoFactory = daoFactory;
+    }
+
+    @Override
+    public boolean onFlushDirty(Object entity, Serializable id, Object[] currentState, Object[] previousState, String[] propertyNames, Type[] types)
+    {
+        checkEntity(entity);
+        return false;
+    }
+
+    @Override
+    public void onDelete(Object entity, Serializable id, Object[] state, String[] propertyNames, Type[] types)
+    {
+        checkEntity(entity);
+    }
+
+    @Override
+    public boolean onSave(Object entity, Serializable id, Object[] state, String[] propertyNames, Type[] types)
+    {
+        checkEntity(entity);
+        return false;
+    }
+
+    private void checkEntity(Object entity)
+    {
+        if (entity instanceof RoleAssignmentPE)
+        {
+            rolesAssignmentsChanged = true;
+        }
+    }
+
+    @Override
+    public void afterTransactionCompletion(Transaction tx)
+    {
+        if (rolesAssignmentsChanged && tx.wasCommitted())
+        {
+            openBisSessionManager.updateAllSessions(daoFactory);
+        }
+    }
+
+}
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/HibernateInterceptorsWrapper.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/HibernateInterceptorsWrapper.java
index a271ac4cc2a..63670020200 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/HibernateInterceptorsWrapper.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/HibernateInterceptorsWrapper.java
@@ -22,16 +22,15 @@ import org.hibernate.EmptyInterceptor;
 import org.hibernate.Transaction;
 import org.hibernate.type.Type;
 
+import ch.systemsx.cisd.openbis.generic.server.SessionsUpdateInterceptor;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.DynamicPropertiesInterceptor;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.EntityValidationInterceptor;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ServiceVersionHolder;
 
 /**
- * A wrapper for two interceptors. {@link DynamicPropertiesInterceptor} and
- * {@link EntityValidationInterceptor}. This class only provides implementation for the three
- * methods implemented in both our implementations. Implemetation assumes, that our interceptors
- * don't change entities when called, and thus always return false from the methods onFlushDirty and
- * onSave.
+ * A wrapper for several interceptors. {@link DynamicPropertiesInterceptor}, {@link EntityValidationInterceptor} and {@link SessionsUpdateInterceptor}
+ * . This class only provides implementation for the three methods implemented in both our implementations. Implemetation assumes, that our
+ * interceptors don't change entities when called, and thus always return false from the methods onFlushDirty and onSave.
  * 
  * @author Jakub Straszewski
  */
@@ -43,11 +42,14 @@ public class HibernateInterceptorsWrapper extends EmptyInterceptor implements Se
 
     EntityValidationInterceptor entityValidationInterceptor;
 
+    SessionsUpdateInterceptor sessionsUpdateInterceptor;
+
     public HibernateInterceptorsWrapper(DynamicPropertiesInterceptor hibernateInterceptor,
-            EntityValidationInterceptor entityValidationInterceptor)
+            EntityValidationInterceptor entityValidationInterceptor, SessionsUpdateInterceptor sessionsUpdateInterceptor)
     {
         this.dynamicPropertiesInterceptor = hibernateInterceptor;
         this.entityValidationInterceptor = entityValidationInterceptor;
+        this.sessionsUpdateInterceptor = sessionsUpdateInterceptor;
     }
 
     @Override
@@ -58,6 +60,7 @@ public class HibernateInterceptorsWrapper extends EmptyInterceptor implements Se
                 propertyNames, types);
         entityValidationInterceptor.onFlushDirty(entity, id, currentState, previousState,
                 propertyNames, types);
+        sessionsUpdateInterceptor.onFlushDirty(entity, id, currentState, previousState, propertyNames, types);
         return false;
     }
 
@@ -67,14 +70,22 @@ public class HibernateInterceptorsWrapper extends EmptyInterceptor implements Se
     {
         dynamicPropertiesInterceptor.onSave(entity, id, state, propertyNames, types);
         entityValidationInterceptor.onSave(entity, id, state, propertyNames, types);
+        sessionsUpdateInterceptor.onSave(entity, id, state, propertyNames, types);
         return false;
     }
 
+    @Override
+    public void onDelete(Object entity, Serializable id, Object[] state, String[] propertyNames, Type[] types)
+    {
+        sessionsUpdateInterceptor.onDelete(entity, id, state, propertyNames, types);
+    }
+
     // This method is only overriden in dynamic property interceptor
     @Override
     public void afterTransactionCompletion(Transaction tx)
     {
         dynamicPropertiesInterceptor.afterTransactionCompletion(tx);
+        sessionsUpdateInterceptor.afterTransactionCompletion(tx);
     }
 
     // This method is only overriden in entity validation interceptor
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/OpenBISHibernateTransactionManager.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/OpenBISHibernateTransactionManager.java
index 5939d307817..86d8b61cc4c 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/OpenBISHibernateTransactionManager.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/OpenBISHibernateTransactionManager.java
@@ -31,6 +31,8 @@ import org.springframework.orm.hibernate3.SessionHolder;
 import org.springframework.transaction.TransactionDefinition;
 import org.springframework.transaction.support.DefaultTransactionStatus;
 
+import ch.systemsx.cisd.openbis.generic.server.OpenBisSessionManager;
+import ch.systemsx.cisd.openbis.generic.server.SessionsUpdateInterceptor;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.HibernateInterceptorsWrapper;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.IDAOFactory;
 import ch.systemsx.cisd.openbis.generic.server.dataaccess.dynamic_property.IDynamicPropertyCalculatorFactory;
@@ -42,8 +44,7 @@ import ch.systemsx.cisd.openbis.generic.shared.managed_property.IManagedProperty
  * An implementation of {@link HibernateTransactionManager} that:
  * <ul>
  * <li>creates a new EntityValidationInterceptor for each hibernate session,</li>
- * <li>injects (and clears) the connection of the current transaction as default managed database
- * connection into EoDSQL.</li> </li>
+ * <li>injects (and clears) the connection of the current transaction as default managed database connection into EoDSQL.</li> </li>
  * </ul>
  * 
  * @author Jakub Straszewski
@@ -64,15 +65,18 @@ public class OpenBISHibernateTransactionManager extends HibernateTransactionMana
 
     private IManagedPropertyEvaluatorFactory managedPropertyEvaluatorFactory;
 
+    private OpenBisSessionManager openBisSessionManager;
+
     public OpenBISHibernateTransactionManager(IDAOFactory daoFactory,
             IEntityValidatorFactory entityValidationFactory,
             IDynamicPropertyCalculatorFactory dynamicPropertyCalculatorFactory,
-            IManagedPropertyEvaluatorFactory managedPropertyEvaluatorFactory)
+            IManagedPropertyEvaluatorFactory managedPropertyEvaluatorFactory, OpenBisSessionManager openBisSessionManager)
     {
         this.daoFactory = daoFactory;
         this.entityValidationFactory = entityValidationFactory;
         this.dynamicPropertyCalculatorFactory = dynamicPropertyCalculatorFactory;
         this.managedPropertyEvaluatorFactory = managedPropertyEvaluatorFactory;
+        this.openBisSessionManager = openBisSessionManager;
     }
 
     public void setDynamicPropertiesInterceptor(
@@ -121,8 +125,10 @@ public class OpenBISHibernateTransactionManager extends HibernateTransactionMana
                 new EntityValidationInterceptor(this, daoFactory, entityValidationFactory,
                         dynamicPropertyCalculatorFactory, managedPropertyEvaluatorFactory);
 
+        SessionsUpdateInterceptor sessionsUpdateInterceptor = new SessionsUpdateInterceptor(openBisSessionManager, daoFactory);
+
         return new HibernateInterceptorsWrapper(dynamicPropertiesInterceptor,
-                entityValidationInterceptor);
+                entityValidationInterceptor, sessionsUpdateInterceptor);
     }
 
     @Override
@@ -166,9 +172,8 @@ public class OpenBISHibernateTransactionManager extends HibernateTransactionMana
     }
 
     /**
-     * Extracts transaction from spring DefaultTransactionStatus. The inner transaction is protected
-     * by a private static class, and the only way to get it is with the reflection. This can lead
-     * to potential problems with the newer hibernate versions.
+     * Extracts transaction from spring DefaultTransactionStatus. The inner transaction is protected by a private static class, and the only way to
+     * get it is with the reflection. This can lead to potential problems with the newer hibernate versions.
      */
     private Transaction extractTransaction(DefaultTransactionStatus status)
             throws NoSuchMethodException, IllegalAccessException, InvocationTargetException
diff --git a/openbis/source/java/genericApplicationContext.xml b/openbis/source/java/genericApplicationContext.xml
index 56d52123239..f7d9676ac1a 100644
--- a/openbis/source/java/genericApplicationContext.xml
+++ b/openbis/source/java/genericApplicationContext.xml
@@ -50,13 +50,14 @@
         <constructor-arg ref="entity-validation-factory" />
         <constructor-arg ref="dynamic-property-calculator-factory" />
         <constructor-arg ref="managed-property-evaluator-factory" />
+        <constructor-arg ref="session-manager" />
         <property name="sessionFactory" ref="hibernate-session-factory" />
         <property name="jdbcExceptionTranslator" ref="exception-translator" />
         <property name="dynamicPropertiesInterceptor">
             <ref bean="dynamic-properties-interceptor" />
         </property>
     </bean>
-    
+
     <bean id="dynamic-properties-interceptor"
           class="ch.systemsx.cisd.openbis.generic.server.dataaccess.db.DynamicPropertiesInterceptor">
           <property name="dynamicPropertyScheduler" ref="dynamic-property-scheduler" />
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java
index c80e0cb7f75..f3e98de0116 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/CommonServerTest.java
@@ -477,8 +477,6 @@ public final class CommonServerTest extends AbstractServerTestCase
                     one(roleAssignmentTable).add(with(assignmentMatcher));
                     one(roleAssignmentTable).save();
 
-                    one(sessionManager).updateAllSessions(daoFactory);
-
                 }
             });
 
@@ -502,8 +500,6 @@ public final class CommonServerTest extends AbstractServerTestCase
 
                     one(roleAssignmentTable).add(with(assignmentMatcher));
                     one(roleAssignmentTable).save();
-
-                    one(sessionManager).updateAllSessions(daoFactory);
                 }
             });
 
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
index 4edcad6437b..df2b3649ea2 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/ETLServiceTest.java
@@ -1179,8 +1179,6 @@ public class ETLServiceTest extends AbstractServerTestCase
                     one(roleAssignmentTable).add(with(roleMatcher));
                     one(roleAssignmentTable).save();
 
-                    one(sessionManager).updateAllSessions(daoFactory);
-
                     allowing(personDAO).tryFindPersonByUserId(USER_FOR_ENTITY_OPERATIONS);
                     PersonPE user = createSystemUser();
                     will(returnValue(user));
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/CommonServerTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/CommonServerTest.java
index f69f8fbca47..9daf0db17a0 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/CommonServerTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/CommonServerTest.java
@@ -22,98 +22,23 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
-import org.apache.commons.collections.CollectionUtils;
-import org.apache.commons.collections.Predicate;
-import org.springframework.transaction.annotation.Propagation;
-import org.springframework.transaction.annotation.Transactional;
-import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
 import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Attachment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ColumnSetting;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ContainerDataSet;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.EntityTypePropertyType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Grantee;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Space;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.displaysettings.ColumnDisplaySettingsUpdate;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.DatabaseInstanceIdentifier;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
 
 /**
  * @author Franz-Josef Elmer
  */
-@Transactional(propagation = Propagation.NOT_SUPPORTED)
 public class CommonServerTest extends SystemTestCase
 {
-    @Test
-    public void testRoleAssingmentDeleted()
-    {
-        String sessionTokenForInstanceAdmin = commonServer.tryAuthenticate("test", "a").getSessionToken();
-        String sessionTokenForSpaceAdmin = commonServer.tryAuthenticate("test_space", "a").getSessionToken();
-
-        // reproduce
-
-        commonServer.deleteSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
-                new SpaceIdentifier("TEST-SPACE"), Grantee.createPerson("test_space"));
-
-        commonServer.updateDisplaySettings(sessionTokenForSpaceAdmin,
-                new ColumnDisplaySettingsUpdate("id_a_b_C", Collections.<ColumnSetting> emptyList()));
-        // clean up
-        commonServer.registerSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
-                new SpaceIdentifier("TEST-SPACE"), Grantee.createPerson("test_space"));
-    }
-
-    @Test
-    public void testRoleAssignmentAdded()
-    {
-        String spaceCode = "TESTGROUP";
-
-        String sessionTokenForInstanceAdmin = commonServer.tryAuthenticate("test", "a").getSessionToken();
-        String sessionTokenForSpaceAdmin = commonServer.tryAuthenticate("test_space", "a").getSessionToken();
-
-        List<Space> spaces = commonServer.listSpaces(sessionTokenForSpaceAdmin, DatabaseInstanceIdentifier.createHome());
-        int matchingSpaces = containstSpace(spaces, spaceCode);
-        assertEquals(spaceCode + " should not be in test_space user groups before the role assignment" + spaces, 0, matchingSpaces);
-
-        commonServer.registerSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
-                new SpaceIdentifier(spaceCode), Grantee.createPerson("test_space"));
-
-        spaces = commonServer.listSpaces(sessionTokenForSpaceAdmin, DatabaseInstanceIdentifier.createHome());
-        matchingSpaces = containstSpace(spaces, spaceCode);
-        assertEquals("Couldn't find " + spaceCode + " space in " + spaces, 1, matchingSpaces);
-
-        // cleanup
-
-        commonServer.deleteSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
-                new SpaceIdentifier(spaceCode), Grantee.createPerson("test_space"));
-
-    }
-
-    private int containstSpace(List<Space> spaces, final String spaceCode)
-    {
-        int matchingSpaces = CollectionUtils.countMatches(spaces, new Predicate<Space>()
-            {
-                @Override
-                public boolean evaluate(Space object)
-                {
-                    return object.getCode().equals(spaceCode);
-                }
-
-            });
-        return matchingSpaces;
-    }
-
-    @AfterClass
-    public void cleanup()
-    {
-    }
 
     @Test
     public void testGetSampleWithAssignedPropertyTypesAndProperties()
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationBuilder.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationBuilder.java
new file mode 100644
index 00000000000..fad65ba6265
--- /dev/null
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationBuilder.java
@@ -0,0 +1,317 @@
+/*
+ * Copyright 2013 ETH Zuerich, CISD
+ *
+ * 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 ch.systemsx.cisd.openbis.systemtest;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+
+import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetBatchUpdateDetails;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Grantee;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Material;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewMaterial;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewMetaproject;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewProject;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSpace;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PhysicalDataSet;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Sample;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode;
+import ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails;
+import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetBatchUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.MaterialUpdateDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.MetaprojectUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.NewExternalData;
+import ch.systemsx.cisd.openbis.generic.shared.dto.NewProperty;
+import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SampleUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.SpaceRoleAssignment;
+import ch.systemsx.cisd.openbis.generic.shared.dto.StorageFormat;
+import ch.systemsx.cisd.openbis.generic.shared.dto.VocabularyUpdatesDTO;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifierFactory;
+import ch.systemsx.cisd.openbis.generic.shared.translator.MaterialTranslator;
+
+public final class EntityOperationBuilder
+{
+    private static long counter = 1000;
+
+    private final List<NewSpace> spaces = new ArrayList<NewSpace>();
+
+    private final List<NewProject> projects = new ArrayList<NewProject>();
+
+    private final List<ProjectUpdatesDTO> projectUpdates = new ArrayList<ProjectUpdatesDTO>();
+
+    private final List<NewExperiment> experiments = new ArrayList<NewExperiment>();
+
+    private final List<ExperimentUpdatesDTO> experimentUpdates =
+            new ArrayList<ExperimentUpdatesDTO>();
+
+    private final List<NewSample> samples = new ArrayList<NewSample>();
+
+    private final List<SampleUpdatesDTO> sampleUpdates = new ArrayList<SampleUpdatesDTO>();
+
+    private final List<NewExternalData> dataSets = new ArrayList<NewExternalData>();
+
+    private final List<DataSetBatchUpdatesDTO> dataSetUpdates =
+            new ArrayList<DataSetBatchUpdatesDTO>();
+
+    private final Map<String, List<NewMaterial>> materials =
+            new HashMap<String, List<NewMaterial>>();
+
+    private final List<MaterialUpdateDTO> materialUpdates = new ArrayList<MaterialUpdateDTO>();
+
+    private final List<NewMetaproject> metaprojectRegistrations =
+            new ArrayList<NewMetaproject>();
+
+    private final List<MetaprojectUpdatesDTO> metaprojectUpdates =
+            new ArrayList<MetaprojectUpdatesDTO>();
+
+    private final List<VocabularyUpdatesDTO> vocabularyUpdates =
+            new ArrayList<VocabularyUpdatesDTO>();
+
+    private final List<SpaceRoleAssignment> spaceRoleAssignments = new ArrayList<SpaceRoleAssignment>();
+
+    private final List<SpaceRoleAssignment> spaceRoleRevocations = new ArrayList<SpaceRoleAssignment>();
+
+    private TechId registrationID = new TechId(counter++);
+
+    private String userID;
+
+    EntityOperationBuilder user(String user)
+    {
+        this.userID = user;
+        return this;
+    }
+
+    EntityOperationBuilder space(String code)
+    {
+        return space(new NewSpace(code, null, null));
+    }
+
+    EntityOperationBuilder space(String code, String user)
+    {
+        return space(new NewSpace(code, null, user));
+    }
+
+    EntityOperationBuilder space(NewSpace space)
+    {
+        spaces.add(space);
+        return this;
+    }
+
+    EntityOperationBuilder material(String materialTypeCode, Material material)
+    {
+        List<NewMaterial> list = materials.get(materialTypeCode);
+        if (list == null)
+        {
+            list = new ArrayList<NewMaterial>();
+            materials.put(materialTypeCode, list);
+        }
+        list.add(MaterialTranslator.translateToNewMaterial(material));
+        return this;
+    }
+
+    EntityOperationBuilder project(SpaceIdentifier spaceIdentifier, String projectCode)
+    {
+        String projectIdentifier =
+                new ProjectIdentifier(spaceIdentifier, projectCode).toString();
+        return project(new NewProject(projectIdentifier, null));
+    }
+
+    EntityOperationBuilder project(NewProject project)
+    {
+        projects.add(project);
+        return this;
+    }
+
+    @SuppressWarnings("unused")
+    EntityOperationBuilder project(ProjectUpdatesDTO project)
+    {
+        projectUpdates.add(project);
+        return this;
+    }
+
+    EntityOperationBuilder experiment(Experiment experiment)
+    {
+        NewExperiment newExperiment =
+                new NewExperiment(experiment.getIdentifier(), experiment.getEntityType()
+                        .getCode());
+        newExperiment.setPermID(experiment.getPermId());
+        newExperiment.setProperties(experiment.getProperties().toArray(new IEntityProperty[0]));
+        experiments.add(newExperiment);
+        return this;
+    }
+
+    EntityOperationBuilder sample(Sample sample)
+    {
+        NewSample newSample = new NewSample();
+        newSample.setIdentifier(sample.getIdentifier());
+        newSample.setSampleType(sample.getSampleType());
+        Experiment experiment = sample.getExperiment();
+        if (experiment != null)
+        {
+            newSample.setExperimentIdentifier(experiment.getIdentifier());
+        }
+        newSample.setProperties(sample.getProperties().toArray(new IEntityProperty[0]));
+        samples.add(newSample);
+        return this;
+    }
+
+    EntityOperationBuilder sampleUpdate(Sample sample)
+    {
+        sampleUpdates.add(new SampleUpdatesDTO(new TechId(sample), sample.getProperties(),
+                null, null, sample.getVersion(), SampleIdentifierFactory.parse(sample
+                        .getIdentifier()), null, null));
+        return this;
+    }
+
+    EntityOperationBuilder dataSet(AbstractExternalData dataSet)
+    {
+        NewExternalData newExternalData = new NewExternalData();
+        newExternalData.setCode(dataSet.getCode());
+        newExternalData.setDataSetType(dataSet.getDataSetType());
+        newExternalData.setDataStoreCode(dataSet.getDataStore().getCode());
+        if (dataSet instanceof PhysicalDataSet)
+        {
+            PhysicalDataSet realDataSet = (PhysicalDataSet) dataSet;
+            newExternalData.setFileFormatType(realDataSet.getFileFormatType());
+            newExternalData.setLocation(realDataSet.getLocation());
+            newExternalData.setLocatorType(realDataSet.getLocatorType());
+        }
+        newExternalData.setStorageFormat(StorageFormat.PROPRIETARY);
+        List<IEntityProperty> properties = dataSet.getProperties();
+        List<NewProperty> newProperties = new ArrayList<NewProperty>();
+        for (IEntityProperty property : properties)
+        {
+            newProperties.add(new NewProperty(property.getPropertyType().getCode(), property
+                    .tryGetAsString()));
+        }
+        newExternalData.setDataSetProperties(newProperties);
+        Sample sample = dataSet.getSample();
+        if (sample != null)
+        {
+            newExternalData.setSampleIdentifierOrNull(SampleIdentifierFactory.parse(sample
+                    .getIdentifier()));
+        }
+        Experiment experiment = dataSet.getExperiment();
+        if (experiment != null)
+        {
+            newExternalData.setExperimentIdentifierOrNull(ExperimentIdentifierFactory
+                    .parse(experiment.getIdentifier()));
+        }
+        dataSets.add(newExternalData);
+        return this;
+    }
+
+    EntityOperationBuilder dataSetUpdate(AbstractExternalData dataSet)
+    {
+        DataSetBatchUpdatesDTO dataSetUpdate = new DataSetBatchUpdatesDTO();
+        dataSetUpdate.setDetails(new DataSetBatchUpdateDetails());
+        dataSetUpdate.setDatasetId(new TechId(dataSet));
+        dataSetUpdate.setDatasetCode(dataSet.getCode());
+        dataSetUpdate.setVersion(dataSet.getVersion());
+        if (dataSet instanceof PhysicalDataSet)
+        {
+            PhysicalDataSet realDataSet = (PhysicalDataSet) dataSet;
+            dataSetUpdate.setFileFormatTypeCode(realDataSet.getFileFormatType().getCode());
+        }
+        dataSetUpdate.setProperties(dataSet.getProperties());
+
+        // Request an update of all properties
+        HashSet<String> propertiesToUpdate = new HashSet<String>();
+        for (IEntityProperty property : dataSet.getProperties())
+        {
+            propertiesToUpdate.add(property.getPropertyType().getCode());
+        }
+        dataSetUpdate.getDetails().setPropertiesToUpdate(propertiesToUpdate);
+
+        Sample sample = dataSet.getSample();
+        if (sample != null)
+        {
+            dataSetUpdate.setSampleIdentifierOrNull(SampleIdentifierFactory.parse(sample
+                    .getIdentifier()));
+        }
+        Experiment experiment = dataSet.getExperiment();
+        if (experiment != null)
+        {
+            dataSetUpdate.setExperimentIdentifierOrNull(ExperimentIdentifierFactory
+                    .parse(experiment.getIdentifier()));
+        }
+        dataSetUpdates.add(dataSetUpdate);
+        return this;
+    }
+
+    EntityOperationBuilder assignRoleToSpace(RoleCode roleCode, String spaceCode, List<String> userIds, List<String> groupCodes)
+    {
+        SpaceRoleAssignment assignment = createSpaceRoleAssignment(roleCode, spaceCode, userIds, groupCodes);
+        spaceRoleAssignments.add(assignment);
+        return this;
+    }
+
+    EntityOperationBuilder revokeRoleFromSpace(RoleCode roleCode, String spaceCode, List<String> userIds, List<String> groupCodes)
+    {
+        SpaceRoleAssignment assignment = createSpaceRoleAssignment(roleCode, spaceCode, userIds, groupCodes);
+        spaceRoleRevocations.add(assignment);
+        return this;
+    }
+
+    private SpaceRoleAssignment createSpaceRoleAssignment(RoleCode roleCode, String spaceIdentifier, List<String> userIds, List<String> groupCodes)
+    {
+        SpaceRoleAssignment assignment = new SpaceRoleAssignment();
+        assignment.setRoleCode(roleCode);
+        assignment.setSpaceIdentifier(new SpaceIdentifierFactory(spaceIdentifier).createIdentifier());
+        ArrayList<Grantee> grantees = new ArrayList<Grantee>();
+        if (userIds != null)
+        {
+            for (String userId : userIds)
+            {
+                grantees.add(Grantee.createPerson(userId));
+            }
+        }
+
+        if (groupCodes != null)
+        {
+            for (String code : groupCodes)
+            {
+                grantees.add(Grantee.createAuthorizationGroup(code));
+            }
+        }
+        assignment.setGrantees(grantees);
+        return assignment;
+    }
+
+    AtomicEntityOperationDetails create()
+    {
+        return new AtomicEntityOperationDetails(registrationID, userID, spaces, projects,
+                projectUpdates, experiments, experimentUpdates, sampleUpdates, samples,
+                materials, materialUpdates, dataSets, dataSetUpdates, metaprojectRegistrations,
+                metaprojectUpdates, vocabularyUpdates, spaceRoleAssignments, spaceRoleRevocations);
+    }
+
+}
\ No newline at end of file
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationTest.java
index 302ff67260b..86d2b59cda1 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/EntityOperationTest.java
@@ -20,13 +20,9 @@ import static org.testng.AssertJUnit.assertEquals;
 import static org.testng.AssertJUnit.assertNotNull;
 import static org.testng.AssertJUnit.fail;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
 
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
@@ -36,20 +32,11 @@ import ch.systemsx.cisd.openbis.generic.shared.IServiceForDataStoreServer;
 import ch.systemsx.cisd.openbis.generic.shared.basic.TechId;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AbstractExternalData;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.AuthorizationGroup;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.DataSetBatchUpdateDetails;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Experiment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Grantee;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.IEntityProperty;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Material;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.MaterialIdentifier;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewExperiment;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewMaterial;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewMetaproject;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewProject;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSample;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.NewSpace;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Person;
-import ch.systemsx.cisd.openbis.generic.shared.basic.dto.PhysicalDataSet;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Project;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleAssignment;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode;
@@ -62,23 +49,10 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.builders.MaterialBuilde
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.builders.SampleBuilder;
 import ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails;
 import ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationResult;
-import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetBatchUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.MaterialUpdateDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.MetaprojectUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.NewExternalData;
-import ch.systemsx.cisd.openbis.generic.shared.dto.NewProperty;
-import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.SampleUpdatesDTO;
-import ch.systemsx.cisd.openbis.generic.shared.dto.SpaceRoleAssignment;
-import ch.systemsx.cisd.openbis.generic.shared.dto.StorageFormat;
-import ch.systemsx.cisd.openbis.generic.shared.dto.VocabularyUpdatesDTO;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ProjectIdentifier;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SampleIdentifierFactory;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
-import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifierFactory;
-import ch.systemsx.cisd.openbis.generic.shared.translator.MaterialTranslator;
 
 /**
  * System tests for {@link IServiceForDataStoreServer#performEntityOperations(String, AtomicEntityOperationDetails)}
@@ -108,253 +82,6 @@ public class EntityOperationTest extends SystemTestCase
 
     private static final SpaceIdentifier SPACE_B = new SpaceIdentifier("CISD", "TESTGROUP");
 
-    private static final class EntityOperationBuilder
-    {
-        private static long counter = 1000;
-
-        private final List<NewSpace> spaces = new ArrayList<NewSpace>();
-
-        private final List<NewProject> projects = new ArrayList<NewProject>();
-
-        private final List<ProjectUpdatesDTO> projectUpdates = new ArrayList<ProjectUpdatesDTO>();
-
-        private final List<NewExperiment> experiments = new ArrayList<NewExperiment>();
-
-        private final List<ExperimentUpdatesDTO> experimentUpdates =
-                new ArrayList<ExperimentUpdatesDTO>();
-
-        private final List<NewSample> samples = new ArrayList<NewSample>();
-
-        private final List<SampleUpdatesDTO> sampleUpdates = new ArrayList<SampleUpdatesDTO>();
-
-        private final List<NewExternalData> dataSets = new ArrayList<NewExternalData>();
-
-        private final List<DataSetBatchUpdatesDTO> dataSetUpdates =
-                new ArrayList<DataSetBatchUpdatesDTO>();
-
-        private final Map<String, List<NewMaterial>> materials =
-                new HashMap<String, List<NewMaterial>>();
-
-        private final List<MaterialUpdateDTO> materialUpdates = new ArrayList<MaterialUpdateDTO>();
-
-        private final List<NewMetaproject> metaprojectRegistrations =
-                new ArrayList<NewMetaproject>();
-
-        private final List<MetaprojectUpdatesDTO> metaprojectUpdates =
-                new ArrayList<MetaprojectUpdatesDTO>();
-
-        private final List<VocabularyUpdatesDTO> vocabularyUpdates =
-                new ArrayList<VocabularyUpdatesDTO>();
-
-        private final List<SpaceRoleAssignment> spaceRoleAssignments = new ArrayList<SpaceRoleAssignment>();
-
-        private final List<SpaceRoleAssignment> spaceRoleRevocations = new ArrayList<SpaceRoleAssignment>();
-
-        private TechId registrationID = new TechId(counter++);
-
-        private String userID;
-
-        EntityOperationBuilder user(String user)
-        {
-            this.userID = user;
-            return this;
-        }
-
-        EntityOperationBuilder space(String code)
-        {
-            return space(new NewSpace(code, null, null));
-        }
-
-        EntityOperationBuilder space(NewSpace space)
-        {
-            spaces.add(space);
-            return this;
-        }
-
-        EntityOperationBuilder material(String materialTypeCode, Material material)
-        {
-            List<NewMaterial> list = materials.get(materialTypeCode);
-            if (list == null)
-            {
-                list = new ArrayList<NewMaterial>();
-                materials.put(materialTypeCode, list);
-            }
-            list.add(MaterialTranslator.translateToNewMaterial(material));
-            return this;
-        }
-
-        EntityOperationBuilder project(SpaceIdentifier spaceIdentifier, String projectCode)
-        {
-            String projectIdentifier =
-                    new ProjectIdentifier(spaceIdentifier, projectCode).toString();
-            return project(new NewProject(projectIdentifier, null));
-        }
-
-        EntityOperationBuilder project(NewProject project)
-        {
-            projects.add(project);
-            return this;
-        }
-
-        @SuppressWarnings("unused")
-        EntityOperationBuilder project(ProjectUpdatesDTO project)
-        {
-            projectUpdates.add(project);
-            return this;
-        }
-
-        EntityOperationBuilder experiment(Experiment experiment)
-        {
-            NewExperiment newExperiment =
-                    new NewExperiment(experiment.getIdentifier(), experiment.getEntityType()
-                            .getCode());
-            newExperiment.setPermID(experiment.getPermId());
-            newExperiment.setProperties(experiment.getProperties().toArray(new IEntityProperty[0]));
-            experiments.add(newExperiment);
-            return this;
-        }
-
-        EntityOperationBuilder sample(Sample sample)
-        {
-            NewSample newSample = new NewSample();
-            newSample.setIdentifier(sample.getIdentifier());
-            newSample.setSampleType(sample.getSampleType());
-            Experiment experiment = sample.getExperiment();
-            if (experiment != null)
-            {
-                newSample.setExperimentIdentifier(experiment.getIdentifier());
-            }
-            newSample.setProperties(sample.getProperties().toArray(new IEntityProperty[0]));
-            samples.add(newSample);
-            return this;
-        }
-
-        EntityOperationBuilder sampleUpdate(Sample sample)
-        {
-            sampleUpdates.add(new SampleUpdatesDTO(new TechId(sample), sample.getProperties(),
-                    null, null, sample.getVersion(), SampleIdentifierFactory.parse(sample
-                            .getIdentifier()), null, null));
-            return this;
-        }
-
-        EntityOperationBuilder dataSet(AbstractExternalData dataSet)
-        {
-            NewExternalData newExternalData = new NewExternalData();
-            newExternalData.setCode(dataSet.getCode());
-            newExternalData.setDataSetType(dataSet.getDataSetType());
-            newExternalData.setDataStoreCode(dataSet.getDataStore().getCode());
-            if (dataSet instanceof PhysicalDataSet)
-            {
-                PhysicalDataSet realDataSet = (PhysicalDataSet) dataSet;
-                newExternalData.setFileFormatType(realDataSet.getFileFormatType());
-                newExternalData.setLocation(realDataSet.getLocation());
-                newExternalData.setLocatorType(realDataSet.getLocatorType());
-            }
-            newExternalData.setStorageFormat(StorageFormat.PROPRIETARY);
-            List<IEntityProperty> properties = dataSet.getProperties();
-            List<NewProperty> newProperties = new ArrayList<NewProperty>();
-            for (IEntityProperty property : properties)
-            {
-                newProperties.add(new NewProperty(property.getPropertyType().getCode(), property
-                        .tryGetAsString()));
-            }
-            newExternalData.setDataSetProperties(newProperties);
-            Sample sample = dataSet.getSample();
-            if (sample != null)
-            {
-                newExternalData.setSampleIdentifierOrNull(SampleIdentifierFactory.parse(sample
-                        .getIdentifier()));
-            }
-            Experiment experiment = dataSet.getExperiment();
-            if (experiment != null)
-            {
-                newExternalData.setExperimentIdentifierOrNull(ExperimentIdentifierFactory
-                        .parse(experiment.getIdentifier()));
-            }
-            dataSets.add(newExternalData);
-            return this;
-        }
-
-        EntityOperationBuilder dataSetUpdate(AbstractExternalData dataSet)
-        {
-            DataSetBatchUpdatesDTO dataSetUpdate = new DataSetBatchUpdatesDTO();
-            dataSetUpdate.setDetails(new DataSetBatchUpdateDetails());
-            dataSetUpdate.setDatasetId(new TechId(dataSet));
-            dataSetUpdate.setDatasetCode(dataSet.getCode());
-            dataSetUpdate.setVersion(dataSet.getVersion());
-            if (dataSet instanceof PhysicalDataSet)
-            {
-                PhysicalDataSet realDataSet = (PhysicalDataSet) dataSet;
-                dataSetUpdate.setFileFormatTypeCode(realDataSet.getFileFormatType().getCode());
-            }
-            dataSetUpdate.setProperties(dataSet.getProperties());
-
-            // Request an update of all properties
-            HashSet<String> propertiesToUpdate = new HashSet<String>();
-            for (IEntityProperty property : dataSet.getProperties())
-            {
-                propertiesToUpdate.add(property.getPropertyType().getCode());
-            }
-            dataSetUpdate.getDetails().setPropertiesToUpdate(propertiesToUpdate);
-
-            Sample sample = dataSet.getSample();
-            if (sample != null)
-            {
-                dataSetUpdate.setSampleIdentifierOrNull(SampleIdentifierFactory.parse(sample
-                        .getIdentifier()));
-            }
-            Experiment experiment = dataSet.getExperiment();
-            if (experiment != null)
-            {
-                dataSetUpdate.setExperimentIdentifierOrNull(ExperimentIdentifierFactory
-                        .parse(experiment.getIdentifier()));
-            }
-            dataSetUpdates.add(dataSetUpdate);
-            return this;
-        }
-
-        EntityOperationBuilder assignRoleToSpace(RoleCode roleCode, String spaceCode, List<String> userIds, List<String> groupCodes)
-        {
-            SpaceRoleAssignment assignment = createSpaceRoleAssignment(roleCode, spaceCode, userIds, groupCodes);
-            spaceRoleAssignments.add(assignment);
-            return this;
-        }
-
-        EntityOperationBuilder revokeRoleFromSpace(RoleCode roleCode, String spaceCode, List<String> userIds, List<String> groupCodes)
-        {
-            SpaceRoleAssignment assignment = createSpaceRoleAssignment(roleCode, spaceCode, userIds, groupCodes);
-            spaceRoleRevocations.add(assignment);
-            return this;
-        }
-
-        private SpaceRoleAssignment createSpaceRoleAssignment(RoleCode roleCode, String spaceIdentifier, List<String> userIds, List<String> groupCodes)
-        {
-            SpaceRoleAssignment assignment = new SpaceRoleAssignment();
-            assignment.setRoleCode(roleCode);
-            assignment.setSpaceIdentifier(new SpaceIdentifierFactory(spaceIdentifier).createIdentifier());
-            ArrayList<Grantee> grantees = new ArrayList<Grantee>();
-            for (String userId : userIds)
-            {
-                grantees.add(Grantee.createPerson(userId));
-            }
-            for (String code : groupCodes)
-            {
-                grantees.add(Grantee.createAuthorizationGroup(code));
-            }
-            assignment.setGrantees(grantees);
-            return assignment;
-        }
-
-        AtomicEntityOperationDetails create()
-        {
-            return new AtomicEntityOperationDetails(registrationID, userID, spaces, projects,
-                    projectUpdates, experiments, experimentUpdates, sampleUpdates, samples,
-                    materials, materialUpdates, dataSets, dataSetUpdates, metaprojectRegistrations,
-                    metaprojectUpdates, vocabularyUpdates, spaceRoleAssignments, spaceRoleRevocations);
-        }
-
-    }
-
     @BeforeClass
     public void createTestUsers()
     {
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/SessionUpdateTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/SessionUpdateTest.java
new file mode 100644
index 00000000000..337a8f06fd2
--- /dev/null
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/SessionUpdateTest.java
@@ -0,0 +1,136 @@
+/*
+ * Copyright 2013 ETH Zuerich, CISD
+ *
+ * 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 ch.systemsx.cisd.openbis.systemtest;
+
+import static org.testng.AssertJUnit.assertEquals;
+import static org.testng.AssertJUnit.fail;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
+
+import org.apache.commons.collections.CollectionUtils;
+import org.apache.commons.collections.Predicate;
+import org.springframework.transaction.annotation.Propagation;
+import org.springframework.transaction.annotation.Transactional;
+import org.testng.annotations.Test;
+
+import ch.systemsx.cisd.common.exceptions.UserFailureException;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.ColumnSetting;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Grantee;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy.RoleCode;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Space;
+import ch.systemsx.cisd.openbis.generic.shared.basic.dto.displaysettings.ColumnDisplaySettingsUpdate;
+import ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.DatabaseInstanceIdentifier;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.SpaceIdentifier;
+
+/**
+ * @author Jakub Straszewski
+ */
+@Transactional(propagation = Propagation.NOT_SUPPORTED)
+public class SessionUpdateTest extends SystemTestCase
+{
+
+    private static final String ADMIN = "test";
+
+    @Test
+    public void testCreateSpaceForUserAndAssignIdenticalRoleFailsWithNoSideeffect()
+    {
+        String sessionToken = authenticateAs(ADMIN);
+        String sessionTokenUser = authenticateAs("test_space");
+
+        commonServer.tryGetSession(sessionTokenUser);
+
+        String spaceIdentifier = new SpaceIdentifier("TEST_SPACE_1").toString();
+
+        AtomicEntityOperationDetails eo =
+                new EntityOperationBuilder().user(ADMIN).space("TEST_SPACE_1", "test_space").
+                        assignRoleToSpace(RoleCode.ADMIN, spaceIdentifier, Arrays.asList("test_space"), null).create();
+
+        try
+        {
+            etlService.performEntityOperations(sessionToken, eo);
+            fail("Exception expected");
+        } catch (UserFailureException ufe)
+        {
+            // this is expected
+        }
+
+        commonServer.updateDisplaySettings(sessionTokenUser,
+                new ColumnDisplaySettingsUpdate("id_a_b_C", Collections.<ColumnSetting> emptyList()));
+    }
+
+    @Test
+    public void testRoleAssingmentDeleted()
+    {
+        String sessionTokenForInstanceAdmin = commonServer.tryAuthenticate("test", "a").getSessionToken();
+        String sessionTokenForSpaceAdmin = commonServer.tryAuthenticate("test_space", "a").getSessionToken();
+
+        // reproduce
+
+        commonServer.deleteSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
+                new SpaceIdentifier("TEST-SPACE"), Grantee.createPerson("test_space"));
+
+        commonServer.updateDisplaySettings(sessionTokenForSpaceAdmin,
+                new ColumnDisplaySettingsUpdate("id_a_b_C", Collections.<ColumnSetting> emptyList()));
+        // clean up
+        commonServer.registerSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
+                new SpaceIdentifier("TEST-SPACE"), Grantee.createPerson("test_space"));
+    }
+
+    @Test
+    public void testRoleAssignmentAdded()
+    {
+        String spaceCode = "TESTGROUP";
+
+        String sessionTokenForInstanceAdmin = commonServer.tryAuthenticate("test", "a").getSessionToken();
+        String sessionTokenForSpaceAdmin = commonServer.tryAuthenticate("test_space", "a").getSessionToken();
+
+        List<Space> spaces = commonServer.listSpaces(sessionTokenForSpaceAdmin, DatabaseInstanceIdentifier.createHome());
+        int matchingSpaces = containstSpace(spaces, spaceCode);
+        assertEquals(spaceCode + " should not be in test_space user groups before the role assignment" + spaces, 0, matchingSpaces);
+
+        commonServer.registerSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
+                new SpaceIdentifier(spaceCode), Grantee.createPerson("test_space"));
+
+        spaces = commonServer.listSpaces(sessionTokenForSpaceAdmin, DatabaseInstanceIdentifier.createHome());
+        matchingSpaces = containstSpace(spaces, spaceCode);
+        assertEquals("Couldn't find " + spaceCode + " space in spaces of test_space user. Found only " + spaces, 1, matchingSpaces);
+
+        // cleanup
+
+        commonServer.deleteSpaceRole(sessionTokenForInstanceAdmin, RoleCode.ADMIN,
+                new SpaceIdentifier(spaceCode), Grantee.createPerson("test_space"));
+
+    }
+
+    private int containstSpace(List<Space> spaces, final String spaceCode)
+    {
+        int matchingSpaces = CollectionUtils.countMatches(spaces, new Predicate<Space>()
+            {
+                @Override
+                public boolean evaluate(Space object)
+                {
+                    return object.getCode().equals(spaceCode);
+                }
+
+            });
+        return matchingSpaces;
+    }
+
+}
-- 
GitLab