From 2e7bdc6e7b3ef1cec743622f2ce7ea4a885cb52a Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 15:12:04 +0000 Subject: [PATCH 1/5] Test that the old cloud endpoint names still work A patch release keeps every public name, and the name of an environment variable a package reads is a public name. These tests ask for FOD_CLOUD_API_URL to be read when _51DEGREES_CLOUD_ENDPOINT is not set, for _51DEGREES_CLOUD_ENDPOINT to win when both are set, for the use of the old name to be logged as deprecated once, and for the two constants that held the old name to hold it again. They cover the cloud request engine builder, the 51Did client, which is run in a Java process of its own so that it reads a real environment, and the cloud engine example. The cases of the environment variable name check now expect the old endpoint name to be allowed. The tests fail until the fix that follows. --- ci/check-environment-variable-names.Tests.ps1 | 17 +- .../CloudRequestEngineBuilderTests.java | 144 +++++++++++++ .../cloudengine/EndpointTests.java | 79 ++++++++ .../did/DidClientEnvironmentTests.java | 190 ++++++++++++++++++ .../fiftyone/pipeline/did/EndpointProbe.java | 71 +++++++ 5 files changed, 497 insertions(+), 4 deletions(-) create mode 100644 pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java create mode 100644 pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientEnvironmentTests.java create mode 100644 pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/EndpointProbe.java diff --git a/ci/check-environment-variable-names.Tests.ps1 b/ci/check-environment-variable-names.Tests.ps1 index 7b9273d80..669629a6e 100644 --- a/ci/check-environment-variable-names.Tests.ps1 +++ b/ci/check-environment-variable-names.Tests.ps1 @@ -32,15 +32,11 @@ Describe 'Find-EnvironmentNameProblem' { @{ Line = 'System.getenv("RESOURCE_KEY")' } @{ Line = 'env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY");' } @{ Line = 'env("_51DEGREES_LICENSE_KEY", "LICENSE_KEY");' } - @{ Line = 'public static final String FOD = "FOD_CLOUD_API_URL";' } @{ Line = '$env:LICENSE_KEY = $Keys.TestResourceKey' } @{ Line = '${env:LICENSE_KEY} = $Keys.TestResourceKey' } @{ Line = '${Env:RESOURCE_KEY}' } @{ Line = '$ENV:RESOURCE_KEY = "key"' } @{ Line = 'Remove-Item Env:\LICENSE_KEY' } - @{ Line = '${env:FOD_CLOUD_API_URL}' } - @{ Line = ' url: ${{ env.FOD_CLOUD_API_URL }}' } - @{ Line = '${FOD_CLOUD_API_URL}' } @{ Line = 'Constants.LEGACY_ENV_VARS' } @{ Line = ' LICENSE_KEY: ${{ secrets.DEVICE_DETECTION_KEY }}' } @{ Line = '- RESOURCE_KEY names the resource key' } @@ -68,6 +64,19 @@ Describe 'Find-EnvironmentNameProblem' { ) { Get-Kind $Line | Should -BeNullOrEmpty } + + # The cloud endpoint is still read from its deprecated name when the + # current name is not set, so that name is allowed wherever it appears. + It 'leaves the deprecated endpoint name alone in ' -ForEach @( + @{ Line = 'public static final String FOD = "FOD_CLOUD_API_URL";' } + @{ Line = 'System.getenv("FOD_CLOUD_API_URL")' } + @{ Line = '${env:FOD_CLOUD_API_URL}' } + @{ Line = ' url: ${{ env.FOD_CLOUD_API_URL }}' } + @{ Line = '${FOD_CLOUD_API_URL}' } + @{ Line = 'FOD_CLOUD_API_URL=https://host/api/v4/ mvn test' } + ) { + Get-Kind $Line | Should -BeNullOrEmpty + } } Describe 'the check over a repository' { diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilderTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilderTests.java index 87851ceb0..8159049a0 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilderTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilderTests.java @@ -22,9 +22,11 @@ package fiftyone.pipeline.cloudrequestengine.flowelements; +import fiftyone.common.testhelpers.TestLogger; import fiftyone.pipeline.cloudrequestengine.CloudRequestException; import fiftyone.pipeline.core.exceptions.PipelineConfigurationException; import fiftyone.pipeline.engines.services.HttpClientDefault; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.slf4j.LoggerFactory; @@ -32,6 +34,10 @@ import java.io.IOException; import java.lang.reflect.Field; import java.net.MalformedURLException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.function.Function; import static org.junit.jupiter.api.Assertions.*; @@ -40,6 +46,17 @@ public class CloudRequestEngineBuilderTests extends CloudRequestEngineTestsBase private final String testResourceKey = "resource_key"; private final String testEndpoint = "https://testEndpoint/"; private final String testEnvVarEndpoint = "https://testEnvVarEndpoint/"; + private final String testOldEnvVarEndpoint = + "https://testOldEnvVarEndpoint/"; + + // The deprecated name of the end point variable, which is also the + // name of the constant that holds it. + private static final String OLD_ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL"; + + // The old name is logged as deprecated once in a process, so what was + // logged is carried from one test of this class to the next. + private static final List deprecationsInEarlierTests = + new ArrayList<>(); public CloudRequestEngineBuilderTests() throws MalformedURLException { super(); @@ -49,6 +66,11 @@ public CloudRequestEngineBuilderTests() throws MalformedURLException { public void setUp() throws IOException { configureMockedClient(); } + + @AfterEach + public void countDeprecations() { + deprecationsInEarlierTests.addAll(deprecationsLogged()); + } @Test @SuppressWarnings("unused") @@ -122,4 +144,126 @@ public void CloudEndPoint_Default_Setting() throws Exception { assertEquals( Constants.END_POINT_DEFAULT + "/", getEndpointValue(builder)); } + + /** + * Gives the builder an environment holding only the variables in the + * map, so that a test chooses which of the two names is set. + */ + private void setEnvironment( + CloudRequestEngineBuilder builder, + Map environment) throws Exception { + Field getEnvVarField = builder.getClass().getDeclaredField("getEnvVar"); + getEnvVarField.setAccessible(true); + getEnvVarField.set( + builder, + (Function)(name) -> environment.get(name)); + } + + /** + * A builder that finds the old name in the environment, and the new + * name only where a value is given for it. + */ + private CloudRequestEngineBuilder builderWithOldName( + String newNameValue) throws Exception { + Map environment = new HashMap<>(); + environment.put(OLD_ENDPOINT_VARIABLE, testOldEnvVarEndpoint); + if (newNameValue != null) { + environment.put(Constants.CLOUD_ENDPOINT_VARIABLE, newNameValue); + } + CloudRequestEngineBuilder builder = + new CloudRequestEngineBuilder(loggerFactory, httpClient); + setEnvironment(builder, environment); + return builder.setResourceKey(testResourceKey); + } + + /** + * The warnings this test's loggers hold that name the old variable. + */ + private List deprecationsLogged() { + List result = new ArrayList<>(); + for (TestLogger logger : loggerFactory.loggers) { + for (String warning : logger.warningsLogged) { + if (warning.contains(OLD_ENDPOINT_VARIABLE)) { + result.add(warning); + } + } + } + return result; + } + + @Test + public void CloudEndPoint_Old_Environment_Name() throws Exception { + CloudRequestEngineBuilder builder = builderWithOldName(null); + + builder.build(); + + assertEquals(testOldEnvVarEndpoint, getEndpointValue(builder)); + assertEquals( + testOldEnvVarEndpoint + "accessibleproperties", + getPropertiesEndpointValue(builder)); + } + + @Test + public void CloudEndPoint_Old_Environment_Name_When_New_Is_Empty() + throws Exception { + CloudRequestEngineBuilder builder = builderWithOldName(""); + + builder.build(); + + assertEquals(testOldEnvVarEndpoint, getEndpointValue(builder)); + } + + @Test + public void CloudEndPoint_New_Environment_Name_Wins() throws Exception { + CloudRequestEngineBuilder builder = + builderWithOldName(testEnvVarEndpoint); + + builder.build(); + + assertEquals(testEnvVarEndpoint, getEndpointValue(builder)); + assertEquals( + testEnvVarEndpoint + "accessibleproperties", + getPropertiesEndpointValue(builder)); + assertEquals(0, deprecationsLogged().size()); + } + + @Test + public void CloudEndPoint_Explicit_Setting_Beats_Old_Name() + throws Exception { + CloudRequestEngineBuilder builder = builderWithOldName(null); + + builder.setEndpoint(testEndpoint).build(); + + assertEquals(testEndpoint, getEndpointValue(builder)); + assertEquals(0, deprecationsLogged().size()); + } + + /** + * Two engines built from the old name log one warning between them, + * however many of this class's tests read the old name before this one. + */ + @Test + public void CloudEndPoint_Old_Environment_Name_Logged_Once() + throws Exception { + builderWithOldName(null).build(); + builderWithOldName(null).build(); + + List logged = new ArrayList<>(deprecationsInEarlierTests); + logged.addAll(deprecationsLogged()); + assertEquals(1, logged.size(), logged.toString()); + String warning = logged.get(0); + assertTrue( + warning.contains(Constants.CLOUD_ENDPOINT_VARIABLE), warning); + assertTrue(warning.contains("deprecated"), warning); + } + + @Test + public void CloudEndPoint_Old_Constant_Is_Kept() throws Exception { + Field old = Constants.class.getField(OLD_ENDPOINT_VARIABLE); + + assertEquals(OLD_ENDPOINT_VARIABLE, old.get(null)); + assertTrue(old.isAnnotationPresent(Deprecated.class)); + assertEquals( + "_51DEGREES_CLOUD_ENDPOINT", Constants.CLOUD_ENDPOINT_VARIABLE); + } } diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java new file mode 100644 index 000000000..cf9d27bd0 --- /dev/null +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java @@ -0,0 +1,79 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package pipeline.developerexamples.cloudengine; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Tests which name the example takes its endpoint from. The example reads + * each name as an environment variable and then as a system property, so + * these tests set system properties, and are skipped where the environment + * already sets either name, because the environment would answer first. + */ +public class EndpointTests { + + private static final String NEW_NAME = "_51DEGREES_CLOUD_ENDPOINT"; + private static final String OLD_NAME = "FOD_CLOUD_API_URL"; + + @BeforeEach + public void setUp() { + assumeTrue(System.getenv(NEW_NAME) == null); + assumeTrue(System.getenv(OLD_NAME) == null); + clear(); + } + + @AfterEach + public void clear() { + System.clearProperty(NEW_NAME); + System.clearProperty(OLD_NAME); + } + + @Test + public void Endpoint_Old_Name_Alone() { + System.setProperty(OLD_NAME, "https://old.example/api/v4"); + + assertEquals("https://old.example/api/v4/", Main.endpoint()); + } + + @Test + public void Endpoint_New_Name_Wins() { + System.setProperty(NEW_NAME, "https://new.example/api/v4/"); + System.setProperty(OLD_NAME, "https://old.example/api/v4/"); + + assertEquals("https://new.example/api/v4/", Main.endpoint()); + } + + @Test + public void Endpoint_Neither_Name() { + String expected = Main.configuredResourceKey() == null + ? "https://51degrees.com/starsign/api/" + : "https://cloud.51degrees.com/api/v4/"; + + assertEquals(expected, Main.endpoint()); + } +} diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientEnvironmentTests.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientEnvironmentTests.java new file mode 100644 index 000000000..562bbddb2 --- /dev/null +++ b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientEnvironmentTests.java @@ -0,0 +1,190 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.did; + +import org.json.JSONObject; +import org.junit.Test; + +import java.io.BufferedReader; +import java.io.File; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * Tests which environment variable {@link DidClient} takes its endpoint + * from. Each test starts {@link EndpointProbe} in a Java process of its own + * with the variables it needs, so the client reads a real environment. + */ +public class DidClientEnvironmentTests { + + private static final String NEW_NAME = "_51DEGREES_CLOUD_ENDPOINT"; + private static final String OLD_NAME = "FOD_CLOUD_API_URL"; + + private static final String NEW_ENDPOINT = "https://new.example/api/v4/"; + private static final String OLD_ENDPOINT = "https://old.example/api/v4/"; + + private static final int PROBE_TIMEOUT_SECONDS = 120; + + @Test + public void oldNameAlone_GivesItsEndpoint() throws Exception { + List output = probe(null, OLD_ENDPOINT); + + assertEquals( + Arrays.asList(OLD_ENDPOINT, OLD_ENDPOINT), endpoints(output)); + } + + @Test + public void oldNameAlone_IsLoggedAsDeprecatedOnce() throws Exception { + List logged = logged(probe(null, OLD_ENDPOINT)); + + assertEquals(logged.toString(), 1, logged.size()); + String record = logged.get(0); + assertTrue(record, record.startsWith("WARNING ")); + assertTrue(record, record.contains(OLD_NAME)); + assertTrue(record, record.contains(NEW_NAME)); + assertTrue(record, record.contains("deprecated")); + } + + @Test + public void newNameBlank_OldNameIsRead() throws Exception { + List output = probe(" ", OLD_ENDPOINT); + + assertEquals( + Arrays.asList(OLD_ENDPOINT, OLD_ENDPOINT), endpoints(output)); + } + + @Test + public void bothNames_NewNameWins() throws Exception { + List output = probe(NEW_ENDPOINT, OLD_ENDPOINT); + + assertEquals( + Arrays.asList(NEW_ENDPOINT, NEW_ENDPOINT), endpoints(output)); + assertEquals(output.toString(), 0, logged(output).size()); + } + + @Test + public void neitherName_GivesTheDefault() throws Exception { + List output = probe(null, null); + + assertEquals( + Arrays.asList( + DidClient.DEFAULT_ENDPOINT, DidClient.DEFAULT_ENDPOINT), + endpoints(output)); + assertEquals(output.toString(), 0, logged(output).size()); + } + + @Test + @SuppressWarnings("deprecation") + public void oldConstant_KeepsItsNameAndValue() throws Exception { + assertEquals(OLD_NAME, DidClient.ENDPOINT_VARIABLE); + assertTrue(DidClient.class.getField("ENDPOINT_VARIABLE") + .isAnnotationPresent(Deprecated.class)); + assertEquals( + NEW_NAME, + DidClient.class.getField("CLOUD_ENDPOINT_VARIABLE").get(null)); + } + + /** + * Runs the probe with each variable set to the value given, or absent + * where the value is null, and returns the lines it printed. + */ + private static List probe(String newValue, String oldValue) + throws Exception { + String java = Paths.get( + System.getProperty("java.home"), "bin", "java").toString(); + ProcessBuilder builder = + new ProcessBuilder(java, EndpointProbe.class.getName()); + Map environment = builder.environment(); + environment.put("CLASSPATH", classPath()); + set(environment, NEW_NAME, newValue); + set(environment, OLD_NAME, oldValue); + builder.redirectErrorStream(true); + + Process process = builder.start(); + List output = new ArrayList<>(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader( + process.getInputStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + output.add(line); + } + } + assertTrue( + "The probe did not finish.", + process.waitFor(PROBE_TIMEOUT_SECONDS, TimeUnit.SECONDS)); + assertEquals(output.toString(), 0, process.exitValue()); + return output; + } + + private static void set( + Map environment, String name, String value) { + if (value == null) { + environment.remove(name); + } else { + environment.put(name, value); + } + } + + /** + * The places the probe loads its classes from, being the classes of + * this module, its test classes and the JSON library the client uses. + */ + private static String classPath() throws Exception { + return location(DidClient.class) + File.pathSeparator + + location(EndpointProbe.class) + File.pathSeparator + + location(JSONObject.class); + } + + private static String location(Class type) throws Exception { + return Paths.get(type.getProtectionDomain().getCodeSource() + .getLocation().toURI()).toString(); + } + + private static List endpoints(List output) { + return withPrefix(output, EndpointProbe.ENDPOINT_PREFIX); + } + + private static List logged(List output) { + return withPrefix(output, EndpointProbe.LOG_PREFIX); + } + + private static List withPrefix( + List output, String prefix) { + List result = new ArrayList<>(); + for (String line : output) { + if (line.startsWith(prefix)) { + result.add(line.substring(prefix.length())); + } + } + return result; + } +} diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/EndpointProbe.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/EndpointProbe.java new file mode 100644 index 000000000..016ee2eeb --- /dev/null +++ b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/EndpointProbe.java @@ -0,0 +1,71 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.did; + +import java.util.logging.Handler; +import java.util.logging.LogRecord; +import java.util.logging.Logger; + +/** + * Prints the endpoint two {@link DidClient} instances resolve from the + * environment of the process, and every record logged while they do. + * {@link DidClientEnvironmentTests} runs this in a Java process of its own, + * because a process cannot change its own environment variables. + */ +public final class EndpointProbe { + + static final String ENDPOINT_PREFIX = "ENDPOINT "; + static final String LOG_PREFIX = "LOG "; + + private EndpointProbe() { + } + + public static void main(String[] args) { + Logger root = Logger.getLogger(""); + for (Handler handler : root.getHandlers()) { + root.removeHandler(handler); + } + root.addHandler(new Handler() { + @Override + public void publish(LogRecord record) { + System.out.println(LOG_PREFIX + record.getLevel().getName() + + " " + record.getMessage()); + } + + @Override + public void flush() { + } + + @Override + public void close() { + } + }); + + for (int i = 0; i < 2; i++) { + System.out.println( + ENDPOINT_PREFIX + new DidClient("resource").getEndpoint()); + } + System.out.flush(); + System.exit(0); + } +} From 98996a06e68785d185b67ff6041d8ddfdd5e3181 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 15:37:59 +0000 Subject: [PATCH 2/5] Keep reading the old cloud endpoint name and bring back its constants The cloud endpoint variable was renamed from FOD_CLOUD_API_URL to _51DEGREES_CLOUD_ENDPOINT, and the old name was no longer read. A patch release keeps every public name, and that covers the name of an environment variable a package reads. The cloud request engine builder, the 51Did client and the cloud engine example now read _51DEGREES_CLOUD_ENDPOINT first and FOD_CLOUD_API_URL when that is not set or is empty. The first use of the old name in a process is logged as a warning that names its replacement. The engine builder logs through its logger factory, the example through its own, and the 51Did client through java.util.logging, because that module has no logging dependency and this adds none. Constants.FOD_CLOUD_API_URL comes back holding "FOD_CLOUD_API_URL", deprecated and pointing at Constants.CLOUD_ENDPOINT_VARIABLE. DidClient.ENDPOINT_VARIABLE holds "FOD_CLOUD_API_URL" again and is deprecated, and the new DidClient.CLOUD_ENDPOINT_VARIABLE holds "_51DEGREES_CLOUD_ENDPOINT". The environment variable name check no longer fails on FOD_CLOUD_API_URL, because the packages read it again. --- ci/check-environment-variable-names.ps1 | 12 ++-- pipeline/README.md | 2 + .../CloudRequestEngineBuilder.java | 39 ++++++++++++- .../flowelements/Constants.java | 8 +++ .../developerexamples/cloudengine/Main.java | 31 +++++++++- .../java/fiftyone/pipeline/did/DidClient.java | 57 +++++++++++++++---- .../fiftyone/pipeline/did/DidClientTests.java | 7 ++- 7 files changed, 134 insertions(+), 22 deletions(-) diff --git a/ci/check-environment-variable-names.ps1 b/ci/check-environment-variable-names.ps1 index 6fa467c2d..328057d35 100644 --- a/ci/check-environment-variable-names.ps1 +++ b/ci/check-environment-variable-names.ps1 @@ -1,9 +1,11 @@ # Every 51Degrees environment variable name in this repository starts with an # underscore, because a POSIX shell cannot set a name that starts with a digit, -# and each variable has one name with nothing read in its place. This reads -# every file git tracks, which leaves out build output and the submodules, and -# fails on any name that starts with the digit and on any old name that is -# still read. +# and each variable has one name with nothing read in its place. The one +# exception is the cloud endpoint, whose deprecated name FOD_CLOUD_API_URL +# the packages still read when _51DEGREES_CLOUD_ENDPOINT is not set, so that +# name is allowed. This reads every file git tracks, which leaves out build +# output and the submodules, and fails on any name that starts with the digit +# and on any other old name that is still read. # # Dot-sourcing the script defines Find-EnvironmentNameProblem without reading # the repository, which is how its tests use it. @@ -50,7 +52,7 @@ $script:DigitInitialPattern = [regex]::new( # variable at all. $script:FallbackPattern = [regex]::new( '(?:["'']|(?i:env)[:.]\\?|\$\{|getenv\(\s*["'']|^[\t\- ]*)' + - '(?:RESOURCE_KEY|LICENSE_KEY|FOD_CLOUD_API_URL)(?![A-Za-z0-9_])|' + + '(?:RESOURCE_KEY|LICENSE_KEY)(?![A-Za-z0-9_])|' + '\bLEGACY_[A-Z_]*ENV_VARS?\b') # The problems on one line, each as its kind, 'digit' or 'fallback', and the diff --git a/pipeline/README.md b/pipeline/README.md index 22e8e8118..66b7d319d 100644 --- a/pipeline/README.md +++ b/pipeline/README.md @@ -139,6 +139,8 @@ and on our [website](https://51degrees.com/documentation/_device_detection__exam Every example that calls the 51Degrees cloud takes its endpoint from the `_51DEGREES_CLOUD_ENDPOINT` environment variable, which is the cloud API base including the `/api/v4/` segment and defaults to `https://cloud.51degrees.com/api/v4/` (the cloud-engine example defaults to the star sign service it was written for, as its row above says). This is the same variable the cloud request engine in this repository honours, so setting it once points every example, and any pipeline built with the engine, at the same place. The resource key comes from `_51DEGREES_RESOURCE_KEY`. +The variable was called `FOD_CLOUD_API_URL` before, and that name is deprecated. The cloud request engine, the 51Did client and the cloud-engine example still read `FOD_CLOUD_API_URL` when `_51DEGREES_CLOUD_ENDPOINT` is not set, and each logs a warning the first time it does. When both are set, `_51DEGREES_CLOUD_ENDPOINT` is used. + A host other than cloud.51degrees.com would be used to (a) use an on premise web server, or (b) use a privately hosted version of the 51Degrees cloud for performance reasons. This is the private hosting option of the cloud service. Both run the same service, so the examples work unchanged. diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilder.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilder.java index 576df6df1..db9f39443 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilder.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilder.java @@ -43,6 +43,7 @@ import java.util.List; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; /** @@ -71,6 +72,11 @@ public class CloudRequestEngineBuilder extends return System.getenv(name); }; + // Set once the use of the deprecated end point variable has been + // logged, so that a process building many engines logs it once. + private static final AtomicBoolean oldEndPointVariableLogged = + new AtomicBoolean(false); + public CloudRequestEngineBuilder(ILoggerFactory loggerFactory) { this(loggerFactory, new HttpClientDefault()); } @@ -91,12 +97,12 @@ protected CloudRequestEngine newEngine(List properties) throws Exception /* * Check if endPoint has been explicitly set via setEndpoint. - * If not check for environment variable _51DEGREES_CLOUD_ENDPOINT. + * If not check for environment variable _51DEGREES_CLOUD_ENDPOINT, + * and then for its deprecated name FOD_CLOUD_API_URL. * If nothing else is set, use default value. */ if (endPoint == null || endPoint.isEmpty()) { - String envVarEndPoint = getEnvVar.apply( - Constants.CLOUD_ENDPOINT_VARIABLE); + String envVarEndPoint = endPointFromEnvironment(); if (envVarEndPoint == null || envVarEndPoint.isEmpty()) { setEndpoint(Constants.END_POINT_DEFAULT); } @@ -156,6 +162,33 @@ public CloudDiscoveryState getDiscoveryState() { return engine == null ? null : engine.getDiscoveryState(); } + /** + * The end point the environment gives, or null or empty when it gives + * none. {@link Constants#CLOUD_ENDPOINT_VARIABLE} is read first. Only + * when that is not set or is empty is the deprecated + * {@link Constants#FOD_CLOUD_API_URL} read, and the first use of it in + * the process is logged as a warning. + */ + @SuppressWarnings("deprecation") + private String endPointFromEnvironment() { + String value = getEnvVar.apply(Constants.CLOUD_ENDPOINT_VARIABLE); + if (value != null && value.isEmpty() == false) { + return value; + } + value = getEnvVar.apply(Constants.FOD_CLOUD_API_URL); + if (value != null && value.isEmpty() == false + && oldEndPointVariableLogged.compareAndSet(false, true)) { + loggerFactory.getLogger(CloudRequestEngineBuilder.class.getName()) + .warn("The environment variable " + + Constants.FOD_CLOUD_API_URL + " is deprecated and " + + Constants.CLOUD_ENDPOINT_VARIABLE + " replaces it. " + + "Set " + Constants.CLOUD_ENDPOINT_VARIABLE + + " to the same value, as a later release may no " + + "longer read " + Constants.FOD_CLOUD_API_URL + "."); + } + return value; + } + private static String trimmed(String value) { return value == null ? "" : value.trim(); } diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/Constants.java index 1a77c3537..84b3e024e 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/Constants.java @@ -28,6 +28,14 @@ public class Constants { "https://cloud.51degrees.com/api/v4"; // Environment variable name to set end point public static final String CLOUD_ENDPOINT_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT"; + /** + * The old name of the environment variable that sets the end point, + * read only when {@link #CLOUD_ENDPOINT_VARIABLE} is not set. + * @deprecated use {@link #CLOUD_ENDPOINT_VARIABLE}, and set the + * environment variable it names in place of this one. + */ + @Deprecated + public static final String FOD_CLOUD_API_URL = "FOD_CLOUD_API_URL"; // Default timeout for each request to the cloud service, as the // specification's Configuration options table gives. public static final int DEFAULT_TIMEOUT_MILLIS = 2000; diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java index 4b039d94c..9f19374f7 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java @@ -34,6 +34,8 @@ import pipeline.developerexamples.cloudengine.flowelements.SimpleCloudEngine; import pipeline.developerexamples.cloudengine.flowelements.SimpleCloudEngineBuilder; +import java.util.concurrent.atomic.AtomicBoolean; + public class Main { private static final ILoggerFactory loggerFactory = LoggerFactory.getILoggerFactory(); private static final HttpClient httpClient = new HttpClientDefault(); @@ -53,6 +55,17 @@ public class Main { private static final String FIFTYONE_CLOUD_ENDPOINT = "https://cloud.51degrees.com/api/v4/"; + /** + * The name the cloud endpoint is read from, and the deprecated name + * read when that one is not set. + */ + private static final String ENDPOINT_VARIABLE = + "_51DEGREES_CLOUD_ENDPOINT"; + private static final String OLD_ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL"; + + private static final AtomicBoolean oldEndpointVariableLogged = + new AtomicBoolean(false); + /** * The cloud endpoint, taken from _51DEGREES_CLOUD_ENDPOINT when that is * set, which is the variable every 51Degrees cloud example honours and @@ -63,10 +76,24 @@ public class Main { * which is the private hosting option of the cloud service. Both run * the same service, so the example works unchanged. Normalised to * end in one slash so the builder appends its three paths to it - * directly. + * directly. The deprecated name FOD_CLOUD_API_URL is read when + * _51DEGREES_CLOUD_ENDPOINT is not set, and the first use of it is + * logged as a warning. */ static String endpoint() { - String value = env("_51DEGREES_CLOUD_ENDPOINT"); + String value = env(ENDPOINT_VARIABLE); + if (value == null) { + value = env(OLD_ENDPOINT_VARIABLE); + if (value != null + && oldEndpointVariableLogged.compareAndSet(false, true)) { + loggerFactory.getLogger(Main.class.getName()).warn( + "The environment variable " + OLD_ENDPOINT_VARIABLE + + " is deprecated and " + ENDPOINT_VARIABLE + + " replaces it. Set " + ENDPOINT_VARIABLE + + " to the same value, as a later release may no " + + "longer read " + OLD_ENDPOINT_VARIABLE + "."); + } + } if (value == null) { // A resource key and an endpoint travel together, because a // key is only known to the service that issued it. So the diff --git a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java b/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java index cc4e28b92..9c2d067d2 100644 --- a/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java +++ b/pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java @@ -59,7 +59,9 @@ import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import java.util.logging.Logger; /** * The client for everything a server does with a 51Did against the @@ -114,7 +116,18 @@ public final class DidClient { * The environment variable read for the API base when the constructor * is given none, the same one the cloud request engine honours. */ - public static final String ENDPOINT_VARIABLE = "_51DEGREES_CLOUD_ENDPOINT"; + public static final String CLOUD_ENDPOINT_VARIABLE = + "_51DEGREES_CLOUD_ENDPOINT"; + + /** + * The old name of the environment variable read for the API base, read + * only when {@link #CLOUD_ENDPOINT_VARIABLE} is not set. + * + * @deprecated use {@link #CLOUD_ENDPOINT_VARIABLE}, and set the + * environment variable it names in place of this one. + */ + @Deprecated + public static final String ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL"; /** * How old the whole held key list may be before a request for a key @@ -136,6 +149,13 @@ public final class DidClient { private static final String USER_AGENT = "pipeline.did/" + version(); + /** + * Set once the use of {@link #ENDPOINT_VARIABLE} has been logged, so + * that a process creating many clients logs it once. + */ + private static final AtomicBoolean OLD_VARIABLE_LOGGED = + new AtomicBoolean(false); + /** * Longest encoded identifier this client will take from a caller. The * figure is arbitrary and deliberately generous, far above anything the @@ -180,7 +200,7 @@ public enum SignatureCheck { /** * A client for the public cloud, or the host named by - * {@value #ENDPOINT_VARIABLE}, with no licence key. + * {@value #CLOUD_ENDPOINT_VARIABLE}, with no licence key. * * @param resourceKey the page's resource key, public by nature */ @@ -190,7 +210,7 @@ public DidClient(String resourceKey) { /** * A client for the public cloud, or the host named by - * {@value #ENDPOINT_VARIABLE}, with a licence key. + * {@value #CLOUD_ENDPOINT_VARIABLE}, with a licence key. * * @param resourceKey the page's resource key, public by nature * @param licenceKey a licence key of the same account, server side @@ -207,9 +227,9 @@ public DidClient(String resourceKey, String licenceKey) { * @param resourceKey the page's resource key, public by nature * @param licenceKey a licence key of the same account, or null * @param endpoint the API base including {@code /api/v4/}, or null - * to read {@value #ENDPOINT_VARIABLE} and fall back - * to {@link #DEFAULT_ENDPOINT}. A trailing slash is - * added where missing. + * to read {@value #CLOUD_ENDPOINT_VARIABLE} and fall + * back to {@link #DEFAULT_ENDPOINT}. A trailing + * slash is added where missing. */ public DidClient(String resourceKey, String licenceKey, String endpoint) { this(builder(resourceKey).licenceKey(licenceKey).endpoint(endpoint)); @@ -268,8 +288,8 @@ public Builder licenceKey(String licenceKey) { /** * @param endpoint the API base including {@code /api/v4/}, or null - * to read {@value #ENDPOINT_VARIABLE} and fall back - * to {@link #DEFAULT_ENDPOINT} + * to read {@value #CLOUD_ENDPOINT_VARIABLE} and + * fall back to {@link #DEFAULT_ENDPOINT} * @return this builder */ public Builder endpoint(String endpoint) { @@ -1152,14 +1172,29 @@ static String encode(String value) { } /** - * The API base to use: the argument, else {@value #ENDPOINT_VARIABLE}, - * else {@link #DEFAULT_ENDPOINT}, ending in exactly one slash so every - * route can be appended directly. + * The API base to use: the argument, else + * {@value #CLOUD_ENDPOINT_VARIABLE}, else the deprecated + * {@value #ENDPOINT_VARIABLE}, else {@link #DEFAULT_ENDPOINT}, ending + * in exactly one slash so every route can be appended directly. The + * first use of the deprecated variable in the process is logged as a + * warning. */ static String resolveEndpoint(String endpoint) { String value = blankToNull(endpoint); + if (value == null) { + value = blankToNull(System.getenv(CLOUD_ENDPOINT_VARIABLE)); + } if (value == null) { value = blankToNull(System.getenv(ENDPOINT_VARIABLE)); + if (value != null + && OLD_VARIABLE_LOGGED.compareAndSet(false, true)) { + Logger.getLogger(DidClient.class.getName()).warning( + "The environment variable " + ENDPOINT_VARIABLE + + " is deprecated and " + CLOUD_ENDPOINT_VARIABLE + + " replaces it. Set " + CLOUD_ENDPOINT_VARIABLE + + " to the same value, as a later release may no " + + "longer read " + ENDPOINT_VARIABLE + "."); + } } if (value == null) { value = DEFAULT_ENDPOINT; diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java index cea3ec788..501a2cabf 100644 --- a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java +++ b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/DidClientTests.java @@ -129,8 +129,13 @@ public void endpoint_IsNormalisedToOneTrailingSlash() { } @Test + @SuppressWarnings("deprecation") public void endpoint_DefaultsToTheCloudOrTheEnvironment() { - String fromEnvironment = System.getenv(DidClient.ENDPOINT_VARIABLE); + String fromEnvironment = + System.getenv(DidClient.CLOUD_ENDPOINT_VARIABLE); + if (fromEnvironment == null || fromEnvironment.trim().isEmpty()) { + fromEnvironment = System.getenv(DidClient.ENDPOINT_VARIABLE); + } String expected = fromEnvironment == null || fromEnvironment.trim().isEmpty() ? DidClient.DEFAULT_ENDPOINT From a5b1ee6fbc65c9af938759c618e6227adadd0480 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 17:26:05 +0000 Subject: [PATCH 3/5] Test that every old environment variable name still works Every environment variable name that a package, a test, an example or a CI script reads is a public name, so a patch release keeps reading the old one when the new one is not set. These tests ask for that wherever main read a name that rc had stopped reading. 51DEGREES_IPI_PATH and 51DEGREES_RESOURCE_KEY in the IP Intelligence test helpers and example helpers, 51DEGREES_DD_PATH in the on-premise web example, and RESOURCE_KEY and LICENSE_KEY in the cloud engine example, the 51Did example and the 51Did live test keys. They also ask for the four helper constants that held an old name to hold it again. The cases of the environment variable name check now expect an old name to be allowed only in the files that read it as a deprecated fallback, and refused everywhere else. The tests fail until the fix that follows. --- ci/check-environment-variable-names.Tests.ps1 | 76 +++++-- .../web/OnPremDataFileOptionTest.java | 20 ++ .../examples/shared/DataFileHelperTest.java | 71 +++++++ .../examples/shared/KeyHelperTest.java | 33 +++ .../shared/testhelpers/FileUtilsTest.java | 67 +++++- .../shared/testhelpers/KeyUtilsTest.java | 64 +++++- .../cloudengine/EndpointTests.java | 48 ++++- .../fodid/DeprecatedKeyNameTests.java | 196 ++++++++++++++++++ .../developerexamples/fodid/KeyProbe.java | 48 +++++ .../pipeline/did/LiveCloudKeysTests.java | 105 ++++++++++ 10 files changed, 709 insertions(+), 19 deletions(-) create mode 100644 pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/DeprecatedKeyNameTests.java create mode 100644 pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/KeyProbe.java create mode 100644 pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeysTests.java diff --git a/ci/check-environment-variable-names.Tests.ps1 b/ci/check-environment-variable-names.Tests.ps1 index 669629a6e..84460b9a8 100644 --- a/ci/check-environment-variable-names.Tests.ps1 +++ b/ci/check-environment-variable-names.Tests.ps1 @@ -32,11 +32,15 @@ Describe 'Find-EnvironmentNameProblem' { @{ Line = 'System.getenv("RESOURCE_KEY")' } @{ Line = 'env("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY");' } @{ Line = 'env("_51DEGREES_LICENSE_KEY", "LICENSE_KEY");' } + @{ Line = 'public static final String FOD = "FOD_CLOUD_API_URL";' } @{ Line = '$env:LICENSE_KEY = $Keys.TestResourceKey' } @{ Line = '${env:LICENSE_KEY} = $Keys.TestResourceKey' } @{ Line = '${Env:RESOURCE_KEY}' } @{ Line = '$ENV:RESOURCE_KEY = "key"' } @{ Line = 'Remove-Item Env:\LICENSE_KEY' } + @{ Line = '${env:FOD_CLOUD_API_URL}' } + @{ Line = ' url: ${{ env.FOD_CLOUD_API_URL }}' } + @{ Line = '${FOD_CLOUD_API_URL}' } @{ Line = 'Constants.LEGACY_ENV_VARS' } @{ Line = ' LICENSE_KEY: ${{ secrets.DEVICE_DETECTION_KEY }}' } @{ Line = '- RESOURCE_KEY names the resource key' } @@ -64,19 +68,6 @@ Describe 'Find-EnvironmentNameProblem' { ) { Get-Kind $Line | Should -BeNullOrEmpty } - - # The cloud endpoint is still read from its deprecated name when the - # current name is not set, so that name is allowed wherever it appears. - It 'leaves the deprecated endpoint name alone in ' -ForEach @( - @{ Line = 'public static final String FOD = "FOD_CLOUD_API_URL";' } - @{ Line = 'System.getenv("FOD_CLOUD_API_URL")' } - @{ Line = '${env:FOD_CLOUD_API_URL}' } - @{ Line = ' url: ${{ env.FOD_CLOUD_API_URL }}' } - @{ Line = '${FOD_CLOUD_API_URL}' } - @{ Line = 'FOD_CLOUD_API_URL=https://host/api/v4/ mvn test' } - ) { - Get-Kind $Line | Should -BeNullOrEmpty - } } Describe 'the check over a repository' { @@ -98,4 +89,63 @@ Describe 'the check over a repository' { Remove-Item -Recurse -Force $repo } } + + It 'passes an old name in a file that reads it as a deprecated fallback' { + $repo = Join-Path ([IO.Path]::GetTempPath()) ([Guid]::NewGuid()) + New-Item -ItemType Directory $repo | Out-Null + try { + git -C $repo init -q + $file = Join-Path $repo 'pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java' + New-Item -ItemType Directory (Split-Path $file) -Force | Out-Null + Set-Content -LiteralPath $file ` + -Value 'String ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL";' + git -C $repo add -A + { & $script:Check -RepoRoot $repo } | Should -Not -Throw + } finally { + Remove-Item -Recurse -Force $repo + } + } + + It 'fails on the same old name in a file that is not named for it' { + $repo = Join-Path ([IO.Path]::GetTempPath()) ([Guid]::NewGuid()) + New-Item -ItemType Directory $repo | Out-Null + try { + git -C $repo init -q + Set-Content -LiteralPath (Join-Path $repo 'Other.java') ` + -Value 'String ENDPOINT_VARIABLE = "FOD_CLOUD_API_URL";' + git -C $repo add -A + { & $script:Check -RepoRoot $repo } | + Should -Throw '*and 1 old names are read*' + } finally { + Remove-Item -Recurse -Force $repo + } + } +} + +Describe 'Test-DeprecatedFallback' { + + # An old name is allowed only in a file that reads it as a deprecated + # fallback or tests that read, and only the names listed for that file. + It 'allows in ' -ForEach @( + @{ Path = 'pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java'; Value = '"FOD_CLOUD_API_URL' } + @{ Path = 'pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java'; Value = '"RESOURCE_KEY' } + @{ Path = 'pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java'; Value = '"LICENSE_KEY' } + @{ Path = 'ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java'; Value = '51DEGREES_IPI_PATH' } + @{ Path = 'examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml'; Value = '51DEGREES_DD_PATH' } + ) { + Test-DeprecatedFallback $Path $Value | Should -BeTrue + } + + It 'refuses in ' -ForEach @( + @{ Path = 'pipeline/pipeline.core/src/main/java/Any.java'; Value = '"FOD_CLOUD_API_URL' } + @{ Path = 'ci/run-unit-tests.ps1'; Value = 'env:RESOURCE_KEY' } + @{ Path = 'pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java'; Value = '"RESOURCE_KEY' } + @{ Path = 'pipeline/pipeline.did/src/main/java/fiftyone/pipeline/did/DidClient.java'; Value = 'LEGACY_ENV_VARS' } + @{ Path = 'pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java'; Value = '"FOD_CLOUD_API_URL' } + @{ Path = 'ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java'; Value = '51DEGREES_DD_PATH' } + @{ Path = 'ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java'; Value = '51degrees_ipi_path' } + @{ Path = 'examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml'; Value = '51DEGREES_IPI_PATH' } + ) { + Test-DeprecatedFallback $Path $Value | Should -BeFalse + } } diff --git a/examples/device-detection/web/getting-started.onprem/src/test/java/fiftyone/devicedetection/examples/web/OnPremDataFileOptionTest.java b/examples/device-detection/web/getting-started.onprem/src/test/java/fiftyone/devicedetection/examples/web/OnPremDataFileOptionTest.java index c77110b95..7f9bc4df7 100644 --- a/examples/device-detection/web/getting-started.onprem/src/test/java/fiftyone/devicedetection/examples/web/OnPremDataFileOptionTest.java +++ b/examples/device-detection/web/getting-started.onprem/src/test/java/fiftyone/devicedetection/examples/web/OnPremDataFileOptionTest.java @@ -41,11 +41,13 @@ */ public class OnPremDataFileOptionTest { private static final String DD_PATH = "_51DEGREES_DD_PATH"; + private static final String OLD_DD_PATH = "51DEGREES_DD_PATH"; private static final String TEST_DATA_FILE = "TestDataFile"; private static final String LITE = "cxx/data/device-detection/51Degrees-LiteV4.1.hash"; private String savedDdPath; + private String savedOldDdPath; private String savedTestDataFile; @Before @@ -53,17 +55,21 @@ public void saveProperties() { // An environment variable of either name would take precedence over // the system properties set here, so the test cannot run. assumeTrue(System.getenv(DD_PATH) == null); + assumeTrue(System.getenv(OLD_DD_PATH) == null); assumeTrue(System.getenv(TEST_DATA_FILE) == null); assumeTrue(System.getenv(TEST_DATA_FILE.toUpperCase()) == null); savedDdPath = System.getProperty(DD_PATH); + savedOldDdPath = System.getProperty(OLD_DD_PATH); savedTestDataFile = System.getProperty(TEST_DATA_FILE); System.clearProperty(DD_PATH); + System.clearProperty(OLD_DD_PATH); System.clearProperty(TEST_DATA_FILE); } @After public void restoreProperties() { restore(DD_PATH, savedDdPath); + restore(OLD_DD_PATH, savedOldDdPath); restore(TEST_DATA_FILE, savedTestDataFile); } @@ -85,6 +91,20 @@ public void prefersDdPathOverTestDataFile() throws Exception { assertEquals("from-dd-path.hash", dataFile()); } + @Test + public void usesOldDdPathWhenNewIsNotSet() throws Exception { + System.setProperty(TEST_DATA_FILE, "from-test-data-file.hash"); + System.setProperty(OLD_DD_PATH, "from-old-dd-path.hash"); + assertEquals("from-old-dd-path.hash", dataFile()); + } + + @Test + public void prefersDdPathOverOldDdPath() throws Exception { + System.setProperty(OLD_DD_PATH, "from-old-dd-path.hash"); + System.setProperty(DD_PATH, "from-dd-path.hash"); + assertEquals("from-dd-path.hash", dataFile()); + } + private static String dataFile() throws Exception { PipelineOptions options = PipelineOptionsFactory.getOptionsFromFile( getFilePath(resourceBase) + "/WEB-INF/51Degrees-OnPrem.xml"); diff --git a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/DataFileHelperTest.java b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/DataFileHelperTest.java index 6d578521f..e1a3418b3 100644 --- a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/DataFileHelperTest.java +++ b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/DataFileHelperTest.java @@ -25,12 +25,14 @@ import fiftyone.pipeline.engines.Constants.PerformanceProfiles; import org.junit.Test; +import java.io.File; import java.io.PrintWriter; import java.io.StringWriter; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; public class DataFileHelperTest { @@ -56,4 +58,73 @@ public void findPerformanceProfile_Unknown_WritesNamesAccepted() { written.toString().contains(profile.name())); } } + + private static final String NEW_PATH_NAME = "_51DEGREES_IPI_PATH"; + private static final String OLD_PATH_NAME = "51DEGREES_IPI_PATH"; + + /** + * System properties stand in for the environment variables, because + * the lookup reads either and a test cannot set an environment + * variable. + */ + @Test + public void getDataFileLocation_OldNameWhenNewIsNotSet() + throws Exception { + assumePathNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_PATH_NAME); + String savedOld = System.clearProperty(OLD_PATH_NAME); + File file = File.createTempFile("old-path-name", ".ipi"); + try { + System.setProperty(OLD_PATH_NAME, file.getPath()); + assertEquals(file.getPath(), + DataFileHelper.getDataFileLocation(null)); + } finally { + restore(NEW_PATH_NAME, savedNew); + restore(OLD_PATH_NAME, savedOld); + file.delete(); + } + } + + @Test + public void getDataFileLocation_NewNameWinsOverOld() throws Exception { + assumePathNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_PATH_NAME); + String savedOld = System.clearProperty(OLD_PATH_NAME); + File newFile = File.createTempFile("new-path-name", ".ipi"); + File oldFile = File.createTempFile("old-path-name", ".ipi"); + try { + System.setProperty(NEW_PATH_NAME, newFile.getPath()); + System.setProperty(OLD_PATH_NAME, oldFile.getPath()); + assertEquals(newFile.getPath(), + DataFileHelper.getDataFileLocation(null)); + } finally { + restore(NEW_PATH_NAME, savedNew); + restore(OLD_PATH_NAME, savedOld); + newFile.delete(); + oldFile.delete(); + } + } + + @Test + @SuppressWarnings("deprecation") + public void oldPathConstantKeepsItsValue() throws Exception { + assertEquals(OLD_PATH_NAME, DataFileHelper.IPI_PATH_ENV_VAR); + assertTrue(DataFileHelper.class.getField("IPI_PATH_ENV_VAR") + .isAnnotationPresent(Deprecated.class)); + assertEquals(NEW_PATH_NAME, + DataFileHelper.class.getField("IPI_PATH_VARIABLE").get(null)); + } + + private static void assumePathNamesNotInEnvironment() { + assumeTrue(System.getenv(NEW_PATH_NAME) == null); + assumeTrue(System.getenv(OLD_PATH_NAME) == null); + } + + private static void restore(String name, String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } } diff --git a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java index 809b7382f..11d30a86f 100644 --- a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java +++ b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java @@ -126,4 +126,37 @@ public void requireResourceKey_ExampleNamesInTheirOrder() { assertEquals(KEY_B, KeyHelper.requireResourceKey(values::get, EXAMPLE_NAMES, "every IP Intelligence property", "ip")); } + + private static final String NEW_KEY_NAME = "_51DEGREES_RESOURCE_KEY"; + private static final String OLD_KEY_NAME = "51DEGREES_RESOURCE_KEY"; + + @Test + public void requireResourceKey_OldNameWhenNewIsNotSet() { + Map values = new HashMap<>(); + values.put(OLD_KEY_NAME, KEY_A); + values.put(KeyHelper.TEST_RESOURCE_KEY, KEY_B); + assertEquals(KEY_A, KeyHelper.requireResourceKey(values::get, + Arrays.asList(NEW_KEY_NAME, KeyHelper.TEST_RESOURCE_KEY), + "every IP Intelligence property", "ip")); + } + + @Test + public void requireResourceKey_NewNameWinsOverOld() { + Map values = new HashMap<>(); + values.put(NEW_KEY_NAME, KEY_B); + values.put(OLD_KEY_NAME, KEY_A); + assertEquals(KEY_B, KeyHelper.requireResourceKey(values::get, + Arrays.asList(NEW_KEY_NAME, KeyHelper.TEST_RESOURCE_KEY), + "every IP Intelligence property", "ip")); + } + + @Test + @SuppressWarnings("deprecation") + public void oldKeyConstantKeepsItsValue() throws Exception { + assertEquals(OLD_KEY_NAME, KeyHelper.RESOURCE_KEY_ENV_VAR); + assertEquals(true, KeyHelper.class.getField("RESOURCE_KEY_ENV_VAR") + .isAnnotationPresent(Deprecated.class)); + assertEquals(NEW_KEY_NAME, + KeyHelper.class.getField("RESOURCE_KEY_VARIABLE").get(null)); + } } diff --git a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/FileUtilsTest.java b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/FileUtilsTest.java index bb1b302b7..b1478996b 100644 --- a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/FileUtilsTest.java +++ b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/FileUtilsTest.java @@ -30,6 +30,7 @@ import java.io.IOException; import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; public class FileUtilsTest { @@ -47,5 +48,69 @@ public void testUserDir() throws IOException { assertEquals(". and user dir should be the same", f.getCanonicalPath(), g.getCanonicalPath()); } + private static final String NEW_PATH_NAME = "_51DEGREES_IPI_PATH"; + private static final String OLD_PATH_NAME = "51DEGREES_IPI_PATH"; -} \ No newline at end of file + /** + * System properties stand in for the environment variables, because + * the lookup reads either and a test cannot set an environment + * variable. + */ + @Test + public void oldPathNameIsReadWhenNewIsNotSet() throws IOException { + assumePathNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_PATH_NAME); + String savedOld = System.clearProperty(OLD_PATH_NAME); + File file = File.createTempFile("old-path-name", ".ipi"); + try { + System.setProperty(OLD_PATH_NAME, file.getPath()); + assertEquals(file, FileUtils.getHashFile()); + } finally { + restore(NEW_PATH_NAME, savedNew); + restore(OLD_PATH_NAME, savedOld); + file.delete(); + } + } + + @Test + public void newPathNameWinsOverOld() throws IOException { + assumePathNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_PATH_NAME); + String savedOld = System.clearProperty(OLD_PATH_NAME); + File newFile = File.createTempFile("new-path-name", ".ipi"); + File oldFile = File.createTempFile("old-path-name", ".ipi"); + try { + System.setProperty(NEW_PATH_NAME, newFile.getPath()); + System.setProperty(OLD_PATH_NAME, oldFile.getPath()); + assertEquals(newFile, FileUtils.getHashFile()); + } finally { + restore(NEW_PATH_NAME, savedNew); + restore(OLD_PATH_NAME, savedOld); + newFile.delete(); + oldFile.delete(); + } + } + + @Test + @SuppressWarnings("deprecation") + public void oldPathConstantKeepsItsValue() throws Exception { + assertEquals(OLD_PATH_NAME, FileUtils.IPI_PATH_ENV_VAR); + assertTrue(FileUtils.class.getField("IPI_PATH_ENV_VAR") + .isAnnotationPresent(Deprecated.class)); + assertEquals(NEW_PATH_NAME, + FileUtils.class.getField("IPI_PATH_VARIABLE").get(null)); + } + + private static void assumePathNamesNotInEnvironment() { + assumeTrue(System.getenv(NEW_PATH_NAME) == null); + assumeTrue(System.getenv(OLD_PATH_NAME) == null); + } + + private static void restore(String name, String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } +} diff --git a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtilsTest.java b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtilsTest.java index f102146b9..03a7d5341 100644 --- a/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtilsTest.java +++ b/ip-intelligence/ip-intelligence.shared/src/test/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtilsTest.java @@ -25,6 +25,7 @@ import org.junit.Test; import static org.junit.Assert.*; +import static org.junit.Assume.assumeTrue; public class KeyUtilsTest { @Test @@ -33,4 +34,65 @@ public void testGetEnv(){ System.out.println(KeyUtils.getNamedKey("path")); } -} \ No newline at end of file + private static final String NEW_KEY_NAME = "_51DEGREES_RESOURCE_KEY"; + private static final String OLD_KEY_NAME = "51DEGREES_RESOURCE_KEY"; + + /** + * System properties stand in for the environment variables, because + * the lookup reads either and a test cannot set an environment + * variable. + */ + @Test + public void oldKeyNameIsReadWhenNewIsNotSet() { + assumeKeyNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_KEY_NAME); + String savedOld = System.clearProperty(OLD_KEY_NAME); + try { + System.setProperty(OLD_KEY_NAME, "old-name-key"); + assertEquals("old-name-key", + KeyUtils.getResourceKey("NoSuchLegacyName")); + } finally { + restore(NEW_KEY_NAME, savedNew); + restore(OLD_KEY_NAME, savedOld); + } + } + + @Test + public void newKeyNameWinsOverOld() { + assumeKeyNamesNotInEnvironment(); + String savedNew = System.clearProperty(NEW_KEY_NAME); + String savedOld = System.clearProperty(OLD_KEY_NAME); + try { + System.setProperty(NEW_KEY_NAME, "new-name-key"); + System.setProperty(OLD_KEY_NAME, "old-name-key"); + assertEquals("new-name-key", + KeyUtils.getResourceKey("NoSuchLegacyName")); + } finally { + restore(NEW_KEY_NAME, savedNew); + restore(OLD_KEY_NAME, savedOld); + } + } + + @Test + @SuppressWarnings("deprecation") + public void oldKeyConstantKeepsItsValue() throws Exception { + assertEquals(OLD_KEY_NAME, KeyUtils.RESOURCE_KEY_ENV_VAR); + assertTrue(KeyUtils.class.getField("RESOURCE_KEY_ENV_VAR") + .isAnnotationPresent(Deprecated.class)); + assertEquals(NEW_KEY_NAME, + KeyUtils.class.getField("RESOURCE_KEY_VARIABLE").get(null)); + } + + private static void assumeKeyNamesNotInEnvironment() { + assumeTrue(System.getenv(NEW_KEY_NAME) == null); + assumeTrue(System.getenv(OLD_KEY_NAME) == null); + } + + private static void restore(String name, String value) { + if (value == null) { + System.clearProperty(name); + } else { + System.setProperty(name, value); + } + } +} diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java index cf9d27bd0..c6969a90e 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java @@ -30,10 +30,11 @@ import static org.junit.jupiter.api.Assumptions.assumeTrue; /** - * Tests which name the example takes its endpoint from. The example reads - * each name as an environment variable and then as a system property, so - * these tests set system properties, and are skipped where the environment - * already sets either name, because the environment would answer first. + * Tests which names the example takes its endpoint and its resource key + * from. The example reads each name as an environment variable and then as + * a system property, so these tests set system properties, and are skipped + * where the environment already sets a name they use, because the + * environment would answer first. */ public class EndpointTests { @@ -76,4 +77,43 @@ public void Endpoint_Neither_Name() { assertEquals(expected, Main.endpoint()); } + + private static final String NEW_KEY_NAME = "_51DEGREES_RESOURCE_KEY"; + private static final String OLD_KEY_NAME = "RESOURCE_KEY"; + + @Test + public void ResourceKey_Old_Name_Alone() { + assumeTrue(System.getenv(NEW_KEY_NAME) == null); + assumeTrue(System.getenv(OLD_KEY_NAME) == null); + String savedNew = System.clearProperty(NEW_KEY_NAME); + try { + System.setProperty(OLD_KEY_NAME, "old-name-key"); + + assertEquals("old-name-key", Main.configuredResourceKey()); + } finally { + System.clearProperty(OLD_KEY_NAME); + if (savedNew != null) { + System.setProperty(NEW_KEY_NAME, savedNew); + } + } + } + + @Test + public void ResourceKey_New_Name_Wins() { + assumeTrue(System.getenv(NEW_KEY_NAME) == null); + assumeTrue(System.getenv(OLD_KEY_NAME) == null); + String savedNew = System.clearProperty(NEW_KEY_NAME); + try { + System.setProperty(NEW_KEY_NAME, "new-name-key"); + System.setProperty(OLD_KEY_NAME, "old-name-key"); + + assertEquals("new-name-key", Main.configuredResourceKey()); + } finally { + System.clearProperty(OLD_KEY_NAME); + System.clearProperty(NEW_KEY_NAME); + if (savedNew != null) { + System.setProperty(NEW_KEY_NAME, savedNew); + } + } + } } diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/DeprecatedKeyNameTests.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/DeprecatedKeyNameTests.java new file mode 100644 index 000000000..9aab1e55c --- /dev/null +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/DeprecatedKeyNameTests.java @@ -0,0 +1,196 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package pipeline.developerexamples.fodid; + +import fiftyone.pipeline.did.DidClient; +import org.json.JSONObject; +import org.junit.jupiter.api.Test; + +import java.io.BufferedReader; +import java.io.File; +import java.io.InputStreamReader; +import java.nio.charset.StandardCharsets; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests which environment variables the example takes its resource key and + * its licence key from. Each test starts {@link KeyProbe} in a Java process + * of its own with the variables it needs, so the example reads a real + * environment. + */ +public class DeprecatedKeyNameTests { + + private static final String RESOURCE_KEY_NAME = "_51DEGREES_RESOURCE_KEY"; + private static final String OLD_RESOURCE_KEY_NAME = "RESOURCE_KEY"; + private static final String LICENCE_KEY_NAME = "_51DEGREES_LICENSE_KEY"; + private static final String OLD_LICENCE_KEY_NAME = "LICENSE_KEY"; + + private static final int PROBE_TIMEOUT_SECONDS = 120; + + @Test + public void oldNamesAlone_GiveTheirKeys() throws Exception { + List output = + probe(null, "old-name-key", null, "old-name-licence"); + + assertEquals("old-name-key", + value(output, KeyProbe.RESOURCE_PREFIX), output.toString()); + assertEquals("old-name-licence", + value(output, KeyProbe.LICENCE_PREFIX), output.toString()); + } + + @Test + public void oldNamesAlone_AreEachReportedAsDeprecatedOnce() + throws Exception { + List output = + probe(null, "old-name-key", null, "old-name-licence"); + + assertEquals(1, deprecations(output, OLD_RESOURCE_KEY_NAME, + RESOURCE_KEY_NAME), output.toString()); + assertEquals(1, deprecations(output, OLD_LICENCE_KEY_NAME, + LICENCE_KEY_NAME), output.toString()); + } + + @Test + public void bothNames_NewNamesWin() throws Exception { + List output = probe( + "new-name-key", "old-name-key", + "new-name-licence", "old-name-licence"); + + assertEquals("new-name-key", + value(output, KeyProbe.RESOURCE_PREFIX), output.toString()); + assertEquals("new-name-licence", + value(output, KeyProbe.LICENCE_PREFIX), output.toString()); + assertEquals(0, deprecations(output, OLD_RESOURCE_KEY_NAME, + RESOURCE_KEY_NAME), output.toString()); + assertEquals(0, deprecations(output, OLD_LICENCE_KEY_NAME, + LICENCE_KEY_NAME), output.toString()); + } + + @Test + public void noNames_GiveNoKeys() throws Exception { + List output = probe(null, null, null, null); + + assertEquals("null", + value(output, KeyProbe.RESOURCE_PREFIX), output.toString()); + assertEquals("null", + value(output, KeyProbe.LICENCE_PREFIX), output.toString()); + } + + /** + * Runs the probe with each variable set to the value given, or absent + * where the value is null, and returns the lines it printed. + */ + private static List probe( + String resourceKey, + String oldResourceKey, + String licenceKey, + String oldLicenceKey) throws Exception { + String java = Paths.get( + System.getProperty("java.home"), "bin", "java").toString(); + ProcessBuilder builder = + new ProcessBuilder(java, KeyProbe.class.getName()); + Map environment = builder.environment(); + environment.put("CLASSPATH", classPath()); + set(environment, RESOURCE_KEY_NAME, resourceKey); + set(environment, OLD_RESOURCE_KEY_NAME, oldResourceKey); + set(environment, LICENCE_KEY_NAME, licenceKey); + set(environment, OLD_LICENCE_KEY_NAME, oldLicenceKey); + builder.redirectErrorStream(true); + + Process process = builder.start(); + List output = new ArrayList<>(); + try (BufferedReader reader = new BufferedReader(new InputStreamReader( + process.getInputStream(), StandardCharsets.UTF_8))) { + String line; + while ((line = reader.readLine()) != null) { + output.add(line); + } + } + assertTrue( + process.waitFor(PROBE_TIMEOUT_SECONDS, TimeUnit.SECONDS), + "The probe did not finish."); + assertEquals(0, process.exitValue(), output.toString()); + return output; + } + + private static void set( + Map environment, String name, String value) { + if (value == null) { + environment.remove(name); + } else { + environment.put(name, value); + } + } + + /** + * The places the probe loads its classes from, being the classes of + * the example, its test classes, the 51Did package and the JSON + * library. + */ + private static String classPath() throws Exception { + return location(CreatorContextDemoServer.class) + File.pathSeparator + + location(KeyProbe.class) + File.pathSeparator + + location(DidClient.class) + File.pathSeparator + + location(JSONObject.class); + } + + private static String location(Class type) throws Exception { + return Paths.get(type.getProtectionDomain().getCodeSource() + .getLocation().toURI()).toString(); + } + + /** The text after the prefix on the one line that starts with it. */ + private static String value(List output, String prefix) { + String found = null; + for (String line : output) { + if (line.startsWith(prefix)) { + assertNull(found, output.toString()); + found = line.substring(prefix.length()); + } + } + return found; + } + + /** + * How many lines say the old name is deprecated and name the new one. + */ + private static int deprecations( + List output, String oldName, String newName) { + int count = 0; + for (String line : output) { + if (line.contains(" " + oldName + " is deprecated") + && line.contains(newName)) { + count++; + } + } + return count; + } +} diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/KeyProbe.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/KeyProbe.java new file mode 100644 index 000000000..074504798 --- /dev/null +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/test/java/pipeline/developerexamples/fodid/KeyProbe.java @@ -0,0 +1,48 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package pipeline.developerexamples.fodid; + +/** + * Prints the resource key and the licence key the example read from the + * environment of the process. {@link DeprecatedKeyNameTests} runs this in + * a Java process of its own, because the example reads both keys once, + * when its class loads, and a process cannot change its own environment + * variables. + */ +public final class KeyProbe { + + static final String RESOURCE_PREFIX = "RESOURCE "; + static final String LICENCE_PREFIX = "LICENCE "; + + private KeyProbe() { + } + + public static void main(String[] args) { + System.out.println( + RESOURCE_PREFIX + CreatorContextDemoServer.RESOURCE); + System.out.println( + LICENCE_PREFIX + CreatorContextDemoServer.LICENCE); + System.out.flush(); + System.exit(0); + } +} diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeysTests.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeysTests.java new file mode 100644 index 000000000..bf0f5a370 --- /dev/null +++ b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeysTests.java @@ -0,0 +1,105 @@ +/* ********************************************************************* + * This Original Work is copyright of 51 Degrees Mobile Experts Limited. + * Copyright 2026 51 Degrees Mobile Experts Limited, Davidson House, + * Forbury Square, Reading, Berkshire, United Kingdom RG1 3EU. + * + * This Original Work is licensed under the European Union Public Licence + * (EUPL) v.1.2 and is subject to its terms as set out below. + * + * If a copy of the EUPL was not distributed with this file, You can obtain + * one at https://opensource.org/licenses/EUPL-1.2. + * + * The 'Compatible Licences' set out in the Appendix to the EUPL (as may be + * amended by the European Commission) shall be deemed incompatible for + * the purposes of the Work and the provisions of the compatibility + * clause in Article 5 of the EUPL shall not apply. + * + * If using the Work as, or as part of, a network application, by + * including the attribution notice(s) required under Article 5 of the EUPL + * in the end user terms of the application under an appropriate heading, + * such notice(s) shall fulfill the requirements of that article. + * ********************************************************************* */ + +package fiftyone.pipeline.did; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assume.assumeTrue; + +/** + * Tests which names {@link LiveCloudKeys} reads a key from. System + * properties stand in for the environment variables, because the lookup + * reads either and a test cannot set an environment variable. + */ +public class LiveCloudKeysTests { + + private static final String RESOURCE_KEY_NAME = "_51DEGREES_RESOURCE_KEY"; + private static final String OLD_RESOURCE_KEY_NAME = "RESOURCE_KEY"; + private static final String LICENCE_KEY_NAME = "_51DEGREES_LICENSE_KEY"; + private static final String OLD_LICENCE_KEY_NAME = "LICENSE_KEY"; + private static final String OTHER_NAME = "TestResourceKey"; + + private static final String[] NAMES = { + RESOURCE_KEY_NAME, OLD_RESOURCE_KEY_NAME, + LICENCE_KEY_NAME, OLD_LICENCE_KEY_NAME, OTHER_NAME, + }; + + private final String[] saved = new String[NAMES.length]; + + @Before + public void clearNames() { + for (int i = 0; i < NAMES.length; i++) { + // An environment variable would answer before the system + // properties set here, so the test cannot run. + assumeTrue(System.getenv(NAMES[i]) == null); + saved[i] = System.clearProperty(NAMES[i]); + } + } + + @After + public void restoreNames() { + for (int i = 0; i < NAMES.length; i++) { + if (saved[i] == null) { + System.clearProperty(NAMES[i]); + } else { + System.setProperty(NAMES[i], saved[i]); + } + } + } + + @Test + public void resourceKey_OldNameWhenNewIsNotSet() { + System.setProperty(OLD_RESOURCE_KEY_NAME, "old-name-key"); + + assertEquals("old-name-key", + LiveCloudKeys.find(new String[] {RESOURCE_KEY_NAME})); + } + + @Test + public void licenceKey_OldNameWhenNewIsNotSet() { + System.setProperty(OLD_LICENCE_KEY_NAME, "old-name-licence"); + + assertEquals("old-name-licence", + LiveCloudKeys.find(new String[] {LICENCE_KEY_NAME})); + } + + @Test + public void newNameWinsOverOld() { + System.setProperty(RESOURCE_KEY_NAME, "new-name-key"); + System.setProperty(OLD_RESOURCE_KEY_NAME, "old-name-key"); + + assertEquals("new-name-key", + LiveCloudKeys.find(new String[] {RESOURCE_KEY_NAME})); + } + + @Test + public void oldNameIsReadOnlyForItsNewName() { + System.setProperty(OLD_RESOURCE_KEY_NAME, "old-name-key"); + + assertNull(LiveCloudKeys.find(new String[] {OTHER_NAME})); + } +} From 3b2bef1d884bbe160d5e21d88aaa2c8f51d31a78 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 17:27:03 +0000 Subject: [PATCH 4/5] Keep reading every old environment variable name as a deprecated fallback Every environment variable name that a package, a test, an example or a CI script reads is a public name. Each reader that main had and rc changed now reads the new name first and the old name when the new one is not set or is empty, and says once that the old name is deprecated and which name replaces it. 51DEGREES_IPI_PATH is read again by the IP Intelligence test helper FileUtils, the example helper DataFileHelper and the country translation integration test. 51DEGREES_RESOURCE_KEY is read again by the test helper KeyUtils and the example helper KeyHelper. 51DEGREES_DD_PATH is read again by the on-premise web example's configuration. RESOURCE_KEY is read again by the cloud engine example, the 51Did example and the 51Did live test keys, and LICENSE_KEY by the last two. FileUtils.IPI_PATH_ENV_VAR, KeyUtils.RESOURCE_KEY_ENV_VAR, DataFileHelper.IPI_PATH_ENV_VAR and KeyHelper.RESOURCE_KEY_ENV_VAR hold their old values again and are deprecated. IPI_PATH_VARIABLE and RESOURCE_KEY_VARIABLE beside them hold the new names, and the code that named the old constants now names those. The environment variable name check has FOD_CLOUD_API_URL back in its pattern, and allows each old name only in the files that read it as a deprecated fallback or test that read. --- ci/check-environment-variable-names.ps1 | 95 +++++++++++++++++-- .../examples/web/GettingStartedWebOnPrem.java | 28 ++++++ .../main/webapp/WEB-INF/51Degrees-OnPrem.xml | 7 +- .../examples/console/GettingStartedMixed.java | 2 +- .../console/GettingStartedOnPrem.java | 2 +- .../examples/console/MetadataOnPrem.java | 2 +- .../examples/console/OfflineProcessing.java | 2 +- .../console/ExamplesWithoutSettingsTest.java | 6 +- .../examples/shared/DataFileHelper.java | 56 +++++++++-- .../examples/shared/KeyHelper.java | 60 ++++++++++-- .../examples/shared/KeyHelperTest.java | 6 +- .../examples/web/GettingStartedWebMixed.java | 4 +- .../examples/web/GettingStartedWebOnPrem.java | 4 +- .../shared/testhelpers/FileUtils.java | 48 +++++++++- .../shared/testhelpers/KeyUtils.java | 40 +++++++- .../CountryTranslationIntegrationTest.java | 33 ++++++- .../OnPremisePerformanceProfileTests.java | 2 +- .../developerexamples/cloudengine/Main.java | 26 ++++- .../fodid/CreatorContextDemoServer.java | 31 +++++- .../fiftyone/pipeline/did/LiveCloudKeys.java | 63 ++++++++++-- 20 files changed, 452 insertions(+), 65 deletions(-) diff --git a/ci/check-environment-variable-names.ps1 b/ci/check-environment-variable-names.ps1 index 328057d35..11d75e85c 100644 --- a/ci/check-environment-variable-names.ps1 +++ b/ci/check-environment-variable-names.ps1 @@ -1,14 +1,15 @@ # Every 51Degrees environment variable name in this repository starts with an # underscore, because a POSIX shell cannot set a name that starts with a digit, -# and each variable has one name with nothing read in its place. The one -# exception is the cloud endpoint, whose deprecated name FOD_CLOUD_API_URL -# the packages still read when _51DEGREES_CLOUD_ENDPOINT is not set, so that -# name is allowed. This reads every file git tracks, which leaves out build -# output and the submodules, and fails on any name that starts with the digit -# and on any other old name that is still read. +# and each variable has one name. An old name is still read, as a deprecated +# fallback for when its new name is not set, by the files listed in +# DeprecatedFallbacks below and by nothing else. This reads every file git +# tracks, which leaves out build output and the submodules, and fails on any +# name that starts with the digit and on any old name, outside the files +# listed for that name. # -# Dot-sourcing the script defines Find-EnvironmentNameProblem without reading -# the repository, which is how its tests use it. +# Dot-sourcing the script defines Find-EnvironmentNameProblem and +# Test-DeprecatedFallback without reading the repository, which is how its +# tests use them. [CmdletBinding()] param( @@ -52,9 +53,82 @@ $script:DigitInitialPattern = [regex]::new( # variable at all. $script:FallbackPattern = [regex]::new( '(?:["'']|(?i:env)[:.]\\?|\$\{|getenv\(\s*["'']|^[\t\- ]*)' + - '(?:RESOURCE_KEY|LICENSE_KEY)(?![A-Za-z0-9_])|' + + '(?:RESOURCE_KEY|LICENSE_KEY|FOD_CLOUD_API_URL)(?![A-Za-z0-9_])|' + '\bLEGACY_[A-Z_]*ENV_VARS?\b') +# The files that read an old name as a deprecated fallback, or that test +# that read, each with the old names it may hold. The names are put together +# from parts so that this file does not match itself. +$oldEndpoint = 'FOD_CLOUD' + '_API_URL' +$oldResourceKey = 'RESOURCE' + '_KEY' +$oldLicenseKey = 'LICENSE' + '_KEY' +$oldDigitKey = $digitInitial + $oldResourceKey +$oldIpiPath = $digitInitial + 'IPI_PATH' +$oldDdPath = $digitInitial + 'DD_PATH' +$cloudEngine = 'pipeline/pipeline.cloudrequestengine/src' +$did = 'pipeline/pipeline.did/src' +$examples = 'pipeline/pipeline.developer-examples/pipeline.developer-examples' +$ipiShared = 'ip-intelligence/ip-intelligence.shared/src' +$ipiHelpers = 'fiftyone/ipintelligence/shared/testhelpers' +$ipiExamples = 'examples/ip-intelligence/shared/src' +$ipiExampleHelpers = 'fiftyone/ipintelligence/examples/shared' +$onPrem = 'examples/device-detection/web/getting-started.onprem/src' +$onPremPackage = 'fiftyone/devicedetection/examples/web' +$script:DeprecatedFallbacks = @{ + "$cloudEngine/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/Constants.java" = + @($oldEndpoint) + "$cloudEngine/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineBuilderTests.java" = + @($oldEndpoint) + "$did/main/java/fiftyone/pipeline/did/DidClient.java" = + @($oldEndpoint) + "$did/test/java/fiftyone/pipeline/did/DidClientEnvironmentTests.java" = + @($oldEndpoint) + "$did/test/java/fiftyone/pipeline/did/LiveCloudKeys.java" = + @($oldResourceKey, $oldLicenseKey) + "$did/test/java/fiftyone/pipeline/did/LiveCloudKeysTests.java" = + @($oldResourceKey, $oldLicenseKey) + "$examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java" = + @($oldEndpoint, $oldResourceKey) + "$examples.cloud-engine/src/test/java/pipeline/developerexamples/cloudengine/EndpointTests.java" = + @($oldEndpoint, $oldResourceKey) + "$examples.fodid/src/main/java/pipeline/developerexamples/fodid/CreatorContextDemoServer.java" = + @($oldResourceKey, $oldLicenseKey) + "$examples.fodid/src/test/java/pipeline/developerexamples/fodid/DeprecatedKeyNameTests.java" = + @($oldResourceKey, $oldLicenseKey) + "$ipiShared/main/java/$ipiHelpers/FileUtils.java" = @($oldIpiPath) + "$ipiShared/test/java/$ipiHelpers/FileUtilsTest.java" = @($oldIpiPath) + "$ipiShared/main/java/$ipiHelpers/KeyUtils.java" = @($oldDigitKey) + "$ipiShared/test/java/$ipiHelpers/KeyUtilsTest.java" = @($oldDigitKey) + "$ipiExamples/main/java/$ipiExampleHelpers/DataFileHelper.java" = + @($oldIpiPath) + "$ipiExamples/test/java/$ipiExampleHelpers/DataFileHelperTest.java" = + @($oldIpiPath) + "$ipiExamples/main/java/$ipiExampleHelpers/KeyHelper.java" = + @($oldDigitKey) + "$ipiExamples/test/java/$ipiExampleHelpers/KeyHelperTest.java" = + @($oldDigitKey) + 'ip-intelligence/ip-intelligence.translation/src/test/java/fiftyone/ipintelligence/translation/CountryTranslationIntegrationTest.java' = + @($oldIpiPath) + "$onPrem/main/webapp/WEB-INF/51Degrees-OnPrem.xml" = @($oldDdPath) + "$onPrem/main/java/$onPremPackage/GettingStartedWebOnPrem.java" = + @($oldDdPath) + "$onPrem/test/java/$onPremPackage/OnPremDataFileOptionTest.java" = + @($oldDdPath) +} + +# Whether the text matched in the file at this path, relative to the root +# with forward slashes, is an old name that file is listed for. The name is +# the run of name characters the match ends with, compared in its exact +# case. +function Test-DeprecatedFallback([string]$Path, [string]$Value) { + $allowed = $script:DeprecatedFallbacks[$Path] + if ($null -eq $allowed) { + return $false + } + $name = [regex]::Match($Value, '[A-Za-z0-9_]+$').Value + return $allowed -ccontains $name +} + # The problems on one line, each as its kind, 'digit' or 'fallback', and the # text matched. function Find-EnvironmentNameProblem([string]$Line) { @@ -101,6 +175,9 @@ foreach ($relative in $tracked) { foreach ($line in [IO.File]::ReadLines($path)) { $lineNumber++ foreach ($problem in Find-EnvironmentNameProblem $line) { + if (Test-DeprecatedFallback $relative $problem.Value) { + continue + } $text = "${relative}:${lineNumber}: $($problem.Value)" if ($problem.Kind -eq 'digit') { $found += $text diff --git a/examples/device-detection/web/getting-started.onprem/src/main/java/fiftyone/devicedetection/examples/web/GettingStartedWebOnPrem.java b/examples/device-detection/web/getting-started.onprem/src/main/java/fiftyone/devicedetection/examples/web/GettingStartedWebOnPrem.java index 1ed151902..e74dec2b1 100644 --- a/examples/device-detection/web/getting-started.onprem/src/main/java/fiftyone/devicedetection/examples/web/GettingStartedWebOnPrem.java +++ b/examples/device-detection/web/getting-started.onprem/src/main/java/fiftyone/devicedetection/examples/web/GettingStartedWebOnPrem.java @@ -62,9 +62,37 @@ public class GettingStartedWebOnPrem extends HttpServlet { public static String resourceBase = "examples/device-detection/web/getting-started.onprem/src/main/webapp"; public static Logger logger = LoggerFactory.getLogger(GettingStartedWebOnPrem.class); + private static final String DD_PATH_VARIABLE = "_51DEGREES_DD_PATH"; + private static final String OLD_DD_PATH_VARIABLE = "51DEGREES_DD_PATH"; + + /** + * Logs a warning when the configuration takes the data file from the + * deprecated name, which it does when that name is set and the name + * that replaces it is not. + */ + private static void warnOfOldDataFileName() { + if (isSet(OLD_DD_PATH_VARIABLE) && isSet(DD_PATH_VARIABLE) == false) { + logger.warn( + "The environment variable " + OLD_DD_PATH_VARIABLE + + " is deprecated and " + DD_PATH_VARIABLE + + " replaces it. Set " + DD_PATH_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + OLD_DD_PATH_VARIABLE + "."); + } + } + + private static boolean isSet(String name) { + String value = System.getenv(name); + if (value == null || value.isEmpty()) { + value = System.getProperty(name); + } + return value != null && value.isEmpty() == false; + } + public static void main(String[] args) throws Exception { configureLogback(getFilePath("logback.xml")); logger.info("Running Example {}", GettingStartedWebOnPrem.class); + warnOfOldDataFileName(); // If the data file the configuration names is not at that path, it // is searched for, and the free Lite file is downloaded if missing. diff --git a/examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml b/examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml index 5ec7feb45..f6bc4c25d 100644 --- a/examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml +++ b/examples/device-detection/web/getting-started.onprem/src/main/webapp/WEB-INF/51Degrees-OnPrem.xml @@ -29,10 +29,11 @@ false - ${_51DEGREES_DD_PATH:-${TestDataFile:-cxx/data/device-detection/51Degrees-LiteV4.1.hash}} + ${_51DEGREES_DD_PATH:-${51DEGREES_DD_PATH:-${TestDataFile:-cxx/data/device-detection/51Degrees-LiteV4.1.hash}}} LowMemory DeviceDetectionHashEngine diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedMixed.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedMixed.java index 7c0940afe..f007e2fb3 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedMixed.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedMixed.java @@ -138,7 +138,7 @@ public static void run(String ddDataFile, logger.error("Failed to find IP Intelligence data file at '{}'. " + "Please provide a valid path to an IP Intelligence data file (.ipi). " + "An explicit path can be supplied via the {} environment variable.", - ipiDataFile, DataFileHelper.IPI_PATH_ENV_VAR); + ipiDataFile, DataFileHelper.IPI_PATH_VARIABLE); throw e; } diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedOnPrem.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedOnPrem.java index 268d00aa3..e25f3e109 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedOnPrem.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/GettingStartedOnPrem.java @@ -129,7 +129,7 @@ public static void run(String dataFile, "An explicit path can be supplied via the {} environment variable. " + "For testing, you can obtain an enterprise data file by contacting us at " + "https://51degrees.com/contact-us?utm_source=code&utm_medium=example&utm_campaign=51degrees-java&utm_content=console-src-main-java-fiftyone-ipintelligence-examples-console-gettingstartedonprem.java&utm_term=data-file-not-found", - dataFile, DataFileHelper.IPI_PATH_ENV_VAR); + dataFile, DataFileHelper.IPI_PATH_VARIABLE); throw e; } diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java index be168e638..223121f1d 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/MetadataOnPrem.java @@ -91,7 +91,7 @@ public static void run(String dataFile, OutputStream output) throws Exception { logger.error("Failed to find IP Intelligence data file at '{}'. " + "Please provide a valid path to an IP Intelligence data file (.ipi). " + "An explicit path can be supplied via the {} environment variable.", - dataFile, DataFileHelper.IPI_PATH_ENV_VAR); + dataFile, DataFileHelper.IPI_PATH_VARIABLE); throw e; } // Build an on-premise IP Intelligence engine with the low memory diff --git a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/OfflineProcessing.java b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/OfflineProcessing.java index 8bb5ea91b..0357b64d8 100644 --- a/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/OfflineProcessing.java +++ b/examples/ip-intelligence/console/src/main/java/fiftyone/ipintelligence/examples/console/OfflineProcessing.java @@ -143,7 +143,7 @@ public static void run(String dataFile, InputStream is, OutputStream os) throws logger.error("Failed to find IP Intelligence data file at '{}'. " + "Please provide a valid path to an IP Intelligence data file (.ipi). " + "An explicit path can be supplied via the {} environment variable.", - dataFile, DataFileHelper.IPI_PATH_ENV_VAR); + dataFile, DataFileHelper.IPI_PATH_VARIABLE); throw e; } diff --git a/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java b/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java index 45a906637..61572fc88 100644 --- a/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java +++ b/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java @@ -61,10 +61,10 @@ private interface Main { */ private static String runWithoutSettings(Main main) throws Exception { assumeTrue("Skipping, a data file or licence key is set in the environment", - System.getenv(DataFileHelper.IPI_PATH_ENV_VAR) == null && + System.getenv(DataFileHelper.IPI_PATH_VARIABLE) == null && System.getenv(LICENSE_KEY) == null && System.getenv(LICENSE_KEY.toUpperCase()) == null); - String dataFile = System.clearProperty(DataFileHelper.IPI_PATH_ENV_VAR); + String dataFile = System.clearProperty(DataFileHelper.IPI_PATH_VARIABLE); String licenseKey = System.clearProperty(LICENSE_KEY); PrintStream out = System.out; ByteArrayOutputStream buffer = new ByteArrayOutputStream(); @@ -74,7 +74,7 @@ private static String runWithoutSettings(Main main) throws Exception { } finally { System.setOut(out); if (dataFile != null) { - System.setProperty(DataFileHelper.IPI_PATH_ENV_VAR, dataFile); + System.setProperty(DataFileHelper.IPI_PATH_VARIABLE, dataFile); } if (licenseKey != null) { System.setProperty(LICENSE_KEY, licenseKey); diff --git a/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/DataFileHelper.java b/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/DataFileHelper.java index 66e118cd2..e617da672 100644 --- a/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/DataFileHelper.java +++ b/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/DataFileHelper.java @@ -47,6 +47,7 @@ import java.util.List; import java.util.Locale; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; import static fiftyone.pipeline.util.FileFinder.getFilePath; @@ -133,7 +134,33 @@ public static String findAvailableDataFile(String... candidates) { * hold an explicit path to the IP Intelligence data file. This name is * checked first, before any search of the project space. */ - public static final String IPI_PATH_ENV_VAR = "_51DEGREES_IPI_PATH"; + public static final String IPI_PATH_VARIABLE = "_51DEGREES_IPI_PATH"; + + /** + * The old name of {@link #IPI_PATH_VARIABLE}, read only when that one + * is not set. + * @deprecated use {@link #IPI_PATH_VARIABLE}, and set the variable it + * names in place of this one. + */ + @Deprecated + public static final String IPI_PATH_ENV_VAR = "51DEGREES_IPI_PATH"; + + // Set once the use of the deprecated name has been logged. + private static final AtomicBoolean OLD_NAME_LOGGED = + new AtomicBoolean(false); + + /** + * The value of the environment variable of this name, or of the system + * property where that is not set or is empty, or null when neither + * holds a value. + */ + private static String environmentOrProperty(String name) { + String value = System.getenv(name); + if (Objects.isNull(value) || value.isEmpty()) { + value = System.getProperty(name); + } + return Objects.isNull(value) || value.isEmpty() ? null : value; + } public static class DatafileInfo { FiftyOneDataFile fileInfo; @@ -362,7 +389,7 @@ public static void cantFindDataFile(String dataFile) { "somewhere in the project space to be found. An explicit " + "path to the data file can be supplied via the {} " + "environment variable or system property.", - dataFile, IPI_PATH_ENV_VAR); + dataFile, IPI_PATH_VARIABLE); } /** @@ -391,9 +418,11 @@ public static File getEvidenceFile(String evidenceFilename) throws Exception { /** * Tries to find the passed file, or if null a default file. Handles both absolute and relative paths. * An absolute path supplied by the caller is used as is. Otherwise an - * explicit path supplied via the {@link #IPI_PATH_ENV_VAR} environment + * explicit path supplied via the {@link #IPI_PATH_VARIABLE} environment * variable or system property is checked before the folder hierarchy is - * searched for the passed filename. + * searched for the passed filename. When neither is set the deprecated + * {@link #IPI_PATH_ENV_VAR} is read, and its first use is logged as a + * warning. * @param dataFilename a filename to find (can be absolute or relative path) * @return a full pathname * @throws Exception if the file was not found @@ -404,16 +433,27 @@ public static String getDataFileLocation(String dataFilename) throws Exception { // system property is checked before any search for the filename if (Objects.isNull(dataFilename) || Paths.get(dataFilename).isAbsolute() == false) { - String envDataFile = System.getenv(IPI_PATH_ENV_VAR); + String envName = IPI_PATH_VARIABLE; + String envDataFile = environmentOrProperty(envName); if (Objects.isNull(envDataFile)) { - envDataFile = System.getProperty(IPI_PATH_ENV_VAR); + envName = IPI_PATH_ENV_VAR; + envDataFile = environmentOrProperty(envName); + if (Objects.nonNull(envDataFile) + && OLD_NAME_LOGGED.compareAndSet(false, true)) { + logger.warn( + "The environment variable " + IPI_PATH_ENV_VAR + + " is deprecated and " + IPI_PATH_VARIABLE + + " replaces it. Set " + IPI_PATH_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + IPI_PATH_ENV_VAR + "."); + } } if (Objects.nonNull(envDataFile)) { if (Files.exists(Paths.get(envDataFile))) { return envDataFile; } logger.warn("Ignoring {} value '{}' as no file exists there", - IPI_PATH_ENV_VAR, envDataFile); + envName, envDataFile); } } @@ -451,7 +491,7 @@ public static String getDataFileLocation(String dataFilename) throws Exception { DataFileHelper.cantFindDataFile(dataFilename); throw new IOException("Data file not found at relative path: " + dataFilename + ". Searched in project directories and current working directory." + - " An explicit path can be supplied via the " + IPI_PATH_ENV_VAR + + " An explicit path can be supplied via the " + IPI_PATH_VARIABLE + " environment variable.", notFound); } } diff --git a/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/KeyHelper.java b/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/KeyHelper.java index acb4fcf7c..2342f4047 100644 --- a/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/KeyHelper.java +++ b/examples/ip-intelligence/shared/src/main/java/fiftyone/ipintelligence/examples/shared/KeyHelper.java @@ -27,6 +27,7 @@ import org.slf4j.LoggerFactory; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; public class KeyHelper { @@ -35,7 +36,20 @@ public class KeyHelper { * hold the resource key. This name is checked first, before the legacy * {@link #TEST_RESOURCE_KEY} name. */ - public static final String RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY"; + public static final String RESOURCE_KEY_VARIABLE = "_51DEGREES_RESOURCE_KEY"; + + /** + * The old name of {@link #RESOURCE_KEY_VARIABLE}, read only when that + * one is not set. + * @deprecated use {@link #RESOURCE_KEY_VARIABLE}, and set the variable + * it names in place of this one. + */ + @Deprecated + public static final String RESOURCE_KEY_ENV_VAR = "51DEGREES_RESOURCE_KEY"; + + // Set once the use of the deprecated name has been logged. + private static final AtomicBoolean OLD_NAME_LOGGED = + new AtomicBoolean(false); public static final String TEST_RESOURCE_KEY = "TestResourceKey"; static Logger logger = LoggerFactory.getLogger(KeyHelper.class); @@ -64,7 +78,7 @@ public class KeyHelper { public static String requireTestResourceKey(String asksFor, String... products) { return requireResourceKey(KeyHelper::readKey, Arrays.asList( - RESOURCE_KEY_ENV_VAR, TEST_RESOURCE_KEY), asksFor, products); + RESOURCE_KEY_VARIABLE, TEST_RESOURCE_KEY), asksFor, products); } static String requireResourceKey(Function read, @@ -75,6 +89,10 @@ static String requireResourceKey(Function read, names.addAll(exampleNames); for (String name : names) { String value = read.apply(name); + if (KeyUtils.isInvalidKey(value) + && RESOURCE_KEY_VARIABLE.equals(name)) { + value = oldNameResourceKey(read); + } if (KeyUtils.isInvalidKey(value) == false) { return value.trim(); } @@ -82,6 +100,28 @@ static String requireResourceKey(Function read, throw new AssertionError(noResourceKey(names, asksFor, products)); } + /** + * The key held under the deprecated {@link #RESOURCE_KEY_ENV_VAR}, or + * null when there is none. It is read only when + * {@link #RESOURCE_KEY_VARIABLE} holds no key, and its first use is + * logged as a warning. + */ + private static String oldNameResourceKey(Function read) { + String value = read.apply(RESOURCE_KEY_ENV_VAR); + if (KeyUtils.isInvalidKey(value)) { + return null; + } + if (OLD_NAME_LOGGED.compareAndSet(false, true)) { + logger.warn( + "The environment variable " + RESOURCE_KEY_ENV_VAR + + " is deprecated and " + RESOURCE_KEY_VARIABLE + + " replaces it. Set " + RESOURCE_KEY_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + RESOURCE_KEY_ENV_VAR + "."); + } + return value; + } + static String noResourceKey(List names, String asksFor, String... products) { return "This test calls the live 51Degrees cloud service, so it " + @@ -149,16 +189,22 @@ public static String getOrSetTestResourceKey(String value) { /** * Obtain a resource key from the passed argument, * from environment variable or from a property. The aligned - * {@link #RESOURCE_KEY_ENV_VAR} name is checked first, then the - * passed variable name. Store as System Property under the - * passed variable name. + * {@link #RESOURCE_KEY_VARIABLE} name is checked first, then the + * deprecated {@link #RESOURCE_KEY_ENV_VAR}, then the passed variable + * name. Store as System Property under the passed variable name. */ public static String getOrSetResourceKey(String value, String variableName, String errorMessage, boolean shouldThrow) { if (Objects.isNull(value)) { // check the aligned name first, then the legacy name - value = KeyUtils.getNamedKey(RESOURCE_KEY_ENV_VAR); + value = KeyUtils.getNamedKey(RESOURCE_KEY_VARIABLE); + if (Objects.isNull(value) || value.isEmpty()) { + String oldNameKey = oldNameResourceKey(KeyUtils::getNamedKey); + if (Objects.nonNull(oldNameKey)) { + value = oldNameKey; + } + } if (Objects.isNull(value)) { value = KeyUtils.getNamedKey(variableName); } @@ -170,7 +216,7 @@ public static String getOrSetResourceKey(String value, String variableName, "runnable example,\n - as an Environment Variable named \"\u001B[36m{}\u001B[0m\"," + "\n - as a System Property named \"\u001B[36m{}\u001B[0m\" " + "(the legacy name \"\u001B[36m{}\u001B[0m\" is also still accepted).", - RESOURCE_KEY_ENV_VAR, RESOURCE_KEY_ENV_VAR, variableName); + RESOURCE_KEY_VARIABLE, RESOURCE_KEY_VARIABLE, variableName); logger.error(errorMessage); if (shouldThrow) { throw new IllegalStateException("\"" + value + "\" is not a valid resource key"); diff --git a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java index 11d30a86f..756cb822b 100644 --- a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java +++ b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java @@ -45,7 +45,7 @@ public class KeyHelperTest { * The names getOrSetTestResourceKey reads, in its order. */ private static final List EXAMPLE_NAMES = Arrays.asList( - KeyHelper.RESOURCE_KEY_ENV_VAR, KeyHelper.TEST_RESOURCE_KEY); + KeyHelper.RESOURCE_KEY_VARIABLE, KeyHelper.TEST_RESOURCE_KEY); /** * With no key anywhere, the lookup logs that there is none and stores @@ -118,11 +118,11 @@ public void requireResourceKey_FirstValidNameWins() { @Test public void requireResourceKey_ExampleNamesInTheirOrder() { Map values = new HashMap<>(); - values.put(KeyHelper.RESOURCE_KEY_ENV_VAR, KEY_A); + values.put(KeyHelper.RESOURCE_KEY_VARIABLE, KEY_A); values.put(KeyHelper.TEST_RESOURCE_KEY, KEY_B); assertEquals(KEY_A, KeyHelper.requireResourceKey(values::get, EXAMPLE_NAMES, "every IP Intelligence property", "ip")); - values.remove(KeyHelper.RESOURCE_KEY_ENV_VAR); + values.remove(KeyHelper.RESOURCE_KEY_VARIABLE); assertEquals(KEY_B, KeyHelper.requireResourceKey(values::get, EXAMPLE_NAMES, "every IP Intelligence property", "ip")); } diff --git a/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java b/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java index 1accfb336..18f5bc05b 100644 --- a/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java +++ b/examples/ip-intelligence/web/getting-started.mixed/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebMixed.java @@ -74,7 +74,7 @@ import static fiftyone.common.testhelpers.LogbackHelper.configureLogback; import static fiftyone.ipintelligence.examples.shared.DataFileHelper.LITE_DATA_FILE_REL_PATH; -import static fiftyone.ipintelligence.examples.shared.DataFileHelper.IPI_PATH_ENV_VAR; +import static fiftyone.ipintelligence.examples.shared.DataFileHelper.IPI_PATH_VARIABLE; import static fiftyone.ipintelligence.examples.shared.PropertyHelper.*; import static fiftyone.pipeline.util.FileFinder.getFilePath; @@ -120,7 +120,7 @@ public static void main(String[] args) throws Exception { } catch (Exception e) { logger.warn("IP Intelligence data file not found at: {}. " + "An explicit path can be supplied via the {} environment variable.", - ipiDataFile, IPI_PATH_ENV_VAR); + ipiDataFile, IPI_PATH_VARIABLE); logger.warn("Will attempt to use default path from XML configuration"); logger.debug("Error finding IP Intelligence data file", e); } diff --git a/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java b/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java index 0520012b8..d87b20d5d 100644 --- a/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java +++ b/examples/ip-intelligence/web/getting-started.onprem/src/main/java/fiftyone/ipintelligence/examples/web/GettingStartedWebOnPrem.java @@ -84,7 +84,7 @@ import static fiftyone.common.testhelpers.LogbackHelper.configureLogback; import static fiftyone.ipintelligence.examples.shared.DataFileHelper.LITE_DATA_FILE_REL_PATH; -import static fiftyone.ipintelligence.examples.shared.DataFileHelper.IPI_PATH_ENV_VAR; +import static fiftyone.ipintelligence.examples.shared.DataFileHelper.IPI_PATH_VARIABLE; import static fiftyone.ipintelligence.examples.shared.PropertyHelper.asString; import static fiftyone.pipeline.util.FileFinder.getFilePath; @@ -125,7 +125,7 @@ public static void main(String[] args) throws Exception { } catch (Exception e) { logger.warn("Data file not found at expected location: {}. " + "An explicit path can be supplied via the {} environment variable.", - dataFile, IPI_PATH_ENV_VAR); + dataFile, IPI_PATH_VARIABLE); logger.warn("Will attempt to use default path from XML configuration"); logger.debug("Error finding data file", e); } diff --git a/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java b/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java index d3db631eb..ded023de3 100644 --- a/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java +++ b/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/FileUtils.java @@ -32,6 +32,7 @@ import java.nio.file.Path; import java.util.Objects; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import static fiftyone.pipeline.util.FileFinder.getFilePath; import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; @@ -43,7 +44,20 @@ public class FileUtils { * supply an explicit path to the IPI data file. When set and the file * exists it is used in preference to searching the project space. */ - public static final String IPI_PATH_ENV_VAR = "_51DEGREES_IPI_PATH"; + public static final String IPI_PATH_VARIABLE = "_51DEGREES_IPI_PATH"; + + /** + * The old name of {@link #IPI_PATH_VARIABLE}, read only when that one + * is not set. + * @deprecated use {@link #IPI_PATH_VARIABLE}, and set the variable it + * names in place of this one. + */ + @Deprecated + public static final String IPI_PATH_ENV_VAR = "51DEGREES_IPI_PATH"; + + // Set once the use of the deprecated name has been reported. + private static final AtomicBoolean OLD_NAME_REPORTED = + new AtomicBoolean(false); public static final String ENTERPRISE_IPI_DATA_FILE_NAME = "Enterprise-IpIntelligenceV41.ipi"; @@ -99,15 +113,26 @@ public static File getHashFile() { } /** - * Check the aligned {@link #IPI_PATH_ENV_VAR} environment variable and - * system property for an explicit path to the IPI data file. + * Check the aligned {@link #IPI_PATH_VARIABLE} environment variable and + * system property for an explicit path to the IPI data file, and when + * neither is set the deprecated {@link #IPI_PATH_ENV_VAR}, whose first + * use is reported on standard error. * * @return a file if the path is set and the file exists, otherwise null */ private static File getExplicitHashFile() { - String explicitPath = System.getenv(IPI_PATH_ENV_VAR); + String explicitPath = environmentOrProperty(IPI_PATH_VARIABLE); if (Objects.isNull(explicitPath)) { - explicitPath = System.getProperty(IPI_PATH_ENV_VAR); + explicitPath = environmentOrProperty(IPI_PATH_ENV_VAR); + if (Objects.nonNull(explicitPath) + && OLD_NAME_REPORTED.compareAndSet(false, true)) { + System.err.println( + "The environment variable " + IPI_PATH_ENV_VAR + + " is deprecated and " + IPI_PATH_VARIABLE + + " replaces it. Set " + IPI_PATH_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + IPI_PATH_ENV_VAR + "."); + } } if (Objects.nonNull(explicitPath)) { File explicitFile = new File(explicitPath); @@ -118,6 +143,19 @@ private static File getExplicitHashFile() { return null; } + /** + * The value of the environment variable of this name, or of the system + * property where that is not set or is empty, or null when neither + * holds a value. + */ + private static String environmentOrProperty(String name) { + String value = System.getenv(name); + if (Objects.isNull(value) || value.isEmpty()) { + value = System.getProperty(name); + } + return Objects.isNull(value) || value.isEmpty() ? null : value; + } + public static File getEvidenceFile() { if (Objects.nonNull(EVIDENCE_FILE)) { return EVIDENCE_FILE.orElse(null); diff --git a/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtils.java b/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtils.java index 8c6b1f225..75533a5ef 100644 --- a/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtils.java +++ b/ip-intelligence/ip-intelligence.shared/src/main/java/fiftyone/ipintelligence/shared/testhelpers/KeyUtils.java @@ -24,6 +24,7 @@ import java.util.Base64; import java.util.Objects; +import java.util.concurrent.atomic.AtomicBoolean; /** * Helpers to obtain keys from the environment @@ -33,19 +34,48 @@ public class KeyUtils { * Name of the aligned environment variable or system property which * supplies the resource key. This name is checked before any legacy name. */ - public static final String RESOURCE_KEY_ENV_VAR = "_51DEGREES_RESOURCE_KEY"; + public static final String RESOURCE_KEY_VARIABLE = "_51DEGREES_RESOURCE_KEY"; + + /** + * The old name of {@link #RESOURCE_KEY_VARIABLE}, read only when that + * one is not set. + * @deprecated use {@link #RESOURCE_KEY_VARIABLE}, and set the variable + * it names in place of this one. + */ + @Deprecated + public static final String RESOURCE_KEY_ENV_VAR = "51DEGREES_RESOURCE_KEY"; + + // Set once the use of the deprecated name has been reported. + private static final AtomicBoolean OLD_NAME_REPORTED = + new AtomicBoolean(false); /** * Obtain a resource key from an environment variable or a system property. *

- * The aligned name {@link #RESOURCE_KEY_ENV_VAR} is checked first and the - * supplied legacy name is checked second. Each name is looked up using + * The aligned name {@link #RESOURCE_KEY_VARIABLE} is checked first. When + * it is not set or is empty the deprecated {@link #RESOURCE_KEY_ENV_VAR} + * is checked, and its first use is reported on standard error. The + * supplied legacy name is checked last. Each name is looked up using * {@link #getNamedKey(String)}. * @param legacyKeyName legacy name to try when the aligned name is not set - * @return the resource key, or null if neither name is set + * @return the resource key, or null if no name is set */ public static String getResourceKey(String legacyKeyName) { - String resourceKey = getNamedKey(RESOURCE_KEY_ENV_VAR); + String resourceKey = getNamedKey(RESOURCE_KEY_VARIABLE); + if (Objects.isNull(resourceKey) || resourceKey.isEmpty()) { + String oldNameKey = getNamedKey(RESOURCE_KEY_ENV_VAR); + if (Objects.nonNull(oldNameKey) && oldNameKey.isEmpty() == false) { + if (OLD_NAME_REPORTED.compareAndSet(false, true)) { + System.err.println( + "The environment variable " + RESOURCE_KEY_ENV_VAR + + " is deprecated and " + RESOURCE_KEY_VARIABLE + + " replaces it. Set " + RESOURCE_KEY_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + RESOURCE_KEY_ENV_VAR + "."); + } + return oldNameKey; + } + } if (Objects.isNull(resourceKey)) { resourceKey = getNamedKey(legacyKeyName); } diff --git a/ip-intelligence/ip-intelligence.translation/src/test/java/fiftyone/ipintelligence/translation/CountryTranslationIntegrationTest.java b/ip-intelligence/ip-intelligence.translation/src/test/java/fiftyone/ipintelligence/translation/CountryTranslationIntegrationTest.java index 028bec978..3f5ab406b 100644 --- a/ip-intelligence/ip-intelligence.translation/src/test/java/fiftyone/ipintelligence/translation/CountryTranslationIntegrationTest.java +++ b/ip-intelligence/ip-intelligence.translation/src/test/java/fiftyone/ipintelligence/translation/CountryTranslationIntegrationTest.java @@ -50,7 +50,8 @@ * complete, localized, ordered country dropdown is produced. *

* The data file is resolved from the {@code TestDataFile} system property, the - * {@code _51DEGREES_IPI_PATH} environment variable, or a few well known + * {@code _51DEGREES_IPI_PATH} environment variable (or its deprecated + * name {@code 51DEGREES_IPI_PATH} when that is not set), or a few well known * locations, and the test is skipped if none is found. *

* The IP Intelligence element data key has differed between engine versions @@ -63,10 +64,38 @@ public class CountryTranslationIntegrationTest { private static final String SAMPLE_IP = "8.8.8.8"; + private static final String IPI_PATH_VARIABLE = "_51DEGREES_IPI_PATH"; + private static final String OLD_IPI_PATH_VARIABLE = "51DEGREES_IPI_PATH"; + + private static boolean oldNameReported = false; + + /** + * The path the environment gives, from the deprecated name when the + * name that replaces it is not set, saying so on standard error the + * first time. + */ + private static String pathFromEnvironment() { + String path = System.getenv(IPI_PATH_VARIABLE); + if (path == null || path.isEmpty()) { + path = System.getenv(OLD_IPI_PATH_VARIABLE); + if (path != null && path.isEmpty() == false + && oldNameReported == false) { + oldNameReported = true; + System.err.println( + "The environment variable " + OLD_IPI_PATH_VARIABLE + + " is deprecated and " + IPI_PATH_VARIABLE + + " replaces it. Set " + IPI_PATH_VARIABLE + + " to the same value, as a later release may no longer " + + "read " + OLD_IPI_PATH_VARIABLE + "."); + } + } + return path; + } + private static String resolveDataFile() { String[] candidates = { System.getProperty("TestDataFile"), - System.getenv("_51DEGREES_IPI_PATH"), + pathFromEnvironment(), "51Degrees-IPIV4EnterpriseIpiV41.ipi", "../51Degrees-IPIV4EnterpriseIpiV41.ipi", "../../51Degrees-IPIV4EnterpriseIpiV41.ipi", diff --git a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java index 01664c283..c7fb46d09 100644 --- a/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java +++ b/ip-intelligence/ip-intelligence/src/test/java/fiftyone/ipintelligence/OnPremisePerformanceProfileTests.java @@ -61,7 +61,7 @@ public static void findDataFile() { public void requireDataFile() { Assume.assumeTrue( "No IP Intelligence data file was found, so this test is " - + "skipped. Set " + FileUtils.IPI_PATH_ENV_VAR + " to the path " + + "skipped. Set " + FileUtils.IPI_PATH_VARIABLE + " to the path " + "of an .ipi file, or download the Lite file with " + "cxx/data/get-data-files.", dataFile != null); diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java index 9f19374f7..963827bdb 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.cloud-engine/src/main/java/pipeline/developerexamples/cloudengine/Main.java @@ -66,6 +66,15 @@ public class Main { private static final AtomicBoolean oldEndpointVariableLogged = new AtomicBoolean(false); + /** + * The deprecated name of the first of the resource key names, read + * when that one is not set. + */ + private static final String OLD_RESOURCE_KEY_VARIABLE = "RESOURCE_KEY"; + + private static final AtomicBoolean oldResourceKeyVariableLogged = + new AtomicBoolean(false); + /** * The cloud endpoint, taken from _51DEGREES_CLOUD_ENDPOINT when that is * set, which is the variable every 51Degrees cloud example honours and @@ -125,11 +134,26 @@ static String endpoint() { /** * The 51Degrees resource key that was found, or null when none of the - * names above is set anywhere. + * names above is set anywhere. When the first name is not set its + * deprecated name RESOURCE_KEY is read before the others, and the + * first use of it is logged as a warning. */ static String configuredResourceKey() { for (String name : RESOURCE_KEY_NAMES) { String value = env(name); + if (value == null && name.equals(RESOURCE_KEY_NAMES[0])) { + value = env(OLD_RESOURCE_KEY_VARIABLE); + if (value != null + && oldResourceKeyVariableLogged.compareAndSet( + false, true)) { + loggerFactory.getLogger(Main.class.getName()).warn( + "The environment variable " + OLD_RESOURCE_KEY_VARIABLE + + " is deprecated and " + name + + " replaces it. Set " + name + + " to the same value, as a later release may no longer " + + "read " + OLD_RESOURCE_KEY_VARIABLE + "."); + } + } if (value != null) { return value; } diff --git a/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/CreatorContextDemoServer.java b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/CreatorContextDemoServer.java index 1057eb480..3eaf3c223 100644 --- a/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/CreatorContextDemoServer.java +++ b/pipeline/pipeline.developer-examples/pipeline.developer-examples.fodid/src/main/java/pipeline/developerexamples/fodid/CreatorContextDemoServer.java @@ -102,9 +102,11 @@ * variables: *

    *
  • {@code _51DEGREES_RESOURCE_KEY}, required. The resource key of the - * page, public by nature.
    • + * page, public by nature. Its deprecated name {@code RESOURCE_KEY} is read + * when it is not set. *
  • {@code _51DEGREES_LICENSE_KEY}, optional. A licence key of the same - * account, server side only.
    • + * account, server side only. Its deprecated name {@code LICENSE_KEY} is + * read when it is not set. *
  • {@code _51DEGREES_CLOUD_ENDPOINT}, optional. The cloud API base including * the {@code /api/v4/} segment, defaulting to * {@code https://cloud.51degrees.com/api/v4/}. This is the same variable @@ -119,9 +121,9 @@ public class CreatorContextDemoServer { static final String RESOURCES = "/fodid/creator-context/"; static final String RESOURCE = - env("_51DEGREES_RESOURCE_KEY"); + key("_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY"); static final String LICENCE = - env("_51DEGREES_LICENSE_KEY"); + key("_51DEGREES_LICENSE_KEY", "LICENSE_KEY"); /** * The one client, built at start-up and shared by every request, @@ -462,6 +464,27 @@ static String env(String... names) { return null; } + /** + * The value of the named environment variable, or of its deprecated + * old name when the named one is not set, saying on standard error + * that the old name is deprecated when it is the one used. + */ + static String key(String name, String oldName) { + String value = env(name); + if (value == null) { + value = env(oldName); + if (value != null) { + System.err.println( + "The environment variable " + oldName + + " is deprecated and " + name + + " replaces it. Set " + name + + " to the same value, as a later release may no longer " + + "read " + oldName + "."); + } + } + return value; + } + static String envOr(String name, String fallback) { String value = env(name); return value == null ? fallback : value; diff --git a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java index 26061200e..88b51e27e 100644 --- a/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java +++ b/pipeline/pipeline.did/src/test/java/fiftyone/pipeline/did/LiveCloudKeys.java @@ -22,6 +22,10 @@ package fiftyone.pipeline.did; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + /** * Where the tests that call the live 51Degrees cloud service read their * resource key and licence key from, and what they say when there is @@ -65,18 +69,65 @@ final class LiveCloudKeys { private LiveCloudKeys() { } + /** + * The names that have a deprecated old name, each followed by it. The + * old name is read only when its new name is not set. + */ + private static final String[][] DEPRECATED_NAMES = { + {"_51DEGREES_RESOURCE_KEY", "RESOURCE_KEY"}, + {"_51DEGREES_LICENSE_KEY", "LICENSE_KEY"}, + }; + + /** The old names whose use has been reported, each reported once. */ + private static final Set REPORTED_OLD_NAMES = + Collections.synchronizedSet(new HashSet()); + /** * The first value set under any of the given names, or null when none - * of them is set anywhere. + * of them is set anywhere. A name with a deprecated old name falls + * back to it, saying so on standard error the first time. */ static String find(String[] names) { for (String name : names) { - String value = System.getenv(name); - if (value == null || value.trim().isEmpty()) { - value = System.getProperty(name); + String value = read(name); + if (value == null) { + String oldName = oldNameOf(name); + if (oldName != null) { + value = read(oldName); + if (value != null && REPORTED_OLD_NAMES.add(oldName)) { + System.err.println( + "The environment variable " + oldName + + " is deprecated and " + name + + " replaces it. Set " + name + + " to the same value, as a later release may no longer " + + "read " + oldName + "."); + } + } } - if (value != null && value.trim().isEmpty() == false) { - return value.trim(); + if (value != null) { + return value; + } + } + return null; + } + + /** + * The trimmed value of the environment variable of this name, or of + * the system property where that is not set or is blank, or null when + * neither holds a value. + */ + private static String read(String name) { + String value = System.getenv(name); + if (value == null || value.trim().isEmpty()) { + value = System.getProperty(name); + } + return value == null || value.trim().isEmpty() ? null : value.trim(); + } + + private static String oldNameOf(String name) { + for (String[] pair : DEPRECATED_NAMES) { + if (pair[0].equals(name)) { + return pair[1]; } } return null; From 02b1bca050841eda152be6707dc21feddbc81e54 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 19:11:07 +0000 Subject: [PATCH 5/5] Guard two tests against the old names they now meet Two tests run a helper with nothing set, and each checked and cleared only the new name of a variable. The helpers now read the old name when the new one is not set, so each test failed wherever the old name was set. KeyHelperTest.getOrSetTestResourceKey_NoKey_StoresNothing now also checks and clears 51DEGREES_RESOURCE_KEY, and ExamplesWithoutSettingsTest also checks and clears 51DEGREES_IPI_PATH, through the deprecated constant that holds it. --- .../console/ExamplesWithoutSettingsTest.java | 12 +++++++++++- .../examples/shared/KeyHelperTest.java | 14 ++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java b/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java index 61572fc88..d14a522f6 100644 --- a/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java +++ b/examples/ip-intelligence/console/src/test/java/fiftyone/ipintelligence/examples/console/ExamplesWithoutSettingsTest.java @@ -57,14 +57,20 @@ private interface Main { /** * Runs the example's main with no arguments and the data file and * licence key system properties removed, and returns what it wrote to - * standard output. + * standard output. The deprecated name of the data file variable is + * read when the current one is not set, so both are checked and + * removed. */ + @SuppressWarnings("deprecation") private static String runWithoutSettings(Main main) throws Exception { assumeTrue("Skipping, a data file or licence key is set in the environment", System.getenv(DataFileHelper.IPI_PATH_VARIABLE) == null && + System.getenv(DataFileHelper.IPI_PATH_ENV_VAR) == null && System.getenv(LICENSE_KEY) == null && System.getenv(LICENSE_KEY.toUpperCase()) == null); String dataFile = System.clearProperty(DataFileHelper.IPI_PATH_VARIABLE); + String oldNameDataFile = + System.clearProperty(DataFileHelper.IPI_PATH_ENV_VAR); String licenseKey = System.clearProperty(LICENSE_KEY); PrintStream out = System.out; ByteArrayOutputStream buffer = new ByteArrayOutputStream(); @@ -76,6 +82,10 @@ private static String runWithoutSettings(Main main) throws Exception { if (dataFile != null) { System.setProperty(DataFileHelper.IPI_PATH_VARIABLE, dataFile); } + if (oldNameDataFile != null) { + System.setProperty(DataFileHelper.IPI_PATH_ENV_VAR, + oldNameDataFile); + } if (licenseKey != null) { System.setProperty(LICENSE_KEY, licenseKey); } diff --git a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java index 756cb822b..df25ee4b2 100644 --- a/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java +++ b/examples/ip-intelligence/shared/src/test/java/fiftyone/ipintelligence/examples/shared/KeyHelperTest.java @@ -24,6 +24,7 @@ import org.junit.Test; +import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; import java.util.List; @@ -42,7 +43,8 @@ public class KeyHelperTest { private static final String KEY_B = "AgAAAAAAAAAAAAAAAAAAAAAA"; /** - * The names getOrSetTestResourceKey reads, in its order. + * The names getOrSetTestResourceKey reads, in its order, apart from + * the deprecated name it reads when the first is not set. */ private static final List EXAMPLE_NAMES = Arrays.asList( KeyHelper.RESOURCE_KEY_VARIABLE, KeyHelper.TEST_RESOURCE_KEY); @@ -53,20 +55,24 @@ public class KeyHelperTest { */ @Test public void getOrSetTestResourceKey_NoKey_StoresNothing() { + // The deprecated name is read as well, so it must not be set + // either. + List namesRead = new ArrayList<>(EXAMPLE_NAMES); + namesRead.add(OLD_KEY_NAME); // A test can clear a system property but not an environment // variable. - for (String name : EXAMPLE_NAMES) { + for (String name : namesRead) { assumeTrue(name + " is set in the environment", System.getenv(name) == null && System.getenv(name.toUpperCase()) == null); } Map saved = new HashMap<>(); - for (String name : EXAMPLE_NAMES) { + for (String name : namesRead) { saved.put(name, System.clearProperty(name)); } try { assertNull(KeyHelper.getOrSetTestResourceKey(false)); - for (String name : EXAMPLE_NAMES) { + for (String name : namesRead) { assertNull(name, System.getProperty(name)); } } finally {