From 4ab3efe584803989877a4d3c9d2d51d3ddbb57a5 Mon Sep 17 00:00:00 2001
From: felmer <felmer>
Date: Mon, 10 Apr 2017 09:53:33 +0000
Subject: [PATCH] SSDM-4927: SampleValidator and SimpleSpaceValidator fixed.
 Tests fixed and extended.

SVN: 38029
---
 .../validator/SampleValidator.java             | 11 +----------
 .../validator/SimpleSpaceValidator.java        |  7 ++++++-
 .../systemtest/asapi/v3/SearchSampleTest.java  |  8 ++++----
 .../api/v1/GeneralInformationServiceTest.java  |  6 +++---
 .../ETLServiceAuthorizationTest.java           | 18 ++++++++++++++++++
 5 files changed, 32 insertions(+), 18 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SampleValidator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SampleValidator.java
index 2ef322d7b7b..230d4d6c3a4 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SampleValidator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SampleValidator.java
@@ -19,7 +19,6 @@ package ch.systemsx.cisd.openbis.generic.server.authorization.validator;
 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.dto.PersonPE;
-import ch.systemsx.cisd.openbis.generic.shared.dto.RoleAssignmentPE;
 
 /**
  * A {@link IValidator} implementation suitable for {@link Sample}.
@@ -49,15 +48,7 @@ public final class SampleValidator extends AbstractValidator<Sample>
             return matchesSpace(person, space);
         } else
         {
-
-            for (RoleAssignmentPE assignment : person.getRoleAssignments())
-            {
-                if (assignment.getSpace() == null)
-                {
-                    return true;
-                }
-            }
-            return false;
+            return person.getRoleAssignments().isEmpty() == false;
         }
     }
 
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SimpleSpaceValidator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SimpleSpaceValidator.java
index 0842bcc92c4..d786ffd5fe2 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SimpleSpaceValidator.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/authorization/validator/SimpleSpaceValidator.java
@@ -33,10 +33,15 @@ public class SimpleSpaceValidator extends AbstractValidator<ICodeHolder>
     public boolean doValidation(PersonPE person, ICodeHolder value)
     {
         final Set<RoleAssignmentPE> roleAssignments = person.getAllPersonRoles();
+        String spaceCode = value.getCode();
+        if (spaceCode == null && roleAssignments.isEmpty() == false)
+        {
+            return true;
+        }
         for (final RoleAssignmentPE roleAssignment : roleAssignments)
         {
             final SpacePE space = roleAssignment.getSpace();
-            if ((space != null && space.getCode().equals(value.getCode()))
+            if ((space != null && space.getCode().equals(spaceCode))
                     || space == null)
             {
                 return true;
diff --git a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/SearchSampleTest.java b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/SearchSampleTest.java
index 0e0473ee358..c3a18e42709 100644
--- a/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/SearchSampleTest.java
+++ b/openbis/sourceTest/java/ch/ethz/sis/openbis/systemtest/asapi/v3/SearchSampleTest.java
@@ -49,11 +49,11 @@ public class SearchSampleTest extends AbstractSampleTest
 {
 
     @Test
-    public void testSearchWithEmptyCriteria()
+    public void testSearchWhichReturnsSharedSamplesForSpaceUser()
     {
-        testSearch(TEST_SPACE_USER, new SampleSearchCriteria(), "/TEST-SPACE/FV-TEST", "/TEST-SPACE/EV-TEST", "/TEST-SPACE/EV-INVALID",
-                "/TEST-SPACE/EV-NOT_INVALID", "/TEST-SPACE/EV-PARENT", "/TEST-SPACE/EV-PARENT-NORMAL", "/TEST-SPACE/CP-TEST-4",
-                "/TEST-SPACE/SAMPLE-TO-DELETE");
+        SampleSearchCriteria sampleSearchCriteria = new SampleSearchCriteria();
+        sampleSearchCriteria.withCode().thatEndsWith("P");
+        testSearch(TEST_SPACE_USER, sampleSearchCriteria, "/DP", "/MP");
     }
 
     @Test
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java
index e6ef8d91fdb..eccc0531d69 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java
@@ -193,7 +193,7 @@ public class GeneralInformationServiceTest extends SystemTestCase
         loginAsObserver();
         samples = generalInformationService.searchForSamples(sessionToken, searchCriteria);
 
-        assertEntities("[]", samples);
+        assertEntities("[/MP]", samples);
     }
 
     @Test
@@ -424,7 +424,7 @@ public class GeneralInformationServiceTest extends SystemTestCase
                 generalInformationService.searchForSamples(sessionToken, searchCriteria,
                         fetchOptions);
 
-        assertEntities("[]", samples);
+        assertEntities("[/DP]", samples);
     }
 
     @Test
@@ -537,7 +537,7 @@ public class GeneralInformationServiceTest extends SystemTestCase
                 generalInformationService.searchForSamples(sessionToken, searchCriteria,
                         fetchOptions);
 
-        assertEntities("[]", samples);
+        assertEntities("[/DP]", samples);
     }
 
     @Test
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ETLServiceAuthorizationTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ETLServiceAuthorizationTest.java
index adea353f539..3b472a8ba58 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ETLServiceAuthorizationTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/authorization/ETLServiceAuthorizationTest.java
@@ -41,6 +41,7 @@ import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Project;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.RoleWithHierarchy;
 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.SampleType;
 import ch.systemsx.cisd.openbis.generic.shared.basic.dto.Space;
 import ch.systemsx.cisd.openbis.generic.shared.dto.AtomicEntityOperationDetails;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DataSetBatchUpdatesDTO;
@@ -71,6 +72,10 @@ public class ETLServiceAuthorizationTest extends BaseTest
 
     private Project anotherProject;
 
+    private Sample sharedSample;
+
+    private Sample childSharedSample;
+
     @BeforeMethod
     public void createSomeEntities()
     {
@@ -80,6 +85,8 @@ public class ETLServiceAuthorizationTest extends BaseTest
         anotherProject = create(aProject().inSpace(anotherSpace));
         experiment = create(anExperiment().inProject(project));
         sample = create(aSample().inExperiment(experiment));
+        sharedSample = create(aSample());
+        childSharedSample = create(aSample().withParent(sharedSample));
         create(aSample().inExperiment(experiment));
     }
 
@@ -94,6 +101,17 @@ public class ETLServiceAuthorizationTest extends BaseTest
 
         assertEquals(2, samples.size());
     }
+    
+    @Test
+    public void testListSharedSampleByASpaceUser()
+    {
+        String sessionToken = create(aSession().withSpaceRole(RoleWithHierarchy.SPACE_USER, space));
+
+        List<Sample> samples = etlService.listSamples(sessionToken, ListSampleCriteria.createForParent(new TechId(sharedSample)));
+        
+        assertEquals(childSharedSample.getId(), samples.get(0).getId());
+        assertEquals(1, samples.size());
+    }
 
     @Test(expectedExceptions =
     { AuthorizationFailureException.class })
-- 
GitLab