Repository: Unleash/unleash-client-java
Pull request #135 (2021-04-20 01:40:03)

Add support for leveraging namePrefix in Java SDK
Also warn if trying to check for toggles that don't match the prefix to avoid confusion for why it is always disabled.

ivarconr: Read quickly through, and the only concern i have is if the implementation will allow multiple query fields at the same time (e.g. project and namsPrefix)? 
checketts: This implementation does not support that. I was just reading through the node code and realized that was supported, so I'll update this PR to support that as well.
checketts: I've updated the code to support both project and namePrefix now.
checketts: I just added the changelog entry. Sorry I had forgotten that before!

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 96f9ee7c..f8a33bf7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 ## 4.2.1
 - fix: Make sure empty bootstrap path does not crash app
 - feat: Default to trying UNLEASH_BOOTSTRAP_FILE environment variable for finding bootstrap
+- feat: Add support for filtering by namePrefix
 
 ## 4.2.0
 - feat: Add ability to bootstrap features from files via UNLEASH_BOOTSTRAP_FILE (#129)
diff --git a/src/main/java/no/finn/unleash/DefaultUnleash.java b/src/main/java/no/finn/unleash/DefaultUnleash.java
index fdb54927..712e4bc6 100644
--- a/src/main/java/no/finn/unleash/DefaultUnleash.java
+++ b/src/main/java/no/finn/unleash/DefaultUnleash.java
@@ -126,6 +126,7 @@ public final class DefaultUnleash implements Unleash {
             String toggleName,
             UnleashContext context,
             BiFunction<String, UnleashContext, Boolean> fallbackAction) {
+        checkIfToggleMatchesNamePrefix(toggleName);
         FeatureToggle featureToggle = toggleRepository.getToggle(toggleName);
         boolean enabled;
         UnleashContext enhancedContext = context.applyStaticFields(config);
@@ -159,6 +160,17 @@ public final class DefaultUnleash implements Unleash {
         return enabled;
     }
 
+    private void checkIfToggleMatchesNamePrefix(String toggleName) {
+        if (config.getNamePrefix() != null) {
+            if (!toggleName.startsWith(config.getNamePrefix())) {
+                LOGGER.warn(
+                        "Toggle [{}] doesnt start with configured name prefix of [{}] so it will always be disabled",
+                        toggleName,
+                        config.getNamePrefix());
+            }
+        }
+    }
+
     @Override
     public Variant getVariant(String toggleName, UnleashContext context) {
         return getVariant(toggleName, context, DISABLED_VARIANT);
diff --git a/src/main/java/no/finn/unleash/repository/HttpToggleFetcher.java b/src/main/java/no/finn/unleash/repository/HttpToggleFetcher.java
index 43f210f7..2d12f066 100644
--- a/src/main/java/no/finn/unleash/repository/HttpToggleFetcher.java
+++ b/src/main/java/no/finn/unleash/repository/HttpToggleFetcher.java
@@ -20,18 +20,13 @@ public final class HttpToggleFetcher implements ToggleFetcher {
     private Optional<String> etag = Optional.empty();
 
     private final URL toggleUrl;
-    private UnleashConfig unleashConfig;
+    private final UnleashConfig unleashConfig;
 
     public HttpToggleFetcher(UnleashConfig unleashConfig) {
         this.unleashConfig = unleashConfig;
-        if (unleashConfig.getProjectName() != null) {
-            this.toggleUrl =
-                    unleashConfig
-                            .getUnleashURLs()
-                            .getFetchTogglesURL(unleashConfig.getProjectName());
-        } else {
-            this.toggleUrl = unleashConfig.getUnleashURLs().getFetchTogglesURL();
-        }
+        this.toggleUrl = unleashConfig
+            .getUnleashURLs()
+            .getFetchTogglesURL(unleashConfig.getProjectName(), unleashConfig.getNamePrefix());
     }
 
     @Override
diff --git a/src/main/java/no/finn/unleash/repository/ToggleBootstrapHandler.java b/src/main/java/no/finn/unleash/repository/ToggleBootstrapHandler.java
index a360df62..78464be1 100644
--- a/src/main/java/no/finn/unleash/repository/ToggleBootstrapHandler.java
+++ b/src/main/java/no/finn/unleash/repository/ToggleBootstrapHandler.java
@@ -43,7 +43,7 @@ public class ToggleBootstrapHandler {
         return new ToggleCollection(Collections.emptyList());
     }
 
-    private static class ToggleBootstrapRead implements UnleashEvent {
+    public static class ToggleBootstrapRead implements UnleashEvent {
         private final ToggleCollection toggleCollection;
 
         private ToggleBootstrapRead(ToggleCollection toggleCollection) {
diff --git a/src/main/java/no/finn/unleash/util/UnleashConfig.java b/src/main/java/no/finn/unleash/util/UnleashConfig.java
index fb08f3a8..c02560f6 100644
--- a/src/main/java/no/finn/unleash/util/UnleashConfig.java
+++ b/src/main/java/no/finn/unleash/util/UnleashConfig.java
@@ -31,6 +31,7 @@ public class UnleashConfig {
     private final String sdkVersion;
     private final String backupFile;
     @Nullable private final String projectName;
+    @Nullable private final String namePrefix;
     private final long fetchTogglesInterval;
     private final long sendMetricsInterval;
     private final boolean disableMetrics;
@@ -53,6 +54,7 @@ public class UnleashConfig {
             String sdkVersion,
             String backupFile,
             @Nullable String projectName,
+            @Nullable String namePrefix,
             long fetchTogglesInterval,
             long sendMetricsInterval,
             boolean disableMetrics,
@@ -108,6 +110,7 @@ public class UnleashConfig {
         this.sdkVersion = sdkVersion;
         this.backupFile = backupFile;
         this.projectName = projectName;
+        this.namePrefix = namePrefix;
         this.fetchTogglesInterval = fetchTogglesInterval;
         this.sendMetricsInterval = sendMetricsInterval;
         this.disableMetrics = disableMetrics;
@@ -220,6 +223,11 @@ public class UnleashConfig {
         return toggleBootstrapProvider;
     }
 
+    @Nullable
+    public String getNamePrefix() {
+        return namePrefix;
+    }
+
     @Nullable
     public Proxy getProxy() {
         return proxy;
@@ -283,9 +291,10 @@ public class UnleashConfig {
         private @Nullable String appName;
         private String environment = "default";
         private String instanceId = getDefaultInstanceId();
-        private String sdkVersion = getDefaultSdkVersion();
+        private final String sdkVersion = getDefaultSdkVersion();
         private @Nullable String backupFile;
         private @Nullable String projectName;
+        private @Nullable String namePrefix;
         private long fetchTogglesInterval = 10;
         private long sendMetricsInterval = 60;
         private boolean disableMetrics = false;
@@ -295,7 +304,7 @@ public class UnleashConfig {
         private @Nullable UnleashScheduledExecutor scheduledExecutor;
         private @Nullable UnleashSubscriber unleashSubscriber;
         private boolean isProxyAuthenticationByJvmProperties;
-        private Strategy fallbackStrategy;
+        private @Nullable Strategy fallbackStrategy;
         private @Nullable ToggleBootstrapProvider toggleBootstrapProvider;
         private @Nullable Proxy proxy;
         private @Nullable Authenticator proxyAuthenticator;
@@ -355,6 +364,11 @@ public class UnleashConfig {
             return this;
         }
 
+        public Builder namePrefix(String namePrefix) {
+            this.namePrefix = namePrefix;
+            return this;
+        }
+
         public Builder fetchTogglesInterval(long fetchTogglesInterval) {
             this.fetchTogglesInterval = fetchTogglesInterval;
             return this;
@@ -446,6 +460,7 @@ public class UnleashConfig {
                     sdkVersion,
                     getBackupFile(),
                     projectName,
+                    namePrefix,
                     fetchTogglesInterval,
                     sendMetricsInterval,
                     disableMetrics,
diff --git a/src/main/java/no/finn/unleash/util/UnleashURLs.java b/src/main/java/no/finn/unleash/util/UnleashURLs.java
index 23760c5d..702365fc 100644
--- a/src/main/java/no/finn/unleash/util/UnleashURLs.java
+++ b/src/main/java/no/finn/unleash/util/UnleashURLs.java
@@ -1,7 +1,10 @@
 package no.finn.unleash.util;
 
+import no.finn.unleash.lang.Nullable;
+
 import java.net.MalformedURLException;
 import java.net.URI;
+import java.net.URISyntaxException;
 import java.net.URL;
 
 public class UnleashURLs {
@@ -25,17 +28,6 @@ public class UnleashURLs {
         return fetchTogglesURL;
     }
 
-    public URL getFetchTogglesURL(String projectName) {
-        try {
-            return URI.create(fetchTogglesURL.toString() + "?project=" + projectName)
-                    .normalize()
-                    .toURL();
-        } catch (Exception e) {
-            throw new IllegalArgumentException(
-                    "Project name [" + projectName + "] was not URL friendly.", e);
-        }
-    }
-
     public URL getClientMetricsURL() {
         return clientMetricsURL;
     }
@@ -43,4 +35,28 @@ public class UnleashURLs {
     public URL getClientRegisterURL() {
         return clientRegisterURL;
     }
+
+    public URL getFetchTogglesURL(@Nullable String projectName, @Nullable String namePrefix) {
+        StringBuilder suffix = new StringBuilder("");
+        appendParam(suffix, "project", projectName);
+        appendParam(suffix, "namePrefix", namePrefix);
+
+        try {
+            return URI.create(fetchTogglesURL + suffix.toString()).normalize().toURL();
+        } catch (IllegalArgumentException|MalformedURLException e) {
+            throw new IllegalArgumentException("fetchTogglesURL [" + fetchTogglesURL + suffix + "] was not URL friendly.", e);
+        }
+    }
+
+    private void appendParam(StringBuilder suffix, String key, @Nullable String value) {
+        if(value == null) return;
+        if(suffix.length() == 0){
+            suffix.append("?");
+        } else {
+            suffix.append("&");
+        }
+        suffix.append(key).append("=").append(value);
+    }
+
+
 }


Test patch:
diff --git a/src/test/java/no/finn/unleash/event/SubscriberTest.java b/src/test/java/no/finn/unleash/event/SubscriberTest.java
index a1b4e0c0..0b96515a 100644
--- a/src/test/java/no/finn/unleash/event/SubscriberTest.java
+++ b/src/test/java/no/finn/unleash/event/SubscriberTest.java
@@ -16,7 +16,11 @@ import no.finn.unleash.DefaultUnleash;
 import no.finn.unleash.SynchronousTestExecutor;
 import no.finn.unleash.Unleash;
 import no.finn.unleash.UnleashException;
+import no.finn.unleash.metric.ClientMetrics;
+import no.finn.unleash.metric.ClientRegistration;
 import no.finn.unleash.repository.FeatureToggleResponse;
+import no.finn.unleash.repository.HttpToggleFetcher;
+import no.finn.unleash.repository.ToggleBootstrapHandler;
 import no.finn.unleash.util.UnleashConfig;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -61,15 +65,14 @@ public class SubscriberTest {
         assertThat(testSubscriber.toggleEnabled).isFalse();
         assertThat(testSubscriber.errors).hasSize(2);
 
-        assertThat(testSubscriber.events)
-                .hasSize(
-                        1 // Bootstrapping
-                                + 3 // feature evaluations
-                                + 2 // toggle fetches
-                                + 1 // unleash ready
-                                + 1 // client registration
-                                + 1 // client metrics
-                        );
+//        assertThat(testSubscriber.events).filteredOn(e -> e instanceof ToggleBootstrapHandler.ToggleBootstrapRead).hasSize(1);
+        assertThat(testSubscriber.events).filteredOn(e -> e instanceof UnleashReady).hasSize(1);
+        assertThat(testSubscriber.events).filteredOn(e -> e instanceof ToggleEvaluated).hasSize(3);
+        assertThat(testSubscriber.events).filteredOn(e -> e instanceof FeatureToggleResponse).hasSize(2);
+        assertThat(testSubscriber.events).filteredOn(e -> e instanceof ClientRegistration).hasSize(1);
+        assertThat(testSubscriber.events).filteredOn(e -> e instanceof ClientMetrics).hasSize(1);
+        assertThat(testSubscriber.events).hasSize(8);
+
     }
 
     private class TestSubscriber implements UnleashSubscriber {
diff --git a/src/test/java/no/finn/unleash/repository/HttpToggleFetcherTest.java b/src/test/java/no/finn/unleash/repository/HttpToggleFetcherTest.java
index 0cad9065..5e4f9f9c 100644
--- a/src/test/java/no/finn/unleash/repository/HttpToggleFetcherTest.java
+++ b/src/test/java/no/finn/unleash/repository/HttpToggleFetcherTest.java
@@ -346,6 +346,6 @@ public class HttpToggleFetcherTest {
                 UnleashConfig.builder().appName("test").unleashAPI(uri).projectName(name).build();
         org.assertj.core.api.Assertions.assertThatThrownBy(() -> new HttpToggleFetcher(config))
                 .isInstanceOf(IllegalArgumentException.class)
-                .hasMessage("Project name [" + name + "] was not URL friendly.");
+                .hasMessageContaining("?project="+name+"] was not URL friendly.");
     }
 }
diff --git a/src/test/java/no/finn/unleash/util/UnleashURLsTest.java b/src/test/java/no/finn/unleash/util/UnleashURLsTest.java
index ef23c972..922d40ee 100644
--- a/src/test/java/no/finn/unleash/util/UnleashURLsTest.java
+++ b/src/test/java/no/finn/unleash/util/UnleashURLsTest.java
@@ -36,6 +36,35 @@ public class UnleashURLsTest {
                 .isEqualTo("http://unleash.com/api/client/features");
     }
 
+    @Test
+    public void should_set_build_fetch_url_if_project_and_prefix_are_null() {
+        UnleashURLs urls = new UnleashURLs(URI.create("http://unleash.com/api/"));
+        assertThat(urls.getFetchTogglesURL(null, null).toString())
+            .isEqualTo("http://unleash.com/api/client/features");
+    }
+
+    @Test
+    public void should_set_build_fetch_url_with_project() {
+        UnleashURLs urls = new UnleashURLs(URI.create("http://unleash.com/api/"));
+        assertThat(urls.getFetchTogglesURL("myProject", null).toString())
+            .isEqualTo("http://unleash.com/api/client/features?project=myProject");
+    }
+
+    @Test
+    public void should_set_build_fetch_url_with_nameprefix() {
+        UnleashURLs urls = new UnleashURLs(URI.create("http://unleash.com/api/"));
+        assertThat(urls.getFetchTogglesURL(null, "prefix.").toString())
+            .isEqualTo("http://unleash.com/api/client/features?namePrefix=prefix.");
+    }
+
+
+    @Test
+    public void should_set_build_fetch_url_with_project_and_nameprefix() {
+        UnleashURLs urls = new UnleashURLs(URI.create("http://unleash.com/api/"));
+        assertThat(urls.getFetchTogglesURL("aproject", "prefix.").toString())
+            .isEqualTo("http://unleash.com/api/client/features?project=aproject&namePrefix=prefix.");
+    }
+
     @Test()
     public void should_throw() {
         assertThrows(