From 9ce41a75d440cfd5c97bb288ba9901a15bd4a36a Mon Sep 17 00:00:00 2001
From: felmer <franz-josef.elmer@id.ethz.ch>
Date: Thu, 14 Nov 2019 09:28:28 +0100
Subject: [PATCH] SSDM-4600: GetRightsExecutor refactored. Checking experiment
creation right implemented. tests added to GetRightsTest
---
.../v3/executor/rights/GetRightsExecutor.java | 237 ++++++++++++------
.../systemtest/asapi/v3/GetRightsTest.java | 54 ++--
2 files changed, 194 insertions(+), 97 deletions(-)
diff --git a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/rights/GetRightsExecutor.java b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/rights/GetRightsExecutor.java
index d0756a2fc6f..77bcd6fdfe3 100644
--- a/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/rights/GetRightsExecutor.java
+++ b/openbis/source/java/ch/ethz/sis/openbis/generic/server/asapi/v3/executor/rights/GetRightsExecutor.java
@@ -18,8 +18,11 @@ package ch.ethz.sis.openbis.generic.server.asapi.v3.executor.rights;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
@@ -29,9 +32,11 @@ import org.apache.commons.lang3.StringUtils;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
-import ch.ethz.sis.openbis.generic.asapi.v3.dto.common.id.CreationId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.common.id.IObjectId;
+import ch.ethz.sis.openbis.generic.asapi.v3.dto.dataset.id.DataSetPermId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.dataset.id.IDataSetId;
+import ch.ethz.sis.openbis.generic.asapi.v3.dto.experiment.id.ExperimentIdentifier;
+import ch.ethz.sis.openbis.generic.asapi.v3.dto.experiment.id.ExperimentPermId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.experiment.id.IExperimentId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.project.id.ProjectIdentifier;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.rights.Right;
@@ -59,6 +64,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SamplePE;
import ch.systemsx.cisd.openbis.generic.shared.dto.SpacePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifierFactory;
/**
* @author Franz-Josef Elmer
@@ -94,14 +100,19 @@ public class GetRightsExecutor implements IGetRightsExecutor
public Map<IObjectId, Rights> getRights(IOperationContext context, List<? extends IObjectId> objectIds, RightsFetchOptions fetchOptions)
{
Map<IObjectId, Rights> result = new HashMap<>();
- List<IHandler> handlers = createHandlers();
+ Map<Class<? extends IObjectId>, IHandler> handlersByObjectIdClass = getHandlersByObjectIdClassMap();
for (IObjectId id : objectIds)
{
- for (IHandler handler : handlers)
+ if (id != null)
{
- handler.handle(id);
+ IHandler handler = handlersByObjectIdClass.get(id.getClass());
+ if (handler != null)
+ {
+ handler.handle(id);
+ }
}
}
+ Set<IHandler> handlers = new LinkedHashSet<>(handlersByObjectIdClass.values());
for (IHandler handler : handlers)
{
handler.addRights(context, result);
@@ -109,9 +120,21 @@ public class GetRightsExecutor implements IGetRightsExecutor
return result;
}
- private List<IHandler> createHandlers()
+ private Map<Class<? extends IObjectId>, IHandler> getHandlersByObjectIdClassMap()
{
- return Arrays.asList(new SampleHandler(), new ExperimentHandler(), new DataSetHandler());
+ Map<Class<? extends IObjectId>, IHandler> map = new LinkedHashMap<>();
+
+ IHandler sampleHandler = new SampleHandler();
+ map.put(SampleIdentifier.class, sampleHandler);
+ map.put(SamplePermId.class, sampleHandler);
+
+ IHandler experimentHandler = new ExperimentHandler();
+ map.put(ExperimentIdentifier.class, experimentHandler);
+ map.put(ExperimentPermId.class, experimentHandler);
+
+ IHandler dataSetHandler = new DataSetHandler();
+ map.put(DataSetPermId.class, dataSetHandler);
+ return map;
}
private static interface IHandler
@@ -121,48 +144,55 @@ public class GetRightsExecutor implements IGetRightsExecutor
void addRights(IOperationContext context, Map<IObjectId, Rights> rightsByIds);
}
- private class SampleHandler implements IHandler
+ private static abstract class AbstractHandler<ID extends IObjectId, ENTITY> implements IHandler
{
- private List<ISampleId> sampleIds = new ArrayList<>();
+ private Class<ID> idClass;
+
+ private List<ID> ids = new ArrayList<>();
+
+ AbstractHandler(Class<ID> idClass)
+ {
+ this.idClass = idClass;
+ }
+ @SuppressWarnings("unchecked")
@Override
public void handle(IObjectId id)
{
- if (id instanceof ISampleId)
+ if (idClass.isAssignableFrom(id.getClass()))
{
- sampleIds.add((ISampleId) id);
+ ids.add((ID) id);
}
}
@Override
public void addRights(IOperationContext context, Map<IObjectId, Rights> rightsByIds)
{
- Map<ISampleId, SamplePE> map = mapSampleByIdExecutor.map(context, sampleIds);
- Set<ISampleId> unknownSamples = new HashSet<>(sampleIds);
- for (Entry<ISampleId, SamplePE> entry : map.entrySet())
+ Map<ID, ENTITY> entitiesByIds = getEntitiesByIds(context, ids);
+ Set<ID> unknownIds = new HashSet<>(ids);
+ for (Entry<ID, ENTITY> entry : entitiesByIds.entrySet())
{
Set<Right> rights = new HashSet<>();
- ISampleId id = entry.getKey();
- SamplePE object = entry.getValue();
-
+ ID id = entry.getKey();
+ ENTITY entity = entry.getValue();
try
{
- sampleAuthorizationExecutor.canUpdate(context, id, object);
+ canUpdate(context, id, entity);
rights.add(Right.UPDATE);
} catch (AuthorizationFailureException e)
{
// silently ignored
}
rightsByIds.put(id, new Rights(rights));
- unknownSamples.remove(id);
+ unknownIds.remove(id);
}
- for (ISampleId id : unknownSamples)
+ for (ID id : unknownIds)
{
Set<Right> rights = new HashSet<>();
try
{
- SamplePE sample = createDummySample(context, id);
- sampleAuthorizationExecutor.canCreate(context, sample);
+ ENTITY entity = createDummyEntity(context, id);
+ canCreate(context, entity);
rights.add(Right.CREATE);
} catch (AuthorizationFailureException e)
{
@@ -171,95 +201,142 @@ public class GetRightsExecutor implements IGetRightsExecutor
rightsByIds.put(id, new Rights(rights));
}
}
+
+ abstract Map<ID, ENTITY> getEntitiesByIds(IOperationContext context, Collection<ID> ids);
+
+ abstract void canUpdate(IOperationContext context, ID id, ENTITY entity);
+
+ abstract ENTITY createDummyEntity(IOperationContext context, ID id);
+
+ abstract void canCreate(IOperationContext context, ENTITY entity);
+
}
- private SamplePE createDummySample(IOperationContext context, ISampleId sampleId)
+ private class SampleHandler extends AbstractHandler<ISampleId, SamplePE>
{
- if (sampleId == null)
- {
- throw new UserFailureException("Unspecified sample id.");
- }
- if (sampleId instanceof CreationId)
+ SampleHandler()
{
- throw new UserFailureException("Sample id '" + sampleId + "' can not be a CreationId.");
+ super(ISampleId.class);
}
- if (sampleId instanceof SamplePermId)
+
+ @Override
+ Map<ISampleId, SamplePE> getEntitiesByIds(IOperationContext context, Collection<ISampleId> ids)
{
- throw new UserFailureException("Sample id '" + sampleId + "' can not be a SamplePermId.");
+ return mapSampleByIdExecutor.map(context, ids);
}
- if (sampleId instanceof SampleIdentifier == false)
+
+ @Override
+ void canUpdate(IOperationContext context, ISampleId id, SamplePE entity)
{
- throw new UserFailureException("Sample id '" + sampleId + "' is of unknown type "
- + sampleId.getClass().getName() + ".");
+ sampleAuthorizationExecutor.canUpdate(context, id, entity);
}
- SpacePE homeSpace = context.getSession().tryGetHomeGroup();
- FullSampleIdentifier sampleIdentifier = new FullSampleIdentifier(((SampleIdentifier) sampleId).getIdentifier(),
- homeSpace == null ? null : homeSpace.getCode());
- SampleIdentifierParts parts = sampleIdentifier.getParts();
- SamplePE samplePE = new SamplePE();
- samplePE.setCode(sampleIdentifier.getSampleCode());
- String spaceCode = parts.getSpaceCodeOrNull();
- if (StringUtils.isNotBlank(spaceCode))
+
+ @Override
+ SamplePE createDummyEntity(IOperationContext context, ISampleId sampleId)
{
- SpacePermId spacePermId = new SpacePermId(spaceCode);
- SpacePE spacePE = mapSpaceByIdExecutor.map(context, Arrays.asList(spacePermId)).get(spacePermId);
- if (spacePE == null)
+ if (sampleId instanceof SamplePermId)
{
- throw new UserFailureException("Unknown space in sample identifier '" + sampleId + "'.");
+ throw new UserFailureException("Unknown sample with perm id " + sampleId + ".");
}
- samplePE.setSpace(spacePE);
- }
- String projectCode = parts.getProjectCodeOrNull();
- if (StringUtils.isNotBlank(projectCode))
- {
- if (StringUtils.isBlank(spaceCode))
+ if (sampleId instanceof SampleIdentifier == false)
{
- throw new UserFailureException("Unknown space in sample identifier '" + sampleId + "'.");
+ throw new UserFailureException("Sample identifier of unsupported type ("
+ + sampleId.getClass().getName() + "): " + sampleId);
}
- ProjectIdentifier projectIdentifier = new ProjectIdentifier(spaceCode, projectCode);
- ProjectPE projectPE = mapProjectByIdExecutor.map(context, Arrays.asList(projectIdentifier)).get(projectIdentifier);
- if (projectPE == null)
+ SpacePE homeSpace = context.getSession().tryGetHomeGroup();
+ FullSampleIdentifier sampleIdentifier = new FullSampleIdentifier(((SampleIdentifier) sampleId).getIdentifier(),
+ homeSpace == null ? null : homeSpace.getCode());
+ SampleIdentifierParts parts = sampleIdentifier.getParts();
+ SamplePE samplePE = new SamplePE();
+ samplePE.setCode(sampleIdentifier.getSampleCode());
+ String spaceCode = parts.getSpaceCodeOrNull();
+ if (StringUtils.isNotBlank(spaceCode))
{
- throw new UserFailureException("Unknown project in sample identifier '" + sampleId + "'.");
+ SpacePermId spacePermId = new SpacePermId(spaceCode);
+ SpacePE spacePE = mapSpaceByIdExecutor.map(context, Arrays.asList(spacePermId)).get(spacePermId);
+ if (spacePE == null)
+ {
+ throw new UserFailureException("Unknown space in sample identifier '" + sampleId + "'.");
+ }
+ samplePE.setSpace(spacePE);
+ }
+ String projectCode = parts.getProjectCodeOrNull();
+ if (StringUtils.isNotBlank(projectCode))
+ {
+ if (StringUtils.isBlank(spaceCode))
+ {
+ throw new UserFailureException("Unknown space in sample identifier '" + sampleId + "'.");
+ }
+ ProjectIdentifier projectIdentifier = new ProjectIdentifier(spaceCode, projectCode);
+ ProjectPE projectPE = mapProjectByIdExecutor.map(context, Arrays.asList(projectIdentifier)).get(projectIdentifier);
+ if (projectPE == null)
+ {
+ throw new UserFailureException("Unknown project in sample identifier '" + sampleId + "'.");
+ }
+ samplePE.setProject(projectPE);
}
- samplePE.setProject(projectPE);
+ return samplePE;
+ }
+
+ @Override
+ void canCreate(IOperationContext context, SamplePE sample)
+ {
+ sampleAuthorizationExecutor.canCreate(context, sample);
}
- return samplePE;
}
- private class ExperimentHandler implements IHandler
+ private class ExperimentHandler extends AbstractHandler<IExperimentId, ExperimentPE>
{
- private List<IExperimentId> experimentIds = new ArrayList<>();
+ ExperimentHandler()
+ {
+ super(IExperimentId.class);
+ }
@Override
- public void handle(IObjectId id)
+ Map<IExperimentId, ExperimentPE> getEntitiesByIds(IOperationContext context, Collection<IExperimentId> ids)
{
- if (id instanceof IExperimentId)
- {
- experimentIds.add((IExperimentId) id);
- }
+ return mapExperimentByIdExecutor.map(context, ids);
}
@Override
- public void addRights(IOperationContext context, Map<IObjectId, Rights> rightsByIds)
+ void canUpdate(IOperationContext context, IExperimentId id, ExperimentPE entity)
+ {
+ experimentAuthorizationExecutor.canUpdate(context, id, entity);
+ }
+
+ @Override
+ ExperimentPE createDummyEntity(IOperationContext context, IExperimentId id)
{
- Map<IExperimentId, ExperimentPE> map = mapExperimentByIdExecutor.map(context, experimentIds);
- for (Entry<IExperimentId, ExperimentPE> entry : map.entrySet())
+ if (id instanceof ExperimentPermId)
{
- Set<Right> rights = new HashSet<>();
- IExperimentId id = entry.getKey();
- ExperimentPE object = entry.getValue();
+ throw new UserFailureException("Unknown experiment with perm id " + id + ".");
+ }
+ if (id instanceof ExperimentIdentifier == false)
+ {
+ throw new UserFailureException("Experiment identifier of unsupported type ("
+ + id.getClass().getName() + "): " + id);
+ }
- try
- {
- experimentAuthorizationExecutor.canUpdate(context, id, object);
- rights.add(Right.UPDATE);
- } catch (AuthorizationFailureException e)
- {
- // silently ignored
- }
- rightsByIds.put(id, new Rights(rights));
+ ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier expeId =
+ ExperimentIdentifierFactory.parse(((ExperimentIdentifier) id).getIdentifier());
+
+ ProjectIdentifier projectIdentifier = new ProjectIdentifier(expeId.getSpaceCode(), expeId.getProjectCode());
+ ProjectPE projectPE = mapProjectByIdExecutor.map(context, Arrays.asList(projectIdentifier)).get(projectIdentifier);
+ if (projectPE == null)
+ {
+ throw new UserFailureException("Unknown project in experiment identifier '" + id + "'.");
}
+ ExperimentPE experimentPE = new ExperimentPE();
+ experimentPE.setProject(projectPE);
+ experimentPE.setCode(expeId.getExperimentCode());
+ return experimentPE;
+ }
+
+ @Override
+ void canCreate(IOperationContext context, ExperimentPE entity)
+ {
+ experimentAuthorizationExecutor.canCreate(context, entity);
+
}
}
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetRightsTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetRightsTest.java
index 3d393cd59c4..d8bdb27da09 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetRightsTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/GetRightsTest.java
@@ -27,6 +27,7 @@ import ch.ethz.sis.openbis.generic.asapi.v3.dto.common.id.CreationId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.common.id.IObjectId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.dataset.id.DataSetPermId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.experiment.id.ExperimentIdentifier;
+import ch.ethz.sis.openbis.generic.asapi.v3.dto.experiment.id.ExperimentPermId;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.rights.Rights;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.rights.fetchoptions.RightsFetchOptions;
import ch.ethz.sis.openbis.generic.asapi.v3.dto.sample.id.SampleIdentifier;
@@ -47,9 +48,10 @@ public class GetRightsTest extends AbstractTest
IObjectId s3 = new SampleIdentifier("/CISD/NEW");
IObjectId s4 = new SampleIdentifier("/CISD/NEMO/NEW");
IObjectId s5 = new SampleIdentifier("/TEST-SPACE/NOE/NEW");
+ IObjectId s6 = new CreationId("123-45");
// When
- Map<IObjectId, Rights> map = v3api.getRights(sessionToken, Arrays.asList(s1, s2, s3, s4, s5), new RightsFetchOptions());
+ Map<IObjectId, Rights> map = v3api.getRights(sessionToken, Arrays.asList(s1, s2, s3, s4, s5, s6), new RightsFetchOptions());
// Then
assertEquals(map.get(s1).getRights().toString(), "[]");
@@ -57,10 +59,11 @@ public class GetRightsTest extends AbstractTest
assertEquals(map.get(s3).getRights().toString(), "[CREATE]");
assertEquals(map.get(s4).getRights().toString(), "[CREATE]");
assertEquals(map.get(s5).getRights().toString(), "[]");
+ assertEquals(map.size(), 5);
}
@Test
- public void testGetSampleCreationRightForSamplePermId()
+ public void testGetSampleCreationRightForUnknownSamplePermId()
{
// Given
String sessionToken = v3api.login(TEST_ROLE_V3, PASSWORD);
@@ -69,20 +72,7 @@ public class GetRightsTest extends AbstractTest
// When
assertUserFailureException(Void -> v3api.getRights(sessionToken, Arrays.asList(s1), new RightsFetchOptions()),
// Then
- "Sample id '123-45' can not be a SamplePermId.");
- }
-
- @Test
- public void testGetSampleCreationRightForCreationId()
- {
- // Given
- String sessionToken = v3api.login(TEST_ROLE_V3, PASSWORD);
- IObjectId s1 = new CreationId("123-45");
-
- // When
- assertUserFailureException(Void -> v3api.getRights(sessionToken, Arrays.asList(s1), new RightsFetchOptions()),
- // Then
- "Sample id '123-45' can not be a CreationId.");
+ "Unknown sample with perm id 123-45.");
}
@Test
@@ -118,13 +108,43 @@ public class GetRightsTest extends AbstractTest
String sessionToken = v3api.login(TEST_ROLE_V3, PASSWORD);
IObjectId s1 = new ExperimentIdentifier("/TEST-SPACE/TEST-PROJECT/EXP-SPACE-TEST");
IObjectId s2 = new ExperimentIdentifier("/CISD/NEMO/EXP1");
+ IObjectId s3 = new ExperimentIdentifier("/CISD/NEMO/NEW");
+ IObjectId s4 = new ExperimentIdentifier("/TEST-SPACE/NOE/NEW");
// When
- Map<IObjectId, Rights> map = v3api.getRights(sessionToken, Arrays.asList(s1, s2), new RightsFetchOptions());
+ Map<IObjectId, Rights> map = v3api.getRights(sessionToken, Arrays.asList(s1, s2, s3, s4), new RightsFetchOptions());
// Then
assertEquals(map.get(s1).getRights().toString(), "[]");
assertEquals(map.get(s2).getRights().toString(), "[UPDATE]");
+ assertEquals(map.get(s3).getRights().toString(), "[CREATE]");
+ assertEquals(map.get(s4).getRights().toString(), "[]");
+ }
+
+ @Test
+ public void testGetExperimentCreationRightForUnknownExperimentPermId()
+ {
+ // Given
+ String sessionToken = v3api.login(TEST_ROLE_V3, PASSWORD);
+ IObjectId s1 = new ExperimentPermId("123-45");
+
+ // When
+ assertUserFailureException(Void -> v3api.getRights(sessionToken, Arrays.asList(s1), new RightsFetchOptions()),
+ // Then
+ "Unknown experiment with perm id 123-45.");
+ }
+
+ @Test
+ public void testGetExperimentCreationRightInMissingProject()
+ {
+ // Given
+ String sessionToken = v3api.login(TEST_ROLE_V3, PASSWORD);
+ IObjectId s1 = new ExperimentIdentifier("/TEST-SPACE/NO-PROJECT/NEW");
+
+ // When
+ assertUserFailureException(Void -> v3api.getRights(sessionToken, Arrays.asList(s1), new RightsFetchOptions()),
+ // Then
+ "Unknown project in experiment identifier '/TEST-SPACE/NO-PROJECT/NEW'.");
}
@Test
--
GitLab