diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluator.java index dc36e85e355b582f2165ca53c2bd6cfa41ee1d33..669723413962672ed8f92ad20b762b2e6a615c35 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluator.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluator.java @@ -16,7 +16,9 @@ package ch.systemsx.cisd.openbis.generic.server.dataaccess.db.dynamic_property; +import java.util.ArrayList; import java.util.HashMap; +import java.util.List; import java.util.Map; import org.apache.log4j.Logger; @@ -53,6 +55,10 @@ public class DynamicPropertyEvaluator implements IDynamicPropertyEvaluator private final Map<ScriptPE, DynamicPropertyCalculator> calculatorsByScript = new HashMap<ScriptPE, DynamicPropertyCalculator>(); + /** path of evaluation - used to generate meaningful error message for cyclic dependencies */ + private final List<EntityTypePropertyTypePE> evaluationPath = + new ArrayList<EntityTypePropertyTypePE>(); + /** Returns a calculator for given script (creates a new one if nothing is found in cache). */ private DynamicPropertyCalculator getCalculator(ScriptPE scriptPE) { @@ -80,17 +86,35 @@ public class DynamicPropertyEvaluator implements IDynamicPropertyEvaluator EntityTypePropertyTypePE etpt = property.getEntityTypePropertyType(); if (etpt.isDynamic()) { - final String dynamicValue = evaluateProperty(entityAdaptor, etpt); + final String dynamicValue = evaluateProperty(entityAdaptor, etpt, false); property.setValue(dynamicValue); } } } + public List<EntityTypePropertyTypePE> getEvaluationPath() + { + return evaluationPath; + } + public String evaluateProperty(IEntityAdaptor entityAdaptor, EntityTypePropertyTypePE etpt) + { + return evaluateProperty(entityAdaptor, etpt, false); + } + + public String evaluateProperty(IEntityAdaptor entityAdaptor, EntityTypePropertyTypePE etpt, + boolean startPath) { assert etpt.isDynamic() == true : "expected dynamic property"; try { + if (startPath) + { + evaluationPath.clear(); + } else + { + evaluationPath.add(etpt); + } final DynamicPropertyCalculator calculator = getCalculator(etpt.getScript()); calculator.setEntity(entityAdaptor); final String dynamicValue = calculator.evalAsString(); @@ -111,4 +135,5 @@ public class DynamicPropertyEvaluator implements IDynamicPropertyEvaluator operationLog.info(errorMsg); return BasicConstant.ERROR_PROPERTY_PREFIX + errorMsg; } + } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/IDynamicPropertyEvaluator.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/IDynamicPropertyEvaluator.java index 7f0ef4c3f03f27ede175cff04d57149f26b24685..f2d700c84e369dcd1865a9171a80496bfc03927e 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/IDynamicPropertyEvaluator.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/IDynamicPropertyEvaluator.java @@ -16,6 +16,8 @@ package ch.systemsx.cisd.openbis.generic.server.dataaccess.db.dynamic_property; +import java.util.List; + import ch.systemsx.cisd.openbis.generic.server.dataaccess.db.dynamic_property.calculator.IEntityAdaptor; import ch.systemsx.cisd.openbis.generic.shared.dto.EntityTypePropertyTypePE; import ch.systemsx.cisd.openbis.generic.shared.dto.IEntityInformationWithPropertiesHolder; @@ -42,4 +44,10 @@ public interface IDynamicPropertyEvaluator public String evaluateProperty(IEntityAdaptor entityAdaptor, EntityTypePropertyTypePE dynamicPropertyETPT); + /** + * @return current path of evaluation of dynamic properties depending on other dynamic + * properties - useful to generate meaningful error message for cyclic dependencies + */ + public List<EntityTypePropertyTypePE> getEvaluationPath(); + } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/calculator/DynamicPropertyAdaptor.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/calculator/DynamicPropertyAdaptor.java index e40d2248130b95fa35db2aa29482dc3d32c19705..80e079c8aadf56cdeff9daf41b834649670cc9ff 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/calculator/DynamicPropertyAdaptor.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/calculator/DynamicPropertyAdaptor.java @@ -75,9 +75,15 @@ class DynamicPropertyAdaptor implements IEntityPropertyAdaptor break; case EVALUATING: // cycle found - return an error + StringBuilder path = new StringBuilder(); + for (EntityTypePropertyTypePE etpt : evaluator.getEvaluationPath()) + { + path.append(etpt.getPropertyType().getCode() + " -> "); + } + path.append(propertyTypeCode()); String errorMsg = - String.format("cycle of dependencies found for dynamic property '%s'", - propertyTypeCode()); + String.format("cycle of dependencies found between dynamic properties: %s", + path.toString()); value = DynamicPropertyEvaluator.errorPropertyValue(errorMsg); state = State.EVALUATED; break; diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluatorTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluatorTest.java index dc903168bb910af3dc7e9d49e0070ea1f195e50a..c1e32a11841ff4bd09985ec1b16d7057e2e5d870 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluatorTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/dynamic_property/DynamicPropertyEvaluatorTest.java @@ -155,7 +155,7 @@ public class DynamicPropertyEvaluatorTest extends AssertJUnit // TODO extend Abs assertEquals(p1.getValue(), dp3.getValue()); // create sample with circular dependency - // dp1 <- dp1 + // dp1 -> dp1 properties = new LinkedHashSet<SamplePropertyPE>(); dp1 = createDynamicSampleProperty("dp1", scriptDp1); properties.add(p1); @@ -163,9 +163,9 @@ public class DynamicPropertyEvaluatorTest extends AssertJUnit // TODO extend Abs sample = createSample("s1", properties); evaluator.evaluateProperties(sample); // cyclic dependency should be found - assertEquals(expectedCyclicDependencyErrorMessage(dp1), dp1.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp1, dp1), dp1.getValue()); - // dp1 <- dp2 <- dp1 + // dp1 -> dp2 -> dp1 properties = new LinkedHashSet<SamplePropertyPE>(); dp1 = createDynamicSampleProperty("dp1", scriptDp2); dp2 = createDynamicSampleProperty("dp2", scriptDp1); @@ -175,14 +175,14 @@ public class DynamicPropertyEvaluatorTest extends AssertJUnit // TODO extend Abs sample = createSample("s1", properties); evaluator.evaluateProperties(sample); // cyclic dependency should be found - assertEquals(expectedCyclicDependencyErrorMessage(dp2), dp1.getValue()); - assertEquals(expectedCyclicDependencyErrorMessage(dp2), dp2.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp2, dp1, dp2), dp1.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp2, dp1, dp2), dp2.getValue()); - // dp1 <- dp2 <- dp3 <- dp1 + // dp1 -> dp2 -> dp3 -> dp1 properties = new LinkedHashSet<SamplePropertyPE>(); - dp1 = createDynamicSampleProperty("dp1", scriptDp3); - dp2 = createDynamicSampleProperty("dp2", scriptDp1); - dp3 = createDynamicSampleProperty("dp3", scriptDp2); + dp1 = createDynamicSampleProperty("dp1", scriptDp2); + dp2 = createDynamicSampleProperty("dp2", scriptDp3); + dp3 = createDynamicSampleProperty("dp3", scriptDp1); properties.add(p1); properties.add(dp1); properties.add(dp2); @@ -190,9 +190,9 @@ public class DynamicPropertyEvaluatorTest extends AssertJUnit // TODO extend Abs sample = createSample("s1", properties); evaluator.evaluateProperties(sample); // cyclic dependency should be found - assertEquals(expectedCyclicDependencyErrorMessage(dp2), dp1.getValue()); - assertEquals(expectedCyclicDependencyErrorMessage(dp2), dp2.getValue()); - assertEquals(expectedCyclicDependencyErrorMessage(dp2), dp3.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp1, dp2, dp3, dp1), dp1.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp1, dp2, dp3, dp1), dp2.getValue()); + assertEquals(expectedCyclicDependencyErrorMessage(dp1, dp2, dp3, dp1), dp3.getValue()); } // @@ -204,11 +204,18 @@ public class DynamicPropertyEvaluatorTest extends AssertJUnit // TODO extend Abs return String.format("%sERROR: %s", BasicConstant.ERROR_PROPERTY_PREFIX, message); } - private static String expectedCyclicDependencyErrorMessage(EntityPropertyPE property) + private static String expectedCyclicDependencyErrorMessage(EntityPropertyPE... properties) { + StringBuilder path = new StringBuilder(); + for (EntityPropertyPE property : properties) + { + path.append(property.getEntityTypePropertyType().getPropertyType().getCode() + .toUpperCase()); + path.append(" -> "); + } + path.delete(path.length() - 4, path.length()); return expectedErrorMessage(String.format( - "cycle of dependencies found for dynamic property '%s'", property - .getEntityTypePropertyType().getPropertyType().getCode().toUpperCase())); + "cycle of dependencies found between dynamic properties: %s", path.toString())); } private static SamplePE createSample(String code, Set<SamplePropertyPE> properties)