From 5cb0a3374a099a18f1fe032ce268a1b3cbd71441 Mon Sep 17 00:00:00 2001
From: izabel <izabel>
Date: Mon, 6 Apr 2009 14:32:32 +0000
Subject: [PATCH] [LMS-816] check mandatory properties

SVN: 10603
---
 .../bo/EntityPropertiesConverter.java         | 45 ++++++++++---
 .../server/business/bo/ExperimentBO.java      |  2 +-
 .../bo/EntityPropertiesConverterTest.java     | 67 ++++++++++++++++---
 .../server/business/bo/ExperimentBOTest.java  | 63 +++++++++++++++--
 4 files changed, 150 insertions(+), 27 deletions(-)

diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverter.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverter.java
index 950d60baf99..f57f0304fe0 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverter.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverter.java
@@ -18,7 +18,6 @@ package ch.systemsx.cisd.openbis.generic.server.business.bo;
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
@@ -55,6 +54,9 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.properties.EntityKind;
  */
 public final class EntityPropertiesConverter implements IEntityPropertiesConverter
 {
+    private static final String NO_ENTITY_PROPERTY_VALUE_FOR_S =
+            "No entity property value for '%s'.";
+
     private final IDAOFactory daoFactory;
 
     private final EntityKind entityKind;
@@ -194,8 +196,7 @@ public final class EntityPropertiesConverter implements IEntityPropertiesConvert
                 getEntityTypePropertyType(entityTypePE, propertyType);
         if (entityTypePropertyTypePE.isMandatory() && valueOrNull == null)
         {
-            throw UserFailureException.fromTemplate("No entity property value for '%s'.",
-                    propertyCode);
+            throw UserFailureException.fromTemplate(NO_ENTITY_PROPERTY_VALUE_FOR_S, propertyCode);
         }
         if (valueOrNull != null)
         {
@@ -239,10 +240,6 @@ public final class EntityPropertiesConverter implements IEntityPropertiesConvert
         assert entityTypeCode != null : "Unspecified entity type code.";
         assert registrator != null : "Unspecified registrator";
         assert properties != null : "Unspecified entity properties";
-        if (properties.length == 0)
-        {
-            return Collections.emptyList();
-        }
         final EntityTypePE entityTypePE = getEntityType(entityTypeCode);
         final List<T> list = new ArrayList<T>();
         for (final EntityProperty<?, ?> property : properties)
@@ -254,16 +251,46 @@ public final class EntityPropertiesConverter implements IEntityPropertiesConvert
                 list.add(convertedPropertyOrNull);
             }
         }
+        checkMandatoryProperties(list, entityTypePE);
         return list;
     }
 
+    /**
+     * Checks if mandatory fields are filled.
+     */
+    private <T extends EntityPropertyPE> void checkMandatoryProperties(List<T> properties,
+            EntityTypePE entityTypePE)
+    {
+        assert properties != null;
+        List<EntityTypePropertyTypePE> assignedProperties =
+                daoFactory.getEntityPropertyTypeDAO(entityKind).listEntityPropertyTypes(
+                        entityTypePE);
+        if (assignedProperties == null || assignedProperties.size() == 0)
+        {
+            return;
+        }
+        Set<EntityTypePropertyTypePE> definedProperties = new HashSet<EntityTypePropertyTypePE>();
+        for (T p : properties)
+        {
+            definedProperties.add(p.getEntityTypePropertyType());
+        }
+        for (EntityTypePropertyTypePE etpt : assignedProperties)
+        {
+            if (etpt.isMandatory() && (definedProperties.contains(etpt) == false))
+            {
+                throw UserFailureException.fromTemplate(NO_ENTITY_PROPERTY_VALUE_FOR_S, etpt
+                        .getPropertyType().getCode());
+            }
+        }
+    }
+
     public final <T extends EntityPropertyPE> T createProperty(PropertyTypePE propertyType,
             EntityTypePropertyTypePE entityTypPropertyType, final PersonPE registrator, String value)
     {
         if (entityTypPropertyType.isMandatory() && value == null)
         {
-            throw UserFailureException.fromTemplate("No entity property value for '%s'.",
-                    propertyType.getCode());
+            throw UserFailureException.fromTemplate(NO_ENTITY_PROPERTY_VALUE_FOR_S, propertyType
+                    .getCode());
         }
         if (value != null)
         {
diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
index 09ccee395a0..64f8f19cf86 100644
--- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
+++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBO.java
@@ -189,9 +189,9 @@ public final class ExperimentBO extends AbstractBusinessObject implements IExper
         final PersonPE registrator = findRegistrator();
         experiment.setCode(experimentIdentifier.getExperimentCode());
         experiment.setRegistrator(registrator);
+        defineExperimentType(newExperiment);
         defineExperimentProperties(newExperiment.getExperimentTypeCode(), newExperiment
                 .getProperties(), registrator);
-        defineExperimentType(newExperiment);
         defineExperimentProject(newExperiment, experimentIdentifier);
         dataChanged = true;
     }
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverterTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverterTest.java
index c641c44c4d8..efa74deecf6 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverterTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/EntityPropertiesConverterTest.java
@@ -68,6 +68,11 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
     }
 
     private void prepareForConvertion(final Expectations exp)
+    {
+        prepareForConvertion(exp, false);
+    }
+
+    private void prepareForConvertion(final Expectations exp, boolean mandatory)
     {
         final SampleTypePE sampleType = createSampleType(SAMPLE_TYPE_CODE);
         final SampleTypePropertyTypePE sampleTypePropertyTypePE = new SampleTypePropertyTypePE();
@@ -75,20 +80,21 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
         final PropertyTypePE propertyType = new PropertyTypePE();
         propertyType.setCode(VARCHAR_PROPERTY_TYPE_CODE);
         sampleTypePropertyTypePE.setPropertyType(propertyType);
+        sampleTypePropertyTypePE.setMandatory(mandatory);
 
-        exp.allowing(daoFactory).getEntityPropertyTypeDAO(EntityKind.SAMPLE);
+        exp.atLeast(1).of(daoFactory).getEntityPropertyTypeDAO(EntityKind.SAMPLE);
         exp.will(Expectations.returnValue(entityPropertyTypeDAO));
 
-        exp.allowing(daoFactory).getEntityTypeDAO(EntityKind.SAMPLE);
+        exp.atLeast(1).of(daoFactory).getEntityTypeDAO(EntityKind.SAMPLE);
         exp.will(Expectations.returnValue(entityTypeDAO));
 
         exp.allowing(daoFactory).getPropertyTypeDAO();
         exp.will(Expectations.returnValue(propertyTypeDAO));
 
-        exp.one(entityTypeDAO).listEntityTypes();
+        exp.atLeast(1).of(entityTypeDAO).listEntityTypes();
         exp.will(Expectations.returnValue(Collections.singletonList(sampleType)));
 
-        exp.one(entityPropertyTypeDAO).listEntityPropertyTypes(sampleType);
+        exp.atLeast(1).of(entityPropertyTypeDAO).listEntityPropertyTypes(sampleType);
         exp.will(Expectations.returnValue(Collections.singletonList(sampleTypePropertyTypePE)));
     }
 
@@ -100,19 +106,20 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
         return sampleType;
     }
 
-    private final static SampleProperty createVarcharSampleProperty(final boolean lowerCase)
+    private final static SampleProperty createVarcharSampleProperty(final boolean lowerCase,
+            String code)
     {
         final SampleProperty sampleProperty = new SampleProperty();
         sampleProperty.setValue("blue");
         final SampleTypePropertyType sampleTypePropertyType = new SampleTypePropertyType();
         final PropertyType propertyType = new PropertyType();
-        String code = VARCHAR_PROPERTY_TYPE_CODE;
+        String newCode = code;
         if (lowerCase)
         {
-            code = code.toLowerCase();
+            newCode = newCode.toLowerCase();
         }
-        propertyType.setLabel(code);
-        propertyType.setCode(code);
+        propertyType.setLabel(newCode);
+        propertyType.setCode(newCode);
         final DataType dataType = new DataType();
         dataType.setCode(DataTypeCode.VARCHAR);
         propertyType.setDataType(dataType);
@@ -124,7 +131,7 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
     private final SampleProperty[] createSampleProperties(final boolean lowerCase)
     {
         return new SampleProperty[]
-            { createVarcharSampleProperty(lowerCase) };
+            { createVarcharSampleProperty(lowerCase, VARCHAR_PROPERTY_TYPE_CODE) };
     }
 
     @Test
@@ -149,6 +156,12 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
     {
         final IEntityPropertiesConverter entityPropertiesConverter =
                 createEntityPropertiesConverter(EntityKind.SAMPLE);
+        context.checking(new Expectations()
+            {
+                {
+                    prepareForConvertion(this);
+                }
+            });
         final List<EntityPropertyPE> properties =
                 entityPropertiesConverter.convertProperties(SampleProperty.EMPTY_ARRAY,
                         SAMPLE_TYPE_CODE, ManagerTestTool.EXAMPLE_PERSON);
@@ -179,6 +192,36 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
                 entityPropertiesConverter.convertProperties(properties, SAMPLE_TYPE_CODE,
                         ManagerTestTool.EXAMPLE_PERSON);
         assertEquals(1, convertedProperties.size());
+        context.assertIsSatisfied();
+    }
+
+    @Test
+    public final void testConvertPropertiesWithMissingMandatory()
+    {
+        final IEntityPropertiesConverter entityPropertiesConverter =
+                createEntityPropertiesConverter(EntityKind.SAMPLE);
+        final PropertyTypePE propertyTypePE = new PropertyTypePE();
+        propertyTypePE.setCode(VARCHAR_PROPERTY_TYPE_CODE);
+        context.checking(new Expectations()
+            {
+                {
+
+                    prepareForConvertion(this, true);
+                }
+            });
+        final SampleProperty[] properties = new SampleProperty[] {};
+        boolean exceptionThrown = false;
+        try
+        {
+            entityPropertiesConverter.convertProperties(properties, SAMPLE_TYPE_CODE,
+                    ManagerTestTool.EXAMPLE_PERSON);
+        } catch (UserFailureException ex)
+        {
+            exceptionThrown = true;
+            assertTrue(ex.getMessage().contains("No entity property value for 'COLOR'."));
+        }
+        assertTrue(exceptionThrown);
+        context.assertIsSatisfied();
     }
 
     @Test
@@ -204,6 +247,7 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
                 entityPropertiesConverter.convertProperties(properties, SAMPLE_TYPE_CODE
                         .toLowerCase(), ManagerTestTool.EXAMPLE_PERSON);
         assertEquals(1, convertedProperties.size());
+        context.assertIsSatisfied();
     }
 
     @Test
@@ -225,6 +269,7 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
             });
         assertEquals(registrator, entityPropertiesConverter.createProperty(propertyType,
                 assignment, registrator, defaultValue).getRegistrator());
+        context.assertIsSatisfied();
     }
 
     @Test(expectedExceptions = UserFailureException.class)
@@ -241,6 +286,7 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
         final String defaultValue = null;
         entityPropertiesConverter.createProperty(propertyType, assignment, registrator,
                 defaultValue);
+        context.assertIsSatisfied();
     }
 
     @Test
@@ -257,5 +303,6 @@ public final class EntityPropertiesConverterTest extends AbstractBOTest
         final String defaultValue = null;
         assertNull(entityPropertiesConverter.createProperty(propertyType, assignment, registrator,
                 defaultValue));
+        context.assertIsSatisfied();
     }
 }
diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
index aa1dadb3718..b753881ffcf 100644
--- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
+++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/business/bo/ExperimentBOTest.java
@@ -16,6 +16,8 @@
 
 package ch.systemsx.cisd.openbis.generic.server.business.bo;
 
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 
 import org.jmock.Expectations;
@@ -32,6 +34,7 @@ import ch.systemsx.cisd.openbis.generic.shared.dto.AttachmentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentTypePE;
+import ch.systemsx.cisd.openbis.generic.shared.dto.ExperimentTypePropertyTypePE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.GroupPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.ProjectPE;
 import ch.systemsx.cisd.openbis.generic.shared.dto.identifier.ExperimentIdentifier;
@@ -164,16 +167,31 @@ public final class ExperimentBOTest extends AbstractBOTest
         context.checking(new Expectations()
             {
                 {
-                    one(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
-                    will(returnValue(entityTypeDAO));
+                    atLeast(1).of(daoFactory).getEntityPropertyTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityPropertyTypeDAO));
+
+                    atLeast(1).of(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityTypeDAO));
+
+                    atLeast(1).of(entityTypeDAO).listEntityTypes();
+                    will(Expectations.returnValue(Collections.singletonList(type)));
+
+                    atLeast(1).of(entityPropertyTypeDAO).listEntityPropertyTypes(type);
+                    will(Expectations.returnValue(new ArrayList<ExperimentTypePropertyTypePE>(type
+                            .getExperimentTypePropertyTypes())));
+
                     one(entityTypeDAO).tryToFindEntityTypeByCode(expTypeCode);
                     will(returnValue(type));
+
                     one(daoFactory).getProjectDAO();
                     will(returnValue(projectDAO));
+
                     one(projectDAO).tryFindProject(dbCode, groupCode, projectCode);
                     will(returnValue(project));
+
                     one(daoFactory).getExperimentDAO();
                     will(returnValue(experimentDAO));
+
                     one(experimentDAO).createExperiment(experiment);
                 }
             });
@@ -200,8 +218,10 @@ public final class ExperimentBOTest extends AbstractBOTest
         context.checking(new Expectations()
             {
                 {
+
                     one(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
                     will(returnValue(entityTypeDAO));
+
                     one(entityTypeDAO).tryToFindEntityTypeByCode(expTypeCode);
                     will(returnValue(null));
                 }
@@ -239,12 +259,25 @@ public final class ExperimentBOTest extends AbstractBOTest
         context.checking(new Expectations()
             {
                 {
-                    one(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
-                    will(returnValue(entityTypeDAO));
+                    atLeast(1).of(daoFactory).getEntityPropertyTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityPropertyTypeDAO));
+
+                    atLeast(1).of(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityTypeDAO));
+
+                    atLeast(1).of(entityTypeDAO).listEntityTypes();
+                    will(Expectations.returnValue(Collections.singletonList(type)));
+
+                    atLeast(1).of(entityPropertyTypeDAO).listEntityPropertyTypes(type);
+                    will(Expectations.returnValue(new ArrayList<ExperimentTypePropertyTypePE>(type
+                            .getExperimentTypePropertyTypes())));
+
                     one(entityTypeDAO).tryToFindEntityTypeByCode(expTypeCode);
                     will(returnValue(type));
+
                     one(daoFactory).getProjectDAO();
                     will(returnValue(projectDAO));
+
                     one(projectDAO).tryFindProject(dbCode, groupCode, projectCode);
                     will(returnValue(null));
                 }
@@ -267,7 +300,7 @@ public final class ExperimentBOTest extends AbstractBOTest
     }
 
     @Test
-    public void testDefineAndSaveAlreadyExistingExperiment()
+    public final void testDefineAndSaveAlreadyExistingExperiment()
     {
         final String expCode = EXP_CODE;
         final String expTypeCode = EXP_TYPE_CODE;
@@ -286,16 +319,31 @@ public final class ExperimentBOTest extends AbstractBOTest
         context.checking(new Expectations()
             {
                 {
-                    one(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
-                    will(returnValue(entityTypeDAO));
+                    atLeast(1).of(daoFactory).getEntityPropertyTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityPropertyTypeDAO));
+
+                    atLeast(1).of(daoFactory).getEntityTypeDAO(EntityKind.EXPERIMENT);
+                    will(Expectations.returnValue(entityTypeDAO));
+
+                    atLeast(1).of(entityTypeDAO).listEntityTypes();
+                    will(Expectations.returnValue(Collections.singletonList(type)));
+
+                    atLeast(1).of(entityPropertyTypeDAO).listEntityPropertyTypes(type);
+                    will(Expectations.returnValue(new ArrayList<ExperimentTypePropertyTypePE>(type
+                            .getExperimentTypePropertyTypes())));
+
                     one(entityTypeDAO).tryToFindEntityTypeByCode(expTypeCode);
                     will(returnValue(type));
+
                     one(daoFactory).getProjectDAO();
                     will(returnValue(projectDAO));
+
                     one(projectDAO).tryFindProject(dbCode, groupCode, projectCode);
                     will(returnValue(project));
+
                     one(daoFactory).getExperimentDAO();
                     will(returnValue(experimentDAO));
+
                     one(experimentDAO).createExperiment(experiment);
                     will(throwException(new DataIntegrityViolationException(
                             "exception description...")));
@@ -348,6 +396,7 @@ public final class ExperimentBOTest extends AbstractBOTest
     private ExperimentTypePE createExperimentType(final String expTypeCode)
     {
         ExperimentTypePE experimentType = new ExperimentTypePE();
+        experimentType.setDatabaseInstance(new DatabaseInstancePE());
         experimentType.setCode(expTypeCode);
         return experimentType;
     }
-- 
GitLab