Attempt to run tests with a compiled version of unleash-engine and yg…

…gdrasil
Unleash · Nov 17, 2023 · 3862610 · 3862610
1 parent 09ed04b
commit 3862610
Show file tree

Hide file tree

Showing 16 changed files with 131 additions and 100 deletions.
diff --git a/.github/workflows/pull_requests.yml b/.github/workflows/pull_requests.yml
@@ -18,4 +18,6 @@ jobs:
           cache: 'maven'
           distribution: 'temurin'
       - name: Build, test, coverage
+        env:
+          YGGDRASIL_LIB_PATH: .
         run: ./mvnw clean test jacoco:report coveralls:report
diff --git a/libyggdrasilffi.so b/libyggdrasilffi.so
diff --git a/pom.xml b/pom.xml
@@ -59,8 +59,8 @@
             <artifactId>unleash-engine</artifactId>
             <groupId>io.getunleash</groupId>
             <scope>system</scope>
-            <version>*</version>
-            <systemPath>/home/gaston/projects/yggdrasil/java-engine/build/libs/unleash-engine-all.jar</systemPath>
+            <version>0.0.1-SNAPSHOT</version>
+            <systemPath>${project.basedir}/unleash-engine-all.jar</systemPath>
         </dependency>
         <dependency>
             <groupId>com.google.code.gson</groupId>

diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -1,5 +1,8 @@
 package io.getunleash;
 
+import static io.getunleash.Variant.DISABLED_VARIANT;
+import static java.util.Optional.ofNullable;
+
 import io.getunleash.engine.*;
 import io.getunleash.event.EventDispatcher;
 import io.getunleash.event.IsEnabledImpressionEvent;
@@ -13,9 +16,6 @@
 import io.getunleash.repository.JsonFeatureParser;
 import io.getunleash.strategy.*;
 import io.getunleash.util.UnleashConfig;
-import org.jetbrains.annotations.NotNull;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeParseException;
@@ -24,9 +24,9 @@
 import java.util.concurrent.atomic.LongAdder;
 import java.util.function.BiPredicate;
 import java.util.stream.Collectors;
-
-import static io.getunleash.Variant.DISABLED_VARIANT;
-import static java.util.Optional.ofNullable;
+import org.jetbrains.annotations.NotNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class DefaultUnleash implements Unleash {
     private static final Logger LOGGER = LoggerFactory.getLogger(DefaultUnleash.class);
@@ -101,17 +101,23 @@ public DefaultUnleash(
             UnleashMetricService metricService,
             boolean failOnMultipleInstantiations) {
 
-        this.unleashEngine = new UnleashEngine(
-            strategyMap.values().stream().map(this::asIStrategy).collect(Collectors.toList()),
-            Optional.ofNullable(unleashConfig.getFallbackStrategy()).map(this::asIStrategy).orElse(null)
-        );
-        featureRepository.addConsumer(featureCollection -> {
-            try {
-                this.unleashEngine.takeState(JsonFeatureParser.toJsonString(featureCollection));
-            } catch (YggdrasilInvalidInputException e) {
-                LOGGER.error("Unable to update features", e);
-            }
-        });
+        this.unleashEngine =
+                new UnleashEngine(
+                        strategyMap.values().stream()
+                                .map(this::asIStrategy)
+                                .collect(Collectors.toList()),
+                        Optional.ofNullable(unleashConfig.getFallbackStrategy())
+                                .map(this::asIStrategy)
+                                .orElse(null));
+        featureRepository.addConsumer(
+                featureCollection -> {
+                    try {
+                        this.unleashEngine.takeState(
+                                JsonFeatureParser.toJsonString(featureCollection));
+                    } catch (YggdrasilInvalidInputException e) {
+                        LOGGER.error("Unable to update features", e);
+                    }
+                });
 
         this.config = unleashConfig;
         this.featureRepository = featureRepository;
@@ -161,21 +167,22 @@ private UnleashContext adapt(Context context) {
         if (context.getCurrentTime() != null) {
             try {
                 currentTime = ZonedDateTime.parse(context.getCurrentTime());
-            } catch (DateTimeParseException e){
-                LOGGER.warn("Unable to parse current time from context: {}, using current time instead", context.getCurrentTime());
+            } catch (DateTimeParseException e) {
+                LOGGER.warn(
+                        "Unable to parse current time from context: {}, using current time instead",
+                        context.getCurrentTime());
             }
-
         }
 
         return new UnleashContext(
-            context.getAppName(),
-            context.getEnvironment(),
-            context.getUserId(),
-            context.getSessionId(),
-            context.getRemoteAddress(),
-            currentTime,
-            context.getProperties()
-        ).applyStaticFields(config);
+                        context.getAppName(),
+                        context.getEnvironment(),
+                        context.getUserId(),
+                        context.getSessionId(),
+                        context.getRemoteAddress(),
+                        currentTime,
+                        context.getProperties())
+                .applyStaticFields(config);
     }
 
     private Context adapt(UnleashContext context) {
@@ -186,7 +193,9 @@ private Context adapt(UnleashContext context) {
         mapped.setSessionId(context.getSessionId().orElse(null));
         mapped.setRemoteAddress(context.getRemoteAddress().orElse(null));
         mapped.setProperties(context.getProperties());
-        mapped.setCurrentTime(DateTimeFormatter.ISO_DATE_TIME.format(context.getCurrentTime().orElse(ZonedDateTime.now())));
+        mapped.setCurrentTime(
+                DateTimeFormatter.ISO_DATE_TIME.format(
+                        context.getCurrentTime().orElse(ZonedDateTime.now())));
         return mapped;
     }
 
@@ -218,7 +227,8 @@ private void dispatchEnabledImpressionDataIfNeeded(
             String toggleName, boolean enabled, UnleashContext context) {
         try {
             if (this.unleashEngine.shouldEmitImpressionEvent(toggleName)) {
-                eventDispatcher.dispatch(new IsEnabledImpressionEvent(toggleName, enabled, context));
+                eventDispatcher.dispatch(
+                        new IsEnabledImpressionEvent(toggleName, enabled, context));
             }
         } catch (YggdrasilError e) {
             LOGGER.warn("Unable to check if impression event should be emitted", e);
@@ -232,28 +242,34 @@ private FeatureEvaluationResult getFeatureEvaluationResult(
             @Nullable Variant defaultVariant) {
         UnleashContext enhancedContext = context.applyStaticFields(config);
         try {
-            VariantDef variantResponse = this.unleashEngine.getVariant(toggleName, adapt(enhancedContext));
+            VariantDef variantResponse =
+                    this.unleashEngine.getVariant(toggleName, adapt(enhancedContext));
             if (variantResponse != null) {
-                return new FeatureEvaluationResult(variantResponse.isEnabled(),
-                    new Variant(variantResponse.getName(), adapt(variantResponse.getPayload()), variantResponse.isEnabled()));
+                return new FeatureEvaluationResult(
+                        variantResponse.isEnabled(),
+                        new Variant(
+                                variantResponse.getName(),
+                                adapt(variantResponse.getPayload()),
+                                variantResponse.isEnabled()));
             }
 
             Boolean isEnabled = this.unleashEngine.isEnabled(toggleName, adapt(enhancedContext));
             if (isEnabled != null) {
                 return new FeatureEvaluationResult(isEnabled, defaultVariant);
             }
 
-            return new FeatureEvaluationResult(fallbackAction.test(toggleName, enhancedContext), defaultVariant);
+            return new FeatureEvaluationResult(
+                    fallbackAction.test(toggleName, enhancedContext), defaultVariant);
 
         } catch (YggdrasilInvalidInputException | YggdrasilError e) {
             throw new RuntimeException(e);
         }
     }
 
     private @Nullable io.getunleash.variant.Payload adapt(@Nullable Payload payload) {
-        return Optional.ofNullable(payload).map(p ->
-            new io.getunleash.variant.Payload(p.getType(), p.getValue())
-        ).orElse(null);
+        return Optional.ofNullable(payload)
+                .map(p -> new io.getunleash.variant.Payload(p.getType(), p.getValue()))
+                .orElse(null);
     }
 
     @Override

diff --git a/src/main/java/io/getunleash/repository/FeatureRepository.java b/src/main/java/io/getunleash/repository/FeatureRepository.java
@@ -9,7 +9,6 @@
 import io.getunleash.util.Throttler;
 import io.getunleash.util.UnleashConfig;
 import io.getunleash.util.UnleashScheduledExecutor;
-
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
@@ -143,7 +142,8 @@ private Runnable updateFeatures(@Nullable final Consumer<UnleashException> handl
                                         LOGGER.error("Error when calling consumer {}", consumer, e);
                                     }
                                 });
-                        // Note: this could be a consumer as well, but we need to differentiate it to be able to read from the backup
+                        // Note: this could be a consumer as well, but we need to differentiate it
+                        // to be able to read from the backup
                         featureBackupHandler.write(featureCollection);
                     } else if (response.getStatus() == ClientFeaturesResponse.Status.UNAVAILABLE) {
                         throttler.handleHttpErrorCodes(response.getHttpStatusCode());

diff --git a/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java b/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java
@@ -37,7 +37,8 @@ public ClientFeaturesResponse fetchFeatures() throws UnleashException {
 
             return getFeatureResponse(connection, true);
         } catch (IOException e) {
-            throw new UnleashException(String.format("Could not fetch toggles from %s", this.toggleUrl), e);
+            throw new UnleashException(
+                    String.format("Could not fetch toggles from %s", this.toggleUrl), e);
         } catch (IllegalStateException e) {
             throw new UnleashException(e.getMessage(), e);
         } finally {

diff --git a/src/main/java/io/getunleash/repository/IFeatureRepository.java b/src/main/java/io/getunleash/repository/IFeatureRepository.java
@@ -2,7 +2,6 @@
 
 import io.getunleash.Segment;
 import io.getunleash.lang.Nullable;
-
 import java.util.function.Consumer;
 
 public interface IFeatureRepository extends ToggleRepository {

diff --git a/src/main/java/io/getunleash/repository/JsonFeatureParser.java b/src/main/java/io/getunleash/repository/JsonFeatureParser.java
@@ -2,7 +2,6 @@
 
 import com.google.gson.Gson;
 import com.google.gson.GsonBuilder;
-
 import java.io.Reader;
 
 public final class JsonFeatureParser {

diff --git a/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java b/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java
@@ -76,7 +76,6 @@ public ClientFeaturesResponse fetchFeatures() throws UnleashException {
                 FeatureCollection features =
                         JsonFeatureParser.fromJson(
                                 Objects.requireNonNull(response.body()).charStream());
-
                 return new ClientFeaturesResponse(
                         ClientFeaturesResponse.Status.CHANGED,
                         features.getToggleCollection(),

diff --git a/src/main/java/io/getunleash/strategy/Strategy.java b/src/main/java/io/getunleash/strategy/Strategy.java
@@ -15,6 +15,10 @@ public interface Strategy {
 
     boolean isEnabled(Map<String, String> parameters);
 
+    /**
+     * @deprecated don't use this method, currently only accessible from tests
+     */
+    @Deprecated
     default FeatureEvaluationResult getResult(
             Map<String, String> parameters,
             UnleashContext unleashContext,
@@ -30,6 +34,10 @@ default boolean isEnabled(Map<String, String> parameters, UnleashContext unleash
         return isEnabled(parameters);
     }
 
+    /**
+     * @deprecated constraint validation should be delegated to Yggdrasil
+     */
+    @Deprecated
     default boolean isEnabled(
             Map<String, String> parameters,
             UnleashContext unleashContext,

diff --git a/src/test/java/io/getunleash/DefaultUnleashTest.java b/src/test/java/io/getunleash/DefaultUnleashTest.java
@@ -87,9 +87,11 @@ public void should_evaluate_missing_segment_as_false() {
                         asList(semverConstraint),
                         asList(404),
                         Collections.emptyList());
-        new UnleashEngineStateHandler(sut).setState(
-            Collections.singletonList(new FeatureToggle(toggleName, true, asList(withMissingSegment))),
-            Collections.singletonList(Segment.DENY_SEGMENT));
+        new UnleashEngineStateHandler(sut)
+                .setState(
+                        Collections.singletonList(
+                                new FeatureToggle(toggleName, true, asList(withMissingSegment))),
+                        Collections.singletonList(Segment.DENY_SEGMENT));
 
         when(contextProvider.getContext())
                 .thenReturn(UnleashContext.builder().addProperty("version", semVer).build());
@@ -107,17 +109,19 @@ public void should_evaluate_segment_collection_with_one_missing_segment_as_false
                         asList(semverConstraint),
                         asList(404, 1),
                         Collections.emptyList());
-        new UnleashEngineStateHandler(sut).setState(
-            Collections.singletonList(new FeatureToggle(toggleName, true, asList(withMissingSegment))),
-            Collections.singletonList(new Segment(
-                1,
-                "always true",
-                asList(
-                    new Constraint(
-                        "always_true",
-                        Operator.NOT_IN,
-                        Collections.EMPTY_LIST)))));
-
+        new UnleashEngineStateHandler(sut)
+                .setState(
+                        Collections.singletonList(
+                                new FeatureToggle(toggleName, true, asList(withMissingSegment))),
+                        Collections.singletonList(
+                                new Segment(
+                                        1,
+                                        "always true",
+                                        asList(
+                                                new Constraint(
+                                                        "always_true",
+                                                        Operator.NOT_IN,
+                                                        Collections.EMPTY_LIST)))));
 
         when(contextProvider.getContext())
                 .thenReturn(UnleashContext.builder().addProperty("version", "1.2.2").build());
@@ -151,7 +155,8 @@ public void should_allow_fallback_strategy() {
 
         sut.isEnabled("toggle1");
 
-        // PR-comment: constraints are no longer managed by the SDK but by Yggdrasil, so we removed the third parameter
+        // PR-comment: constraints are no longer managed by the SDK but by Yggdrasil, so we removed
+        // the third parameter
         verify(fallback).isEnabled(any(), any());
     }
 

diff --git a/src/test/java/io/getunleash/DependentFeatureToggleTest.java b/src/test/java/io/getunleash/DependentFeatureToggleTest.java
@@ -123,7 +123,8 @@ public void should_trigger_impression_event_for_parent_toggle_when_checking_chil
         stateHandler.setState(child, parent);
         boolean enabled = sut.isEnabled("child", UnleashContext.builder().userId("7").build());
         assertThat(enabled).isTrue();
-        // if child does not have impression event enabled, even if the parent has them, we're not triggering impression event
+        // if child does not have impression event enabled, even if the parent has them, we're not
+        // triggering impression event
         verify(eventDispatcher, never()).dispatch(any(IsEnabledImpressionEvent.class));
     }
 
@@ -148,8 +149,7 @@ public void should_trigger_impression_event_for_parent_variant_when_checking_chi
         stateHandler.setState(child, parent);
         Variant variant = sut.getVariant("child", UnleashContext.builder().userId("7").build());
         assertThat(variant).isNotNull();
-        // TODO should this be 1? Now the SDK doesn't know whether about the parent/child relationship and the Engine will be checked only once
-        verify(eventDispatcher, times(2)).dispatch(any(VariantImpressionEvent.class));
+        verify(eventDispatcher, times(1)).dispatch(any(VariantImpressionEvent.class));
     }
 
     @Test