feat: use simple expressions for validation expressions as well

molgenis · May 27, 2021 · 93c5978 · 93c5978
1 parent 9a93f1a
commit 93c5978
Show file tree

Hide file tree

Showing 14 changed files with 130 additions and 395 deletions.
diff --git a/molgenis-data-i18n/src/main/java/org/molgenis/data/i18n/model/LanguageMetadata.java b/molgenis-data-i18n/src/main/java/org/molgenis/data/i18n/model/LanguageMetadata.java
@@ -28,7 +28,7 @@ public void init() {
     // 2 or 3 characters, alphanumeric, lowercase
     addAttribute(CODE, ROLE_ID)
         .setDescription("Lowercase ISO 639 alpha-2 or alpha-3 code")
-        .setValidationExpression("/^[a-z]{2,3}$/.test($('code').value())");
+        .setValidationExpression("regex('^[a-z]{2,3}$','code')");
     addAttribute(NAME).setNillable(false);
     addAttribute(ACTIVE).setDataType(BOOL).setNillable(false);
   }

diff --git a/molgenis-data-security/src/main/java/org/molgenis/data/security/auth/GroupMetadata.java b/molgenis-data-security/src/main/java/org/molgenis/data/security/auth/GroupMetadata.java
@@ -3,6 +3,7 @@
 import static java.lang.String.format;
 import static java.util.Collections.singleton;
 import static java.util.Objects.requireNonNull;
+import static org.apache.commons.lang3.StringEscapeUtils.escapeJava;
 import static org.molgenis.data.meta.AttributeType.BOOL;
 import static org.molgenis.data.meta.AttributeType.ONE_TO_MANY;
 import static org.molgenis.data.meta.AttributeType.TEXT;
@@ -56,8 +57,7 @@ public void init() {
         .setDescription("Name of the group")
         .setNillable(false)
         .setUnique(true)
-        .setValidationExpression(
-            format("$('name').matches(%s).value()", UNIFIED_IDENTIFIER_REGEX_JS));
+        .setValidationExpression(format("regex('%s',{name}", escapeJava(UNIFIED_IDENTIFIER_REGEX)));
     addAttribute(LABEL, ROLE_LABEL, ROLE_LOOKUP).setLabel("Label").setNillable(false);
     getLanguageCodes()
         .map(languageCode -> getI18nAttributeName(LABEL, languageCode))

diff --git a/molgenis-data-security/src/main/java/org/molgenis/data/security/auth/RoleMetadata.java b/molgenis-data-security/src/main/java/org/molgenis/data/security/auth/RoleMetadata.java
@@ -2,6 +2,7 @@
 
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
+import static org.apache.commons.lang3.StringEscapeUtils.escapeJava;
 import static org.molgenis.data.meta.AttributeType.MREF;
 import static org.molgenis.data.meta.AttributeType.TEXT;
 import static org.molgenis.data.meta.AttributeType.XREF;
@@ -50,8 +51,7 @@ public void init() {
         .setDescription("Name of the Role")
         .setUnique(true)
         .setNillable(false)
-        .setValidationExpression(
-            format("$('name').matches(%s).value()", UNIFIED_IDENTIFIER_REGEX_JS))
+        .setValidationExpression(format("regex('%s',{name}", escapeJava(UNIFIED_IDENTIFIER_REGEX)))
         .setReadOnly(true);
     addAttribute(LABEL, ROLE_LABEL, ROLE_LOOKUP).setLabel("Label").setNillable(false);
     getLanguageCodes()

diff --git a/molgenis-data-validation/pom.xml b/molgenis-data-validation/pom.xml
@@ -14,7 +14,7 @@
     <dependency>
       <groupId>org.molgenis</groupId>
       <artifactId>molgenis-expressions_2.13</artifactId>
-      <version>0.10</version>
+      <version>0.12.2</version>
     </dependency>
     <dependency>
       <groupId>org.molgenis</groupId>

diff --git a/...data-validation/src/main/java/org/molgenis/data/validation/EntityAttributesValidator.java b/...data-validation/src/main/java/org/molgenis/data/validation/EntityAttributesValidator.java
@@ -33,14 +33,10 @@
  */
 @Component
 public class EntityAttributesValidator {
-  private final ExpressionValidator expressionValidator;
   private final SimpleExpressionValidator simpleExpressionValidator;
   private EmailValidator emailValidator;
 
-  EntityAttributesValidator(
-      ExpressionValidator expressionValidator,
-      SimpleExpressionValidator simpleExpressionValidator) {
-    this.expressionValidator = requireNonNull(expressionValidator);
+  EntityAttributesValidator(SimpleExpressionValidator simpleExpressionValidator) {
     this.simpleExpressionValidator = requireNonNull(simpleExpressionValidator);
   }
 
@@ -215,7 +211,7 @@ private Set<ConstraintViolation> checkValidationExpressions(Entity entity, Entit
 
     if (!validationExpressions.isEmpty()) {
       List<Boolean> results =
-          expressionValidator.resolveBooleanExpressions(validationExpressions, entity);
+          simpleExpressionValidator.resolveBooleanExpressions(validationExpressions, entity);
       for (int i = 0; i < results.size(); i++) {
         if (!Boolean.TRUE.equals(results.get(i))) {
           violations.add(

diff --git a/...data-validation/src/main/java/org/molgenis/data/validation/SimpleExpressionValidator.java b/...data-validation/src/main/java/org/molgenis/data/validation/SimpleExpressionValidator.java
@@ -19,21 +19,27 @@ public SimpleExpressionValidator(SimpleExpressionEvaluator evaluator) {
   }
 
   List<Boolean> resolveBooleanExpressions(List<String> expressions, Entity entity) {
-    Map<String, Object> context =
-        evaluator.getVariableNames(expressions).stream()
-            .reduce(
-                new HashMap<>(),
-                (soFar, variableName) -> {
-                  soFar.put(variableName, SimpleExpressionEvaluator.resolve(entity, variableName));
-                  return soFar;
-                },
-                (map, map2) -> map);
-    return evaluator.parseAndEvaluate(expressions, context).stream()
-        .map(this::convertToBoolean)
-        .collect(Collectors.toList());
+    try {
+      var variableNames = evaluator.getVariableNames(expressions);
+      Map<String, Object> context =
+          variableNames.stream()
+              .reduce(
+                  new HashMap<>(),
+                  (soFar, variableName) -> {
+                    soFar.put(
+                        variableName, SimpleExpressionEvaluator.resolve(entity, variableName));
+                    return soFar;
+                  },
+                  (map, map2) -> map);
+      return evaluator.parseAndEvaluate(expressions, context).stream()
+          .map(this::convertToBoolean)
+          .collect(Collectors.toList());
+    } catch (Exception ex) {
+      throw ex;
+    }
   }
 
   private Boolean convertToBoolean(Try<Object> result) {
-    return result.map(Object::toString).map(Boolean::valueOf).getOrElse(null);
+    return result.map(Object::toString).map(Boolean::valueOf).getOrElse(() -> null);
   }
 }
diff --git a/...s-data-validation/src/main/java/org/molgenis/data/validation/meta/AttributeValidator.java b/...s-data-validation/src/main/java/org/molgenis/data/validation/meta/AttributeValidator.java
@@ -4,6 +4,7 @@
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
 import static org.molgenis.data.meta.AttributeType.BOOL;
 import static org.molgenis.data.meta.AttributeType.CATEGORICAL;
 import static org.molgenis.data.meta.AttributeType.CATEGORICAL_MREF;
@@ -34,6 +35,7 @@
 import static org.molgenis.data.validation.meta.AttributeValidator.ValidationMode.UPDATE;
 
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.Arrays;
@@ -42,6 +44,7 @@
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Stream;
 import org.hibernate.validator.internal.constraintvalidators.bv.EmailValidator;
 import org.molgenis.data.DataService;
 import org.molgenis.data.Entity;
@@ -56,6 +59,7 @@
 import org.molgenis.data.support.TemplateExpressionEvaluator;
 import org.molgenis.data.util.EntityUtils;
 import org.molgenis.data.validation.MolgenisValidationException;
+import org.molgenis.data.validation.SimpleExpressionEvaluator;
 import org.molgenis.util.UnexpectedEnumException;
 import org.molgenis.validation.ConstraintViolation;
 import org.springframework.stereotype.Component;
@@ -73,10 +77,15 @@ public enum ValidationMode {
   private final DataService dataService;
   private final EntityManager entityManager;
   private final EmailValidator emailValidator;
+  private final SimpleExpressionEvaluator simpleExpressionEvaluator;
 
-  public AttributeValidator(DataService dataService, EntityManager entityManager) {
+  public AttributeValidator(
+      DataService dataService,
+      EntityManager entityManager,
+      SimpleExpressionEvaluator simpleExpressionEvaluator) {
     this.dataService = requireNonNull(dataService);
     this.entityManager = requireNonNull(entityManager);
+    this.simpleExpressionEvaluator = requireNonNull(simpleExpressionEvaluator);
     this.emailValidator = new EmailValidator();
   }
 
@@ -86,6 +95,7 @@ public void validate(Attribute attr, ValidationMode validationMode) {
     validateParent(attr);
     validateChildren(attr);
     validateExpression(attr);
+    validateSimpleExpressions(attr);
 
     switch (validationMode) {
       case ADD:
@@ -106,6 +116,29 @@ public void validate(Attribute attr, ValidationMode validationMode) {
     }
   }
 
+  private void validateSimpleExpressions(Attribute attr) {
+    List<String> expressions =
+        Stream.of(
+                attr.getValidationExpression(),
+                attr.getNullableExpression(),
+                attr.getVisibleExpression())
+            .filter(Objects::nonNull)
+            .collect(toList());
+    final var usedVariables = simpleExpressionEvaluator.getVariableNames(expressions);
+    if (usedVariables.isEmpty()) {
+      return;
+    }
+    final var attributeNames =
+        stream(attr.getEntity().getAllAttributes()).map(Attribute::getName).collect(toSet());
+    var unknownAttributes = Sets.difference(usedVariables, attributeNames);
+    if (!unknownAttributes.isEmpty()) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Unknown attributes in expressions for attribute %s: %s",
+              attr.getName(), unknownAttributes));
+    }
+  }
+
   private void validateExpression(Attribute attr) {
     if (attr.getExpression() != null && isValidatable(attr)) {
       // throws exception if invalid

diff --git a/...-validation/src/test/java/org/molgenis/data/validation/EntityAttributesValidatorTest.java b/...-validation/src/test/java/org/molgenis/data/validation/EntityAttributesValidatorTest.java
@@ -23,6 +23,7 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
@@ -37,19 +38,14 @@
 
 @MockitoSettings(strictness = Strictness.LENIENT)
 class EntityAttributesValidatorTest extends AbstractMockitoTest {
-  @Mock private ExpressionValidator expressionValidator;
   @Mock private SimpleExpressionValidator simpleExpressionValidator;
-
-  private EntityAttributesValidator entityAttributesValidator;
+  @InjectMocks private EntityAttributesValidator entityAttributesValidator;
 
   private EntityType intRangeMinMeta;
   private EntityType intRangeMaxMeta;
 
   @BeforeEach
   void setUpBeforeMethod() {
-    entityAttributesValidator =
-        new EntityAttributesValidator(expressionValidator, simpleExpressionValidator);
-
     Attribute idAttr = when(mock(Attribute.class).getName()).thenReturn("id").getMock();
     when(idAttr.getDataType()).thenReturn(STRING);
     when(idAttr.getMaxLength()).thenReturn(255);

diff --git a/...ta-validation/src/test/java/org/molgenis/data/validation/meta/AttributeValidatorTest.java b/...ta-validation/src/test/java/org/molgenis/data/validation/meta/AttributeValidatorTest.java
@@ -9,6 +9,7 @@
 import static org.mockito.Mockito.RETURNS_SELF;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
+import static org.mockito.quality.Strictness.LENIENT;
 import static org.molgenis.data.Sort.Direction.ASC;
 import static org.molgenis.data.meta.AttributeType.BOOL;
 import static org.molgenis.data.meta.AttributeType.CATEGORICAL;
@@ -25,10 +26,12 @@
 import static org.molgenis.data.validation.meta.AttributeValidator.ValidationMode.ADD_SKIP_ENTITY_VALIDATION;
 
 import java.util.Optional;
-import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoSettings;
 import org.molgenis.data.DataService;
 import org.molgenis.data.Entity;
 import org.molgenis.data.EntityManager;
@@ -42,19 +45,16 @@
 import org.molgenis.data.meta.model.EntityType;
 import org.molgenis.data.support.TemplateExpressionSyntaxException;
 import org.molgenis.data.validation.MolgenisValidationException;
+import org.molgenis.data.validation.SimpleExpressionEvaluator;
 import org.molgenis.data.validation.meta.AttributeValidator.ValidationMode;
-
-class AttributeValidatorTest {
-  private AttributeValidator attributeValidator;
-  private DataService dataService;
-  private EntityManager entityManager;
-
-  @BeforeEach
-  void beforeMethod() {
-    dataService = mock(DataService.class);
-    entityManager = mock(EntityManager.class);
-    attributeValidator = new AttributeValidator(dataService, entityManager);
-  }
+import org.molgenis.test.AbstractMockitoTest;
+
+@MockitoSettings(strictness = LENIENT)
+class AttributeValidatorTest extends AbstractMockitoTest {
+  @InjectMocks private AttributeValidator attributeValidator;
+  @Mock private DataService dataService;
+  @Mock private EntityManager entityManager;
+  @Mock private SimpleExpressionEvaluator simpleExpressionEvaluator;
 
   @Test
   void validateAttributeInvalidName() {