Merge pull request #53 from isa-group/hotfix/feature-evaluation-crashing

fix: empty strings as default value for expression and serverExpression

Author: Alex-GF

src/main/java/io/github/isagroup/models/FeatureStatus.java

@@ -38,13 +38,19 @@ public void setLimit(Object limit) {
     }
 
     public static Optional<Boolean> computeFeatureEvaluation(String expression, PlanContextManager planContextManager) {
-        ExpressionParser expressionParser = new SpelExpressionParser();
-        EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding().build();
+
+        if (expression == null) {
+            throw new IllegalArgumentException(
+                    "expression was null. A expression must be provided to compute its evaluation");
+        }
 
         if (expression.trim().isEmpty()) {
             return Optional.of(false);
         }
 
+        ExpressionParser expressionParser = new SpelExpressionParser();
+        EvaluationContext evaluationContext = SimpleEvaluationContext.forReadOnlyDataBinding().build();
+
         return Optional.ofNullable(expressionParser.parseExpression(expression).getValue(evaluationContext,
                 planContextManager,
                 Boolean.class));

src/main/java/io/github/isagroup/services/parsing/FeatureParser.java

@@ -249,13 +249,23 @@ private static void loadBasicAttributes(Feature feature, String featureName, Map
                     + "; Current defaultValue: " + (String) map.get("defaultValue"));
         }
 
-        try {
-            feature.setExpression((String) map.get("expression"));
-            feature.setServerExpression((String) map.get("serverExpression"));
-        } catch (NoSuchElementException e) {
-            throw new PricingParsingException("The feature " + featureName
-                    + " does not have either an evaluation expression or serverExpression.");
+        if (map.get("expression") != null && !(map.get("expression") instanceof String)) {
+            throw new PricingParsingException("'expression' must be a String");
+        }
+        String expression = "";
+        if (map.get("expression") != null) {
+            expression = (String) map.get("expression");
+        }
+        feature.setExpression(expression);
+
+        if (map.get("serverExpression") != null && !(map.get("serverExpression") instanceof String)) {
+            throw new PricingParsingException("'expression' must be a String");
+        }
+        String serverExpression = "";
+        if (map.get("serverExpression") != null) {
+            serverExpression = (String) map.get("serverExpression");
         }
+        feature.setServerExpression(serverExpression);
 
         String featureTag = (String) map.get("tag");
 

src/test/java/io/github/isagroup/PricingEvaluatorUtilTests.java

@@ -4,7 +4,9 @@
 import java.util.Map;
 
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;
+import org.springframework.expression.spel.SpelEvaluationException;
 
 import io.github.isagroup.services.jwt.PricingJwtUtils;
 
@@ -127,4 +129,46 @@ void tokenUserContextTest() {
                 "UserContext havePetsDashboard value is not the same after token codification");
 
     }
+
+    private class NoExpressionsInFeatures extends PricingContext {
+
+        @Override
+        public String getConfigFilePath() {
+            return "pricing/feature-inoperative-no-expression.yaml";
+        }
+
+        @Override
+        public String getJwtSecret() {
+            return "Testing";
+        }
+
+        @Override
+        public Map<String, Object> getUserContext() {
+            Map<String, Object> map = new HashMap<>();
+            map.put("pets", 1);
+            return map;
+        }
+
+        @Override
+        public String getUserPlan() {
+            return "BASIC";
+        }
+
+    }
+
+    @Test
+    @DisplayName("Given a feature with no 'expression' should be disabled in the pricing token")
+    void givenAFeatureWithNoExpressionShouldNotThrowWhenComputingUserToken() {
+
+        PricingContext context = new NoExpressionsInFeatures();
+        PricingEvaluatorUtil pricingEvaluatorUtil = new PricingEvaluatorUtil(context);
+        PricingJwtUtils pricingJwtUtils = new PricingJwtUtils(context);
+        assertDoesNotThrow(() -> pricingEvaluatorUtil.generateUserToken());
+
+        Map<String, Map<String, Object>> featureStatuses = pricingJwtUtils
+                .getFeaturesFromJwtToken(pricingEvaluatorUtil.generateUserToken());
+        assertFalse((boolean) featureStatuses.get("support").get("eval"));
+
+    }
+
 }

src/test/java/io/github/isagroup/parsing/positive/FeatureParserTest.java

@@ -48,6 +48,8 @@ void givenGuaranteeFeatureShouldParse() {
         guaranteeFeature.setDefaultValue("99.9%");
         guaranteeFeature.setValueType(ValueType.TEXT);
         guaranteeFeature.setDocURL("https://example.org");
+        guaranteeFeature.setExpression("");
+        guaranteeFeature.setServerExpression("");
 
         PricingManager pricingManager = YamlUtils
                 .retrieveManagerFromYaml(POSITIVE_CASES + "type/guarantee-feature.yml");
@@ -65,7 +67,7 @@ void givenAFeatureExpressionShouldBeOptional() {
                 .retrieveManagerFromYaml(POSITIVE_CASES + "expression/expression-is-null.yml");
 
         assertNotNull(pricingManager.getFeatures().get(featName));
-        assertNull(pricingManager.getFeatures().get(featName).getExpression());
+        assertEquals("", pricingManager.getFeatures().get(featName).getExpression());
 
     }
 }

src/test/resources/pricing/feature-inoperative-no-expression.yaml

@@ -0,0 +1,30 @@
+saasName: Inoperative support
+syntaxVersion: "2.1"
+createdAt: "2024-01-15"
+currency: EUR
+features:
+  pets:
+    valueType: BOOLEAN
+    defaultValue: true
+    expression: userContext['pets'] < planContext['usageLimits']['maxPets']
+    serverExpression: userContext['pets'] <= planContext['usageLimits']['maxPets']
+    type: DOMAIN
+  support:
+    valueType: BOOLEAN
+    defaultValue: false
+    type: SUPPORT
+usageLimits:
+  maxPets:
+    valueType: NUMERIC
+    defaultValue: 2
+    unit: pet
+    type: NON_RENEWABLE
+    linkedFeatures:
+      - pets
+plans:
+  BASIC:
+    description: Basic plan
+    price: 0.0
+    unit: user/month
+    features: null
+    usageLimits: null