From a192fbf87bf082bb7e7ea1cedfa96ea2a544bfd3 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:00:52 +0000 Subject: [PATCH 01/10] Test the credentials a Cloud Request Engine accepts and sends Cover the rows of the specification's table of credentials, the data request for a license key alone, a resource key alone, both keys, a resource key with requested properties and all three, the accessible properties request for each, the report of requested properties the answer left out, and the license key staying out of every URL, log entry and error message when a request fails. --- .../CloudRequestEngineLicenseKeyTests.java | 536 ++++++++++++++++++ 1 file changed, 536 insertions(+) create mode 100644 pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java new file mode 100644 index 00000000..09163a9e --- /dev/null +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -0,0 +1,536 @@ +/* ********************************************************************* + * 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.cloudrequestengine.flowelements; + +import fiftyone.pipeline.core.data.AccessiblePropertyMetaData; +import fiftyone.pipeline.core.data.FlowData; +import fiftyone.pipeline.core.data.FlowError; +import fiftyone.pipeline.core.exceptions.PipelineConfigurationException; +import fiftyone.pipeline.core.flowelements.Pipeline; +import fiftyone.pipeline.core.flowelements.PipelineBuilder; +import fiftyone.pipeline.engines.services.HttpClient; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.ILoggerFactory; +import org.slf4j.Logger; + +import java.io.IOException; +import java.io.UnsupportedEncodingException; +import java.net.HttpURLConnection; +import java.net.URL; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.*; + +/** + * The credentials a Cloud Request Engine accepts, what it sends for each + * and that a license key stays out of URLs, logs and messages. + */ +public class CloudRequestEngineLicenseKeyTests { + + private static final String BASE = "https://cloud.example/api/v4/"; + private static final String LICENSE_KEY = "abcdefghijkl1234"; + private static final String RESOURCE_KEY = "resourcekey"; + private static final String PROPERTIES = "device.ismobile,device.iscrawler"; + private static final List PROPERTY_LIST = + Arrays.asList("device.ismobile", "device.iscrawler"); + + /** A request the engine made. */ + private static class Call { + final String method; + final URL url; + final String body; + + Call(String method, URL url, String body) { + this.method = method; + this.url = url; + this.body = body; + } + } + + /** Records every request and answers from fixed responses. */ + private static class FakeCloud implements HttpClient { + final List calls = new ArrayList<>(); + String dataResponse = "{\"device\":{\"ismobile\":true," + + "\"iscrawler\":false}}"; + int dataStatus = 200; + String propertiesResponse = + "{\"Products\":{\"device\":{\"DataTier\":\"t\",\"Properties\":" + + "[{\"Name\":\"IsMobile\",\"Type\":\"Boolean\"," + + "\"Category\":\"Device\"}]}}}"; + String evidenceKeysResponse = "[\"query.User-Agent\"]"; + private final Map urls = new HashMap<>(); + + @Override + public HttpURLConnection connect(URL url) { + HttpURLConnection connection = mock(HttpURLConnection.class); + try { + when(connection.getURL()).thenReturn(url); + when(connection.getResponseCode()).thenReturn( + url.getPath().endsWith("json") ? dataStatus : 200); + } catch (IOException e) { + throw new IllegalStateException(e); + } + when(connection.getHeaderFields()).thenReturn( + Collections.emptyMap()); + urls.put(connection, url); + return connection; + } + + @Override + public String postData( + HttpURLConnection connection, + Map headers, + byte[] data) { + URL url = urls.get(connection); + calls.add(new Call( + "POST", + url, + data == null ? "" : + new String(data, StandardCharsets.UTF_8))); + return answer(url); + } + + @Override + public String getResponseString(HttpURLConnection connection) { + return getResponseString(connection, null); + } + + @Override + public String getResponseString( + HttpURLConnection connection, + Map headers) { + URL url = urls.get(connection); + calls.add(new Call("GET", url, "")); + return answer(url); + } + + private String answer(URL url) { + if (url.getPath().endsWith("accessibleproperties")) { + return propertiesResponse; + } + if (url.getPath().endsWith("evidencekeys")) { + return evidenceKeysResponse; + } + return dataResponse; + } + + List callsTo(String pathEnd) { + List found = new ArrayList<>(); + for (Call call : calls) { + if (call.url.getPath().endsWith(pathEnd)) { + found.add(call); + } + } + return found; + } + } + + private FakeCloud cloud; + private Logger logger; + private ILoggerFactory loggerFactory; + + @BeforeEach + public void setUp() { + cloud = new FakeCloud(); + logger = mock(Logger.class); + loggerFactory = mock(ILoggerFactory.class); + when(loggerFactory.getLogger(anyString())).thenReturn(logger); + } + + private CloudRequestEngineBuilder builder() { + return new CloudRequestEngineBuilder(loggerFactory, cloud); + } + + /** + * Build with the test endpoint set last, as the default endpoint is when + * no endpoint is set, so the data endpoint's route reflects the keys. + */ + private CloudRequestEngine build(CloudRequestEngineBuilder builder) + throws Exception { + return builder.setEndpoint(BASE).build(); + } + + private String process(CloudRequestEngine engine) throws Exception { + try (Pipeline pipeline = new PipelineBuilder(loggerFactory) + .setSuppressProcessException(true) + .addFlowElement(engine) + .build(); + FlowData data = pipeline.createFlowData()) { + data.addEvidence("query.User-Agent", "iPhone"); + data.process(); + StringBuilder errors = new StringBuilder(); + for (FlowError error : data.getErrors()) { + for (Throwable t = error.getThrowable(); t != null; + t = t.getCause()) { + errors.append(t).append('\n'); + } + } + return errors.toString(); + } + } + + private static Map form(String body) + throws UnsupportedEncodingException { + Map fields = new HashMap<>(); + for (String pair : body.split("&")) { + if (pair.isEmpty()) { + continue; + } + String[] parts = pair.split("=", 2); + fields.put( + URLDecoder.decode(parts[0], "UTF-8"), + URLDecoder.decode(parts.length > 1 ? parts[1] : "", "UTF-8")); + } + return fields; + } + + private Map dataRequest(String pathEnd) + throws UnsupportedEncodingException { + List posts = cloud.callsTo(pathEnd); + assertEquals(1, posts.size()); + assertEquals("POST", posts.get(0).method); + return form(posts.get(0).body); + } + + private void assertRefused( + CloudRequestEngineBuilder builder, + String... expectedInMessage) { + PipelineConfigurationException e = assertThrows( + PipelineConfigurationException.class, + () -> build(builder)); + for (String expected : expectedInMessage) { + assertTrue( + e.getMessage().contains(expected), + "Message should contain '" + expected + "' but was: " + + e.getMessage()); + } + assertTrue(cloud.calls.isEmpty(), "A refused build sends nothing."); + } + + // The rows of the specification's table of accepted combinations. + + @Test + public void Build_ResourceKeyOnly_Accepted() throws Exception { + assertNotNull(build(builder().setResourceKey(RESOURCE_KEY))); + } + + @Test + public void Build_ResourceKeyAndLicenseKey_Accepted() throws Exception { + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(LICENSE_KEY))); + } + + @Test + public void Build_AllThree_Accepted() throws Exception { + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(LICENSE_KEY) + .setProperties(PROPERTY_LIST))); + } + + @Test + public void Build_LicenseKeyAndProperties_Accepted() throws Exception { + assertNotNull(build(builder() + .setLicenseKey(LICENSE_KEY) + .setProperties(PROPERTY_LIST))); + } + + @Test + public void Build_LicenseKeyWithoutProperties_Refused() { + assertRefused( + builder().setLicenseKey(LICENSE_KEY), + "answers 400", + "setProperty"); + } + + @Test + public void Build_ResourceKeyWithProperties_Accepted() throws Exception { + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setProperty("device.hardwarevendor"))); + } + + @Test + public void Build_NoCredential_Refused() { + assertRefused(builder(), "401", "resource key"); + assertRefused(builder().setProperties(PROPERTY_LIST), "401"); + } + + @Test + public void Build_BlankValuesCountAsAbsent() throws Exception { + assertRefused( + builder().setResourceKey(" ").setLicenseKey("\n"), + "401"); + assertRefused( + builder() + .setLicenseKey(LICENSE_KEY) + .setProperties(Arrays.asList(" ", "")), + "answers 400"); + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(" ") + .setProperty(" "))); + } + + @Test + public void Build_RefusalsDoNotContainTheLicenseKey() { + PipelineConfigurationException e = assertThrows( + PipelineConfigurationException.class, + () -> build(builder().setLicenseKey(LICENSE_KEY))); + assertFalse(e.getMessage().contains(LICENSE_KEY)); + } + + // Requests. + + @Test + public void Process_LicenseKeyAlone_SendsLicenseAndValues() + throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY + "\n") + .setProperties( + Arrays.asList(" Device.IsMobile", "device.iscrawler "))); + + assertEquals("", process(engine)); + + Map fields = dataRequest("/api/v4/json"); + assertEquals(LICENSE_KEY, fields.get("license")); + assertEquals(PROPERTIES, fields.get("values")); + assertFalse(fields.containsKey("resource")); + assertFalse(fields.containsKey("licence")); + assertEquals("iPhone", fields.get("User-Agent")); + } + + @Test + public void Process_ResourceKey_SendsResourceOnly() throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY)); + + assertEquals("", process(engine)); + + Map fields = dataRequest(RESOURCE_KEY + ".json"); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertFalse(fields.containsKey("license")); + assertFalse(fields.containsKey("values")); + } + + @Test + public void Process_ResourceKeyAndLicenseKey_SendsBoth() + throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(LICENSE_KEY)); + + assertEquals("", process(engine)); + + Map fields = dataRequest(RESOURCE_KEY + ".json"); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertEquals(LICENSE_KEY, fields.get("license")); + assertFalse(fields.containsKey("values")); + } + + @Test + public void Process_ResourceKeyAndProperties_SendsResourceAndValues() + throws Exception { + cloud.dataResponse = "{\"device\":{\"ismobile\":true," + + "\"hardwarevendor\":null," + + "\"hardwarevendornullreason\":\"Paid feature\"}}"; + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setProperty("Device.HardwareVendor")); + + assertEquals("", process(engine)); + + Map fields = dataRequest(RESOURCE_KEY + ".json"); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertEquals("device.hardwarevendor", fields.get("values")); + assertFalse(fields.containsKey("license")); + verify(logger, never()).warn(anyString()); + } + + @Test + public void Process_AllThree_SendsResourceLicenseAndValues() + throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(LICENSE_KEY) + .setProperty("IP.RegisteredName")); + cloud.dataResponse = "{\"device\":{\"ismobile\":true}," + + "\"ip\":{\"registeredname\":\"Example\"}}"; + + assertEquals("", process(engine)); + + Map fields = dataRequest(RESOURCE_KEY + ".json"); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertEquals(LICENSE_KEY, fields.get("license")); + assertEquals("ip.registeredname", fields.get("values")); + for (Call call : cloud.calls) { + assertFalse(call.url.toString().contains(LICENSE_KEY)); + } + } + + // Discovery. + + @Test + public void Discovery_LicenseKeyAlone_PostsCredentialsInTheBody() + throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setProperties(PROPERTY_LIST)); + + Map products = + engine.getPublicProperties(); + assertTrue(products.containsKey("device")); + assertNotNull(engine.getEvidenceKeyFilter()); + + List properties = cloud.callsTo("accessibleproperties"); + assertEquals(1, properties.size()); + assertEquals("POST", properties.get(0).method); + assertNull(properties.get(0).url.getQuery()); + assertFalse(properties.get(0).url.toString().contains(LICENSE_KEY)); + Map fields = form(properties.get(0).body); + assertEquals(LICENSE_KEY, fields.get("license")); + assertFalse(fields.containsKey("resource")); + + List evidence = cloud.callsTo("evidencekeys"); + assertEquals(1, evidence.size()); + assertEquals("GET", evidence.get(0).method); + assertNull(evidence.get(0).url.getQuery()); + } + + @Test + public void Discovery_ResourceKeyAndProperties_GetsWithTheKeyInTheQuery() + throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setProperty("device.hardwarevendor")); + + engine.getPublicProperties(); + + List properties = cloud.callsTo("accessibleproperties"); + assertEquals(1, properties.size()); + assertEquals("GET", properties.get(0).method); + assertEquals( + "Resource=" + RESOURCE_KEY, + properties.get(0).url.getQuery()); + } + + @Test + public void Discovery_BothKeys_PostsBothInTheBody() throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(LICENSE_KEY)); + + engine.getPublicProperties(); + + Call call = cloud.callsTo("accessibleproperties").get(0); + assertEquals("POST", call.method); + assertNull(call.url.getQuery()); + Map fields = form(call.body); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertEquals(LICENSE_KEY, fields.get("license")); + } + + // Keys in URLs, logs and messages. + + @Test + public void Process_FailedRequest_LicenseKeyInNoUrlLogOrMessage() + throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setProperties(Arrays.asList( + "device.ismobile", "device.other"))); + assertEquals("", process(engine)); + + cloud.dataStatus = 500; + cloud.dataResponse = "{\"status\":\"failure\"}"; + String errors = process(engine); + + assertTrue(errors.contains("500"), errors); + assertFalse(errors.contains(LICENSE_KEY), errors); + for (Call call : cloud.calls) { + assertFalse(call.url.toString().contains(LICENSE_KEY)); + } + assertFalse(mockingDetails(logger).getInvocations().isEmpty()); + mockingDetails(logger).getInvocations().forEach(i -> + assertFalse( + Arrays.deepToString(i.getArguments()).contains(LICENSE_KEY), + i.toString())); + } + + // Properties the service did not return. + + @Test + public void Process_PropertyDropped_ReportedOnceAndRequestSucceeds() + throws Exception { + cloud.dataResponse = "{\"device\":{\"ismobile\":true}}"; + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setProperties( + Arrays.asList("Device.IsMobile", "Device.IsCrawler"))); + + assertEquals("", process(engine)); + assertEquals("", process(engine)); + + verify(logger, times(1)).warn(argThat( + (String m) -> m.contains("device.iscrawler") && + m.contains("not covered") && + !m.contains("device.ismobile"))); + verify(logger, times(1)).warn(anyString()); + } + + @Test + public void Process_PropertyPresentButNull_CountsAsAnswered() + throws Exception { + cloud.dataResponse = "{\"device\":{\"ismobile\":true," + + "\"iscrawler\":null,\"iscrawlernullreason\":\"No data\"}}"; + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setProperties(PROPERTY_LIST)); + + assertEquals("", process(engine)); + + verify(logger, never()).warn(anyString()); + } + + @Test + public void Process_ResourceKey_NothingToCompare() throws Exception { + cloud.dataResponse = "{\"device\":{\"ismobile\":true}}"; + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY)); + + assertEquals("", process(engine)); + + verify(logger, never()).warn(anyString()); + } +} From 389895855c40e8219c8f7c83a691b2d89f58244f Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:00:55 +0000 Subject: [PATCH 02/10] Authenticate a Cloud Request Engine on a license key alone A license key without a resource key is accepted when the properties wanted are named. The properties set with setProperty or setProperties, which reached newEngine and were never used, now go to the cloud service as 'values' whenever the list is not empty, beside a resource key, a license key or both, so the service adds them to its answer. The builder refuses at build time only the two combinations the cloud service refuses on every request, being no credential at all and a license key with no requested properties, each with a message that says why the combination cannot work. When a license key is set, the accessible properties request is a POST that carries the license key, and the resource key if there is one, in its body, so discovery finds what the license key grants and the key never appears in a URL. A resource key alone is sent in the query string as before. The engine warns once about requested properties the answer left out. A blank resource key counts as absent, so the data endpoint for a license key alone is the plain json endpoint. --- .../cloudrequestengine/Constants.java | 22 +++ .../CloudRequestEngineBuilder.java | 84 +++++++++-- .../CloudRequestEngineDefault.java | 140 +++++++++++++++++- 3 files changed, 225 insertions(+), 21 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java index 593c618d..538c0111 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java @@ -27,6 +27,28 @@ public class Constants { public static final String OriginHeaderName = "Origin"; public class Messages { + public static final String NoCredential = + "The Cloud Request Engine needs a resource key or a license key " + + "to authenticate with, as the cloud service answers 401 to a " + + "request that carries neither. Use setResourceKey(String) with " + + "a resource key created for free at " + + "https://configure.51degrees.com?utm_source=code&utm_medium=comment&utm_campaign=51degrees-java&utm_content=pipeline.cloudrequestengine-src-main-java-fiftyone-pipeline-cloudrequestengine-constants.java&utm_term=no-credential"; + + public static final String LicenseKeyNeedsProperties = + "The Cloud Request Engine has a license key and no requested " + + "properties. A license key names no properties of its own, so " + + "the cloud service answers 400 to every request made with one " + + "that does not list the properties it wants. Use setProperty " + + "or setProperties to name the fully qualified properties " + + "wanted, for example \"device.ismobile\" and " + + "\"device.iscrawler\"."; + + public static final String PropertiesNotCovered = + "The cloud service did not return the requested properties %s " + + "in its answer. They are not covered by the keys in use, so " + + "they will be missing from every result. The other requested " + + "properties came back and are usable."; + public static final String EvidenceConflict = "'%s:%s' evidence conflicts with %s"; public static final String MessageNoDataInResponse = 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 bc72731b..27808486 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 @@ -25,6 +25,7 @@ import fiftyone.pipeline.annotations.DefaultValue; import fiftyone.pipeline.annotations.ElementBuilder; +import fiftyone.pipeline.cloudrequestengine.Constants.Messages; import fiftyone.pipeline.cloudrequestengine.data.CloudRequestData; import fiftyone.pipeline.core.data.FlowData; import fiftyone.pipeline.core.data.factories.ElementDataFactory; @@ -36,7 +37,9 @@ import fiftyone.pipeline.engines.services.HttpClientDefault; import org.slf4j.ILoggerFactory; +import java.util.ArrayList; import java.util.List; +import java.util.Locale; import java.util.function.Function; /** @@ -75,12 +78,13 @@ public CloudRequestEngineBuilder( @Override protected CloudRequestEngine newEngine(List properties) throws Exception { - if(resourceKey == null || resourceKey.isEmpty()){ - throw new PipelineConfigurationException("A resource key is " + - "required to access the cloud server. Please use the " + - "'setResourceKey(String) method to supply your resource " + - "key obtained from https://configure.51degrees.com?utm_source=code&utm_medium=comment&utm_campaign=51degrees-java&utm_content=pipeline.cloudrequestengine-src-main-java-fiftyone-pipeline-cloudrequestengine-flowelements-cloudrequestenginebuilder.java&utm_term=resource-key-required"); - } + String trimmedResourceKey = trimmed(resourceKey); + String trimmedLicenseKey = trimmed(licenseKey); + List requestedProperties = normalized(properties); + checkCredentials( + trimmedResourceKey, + trimmedLicenseKey, + requestedProperties); /* * Check if endPoint has been explicitly set via setEndpoint. @@ -103,12 +107,56 @@ protected CloudRequestEngine newEngine(List properties) throws Exception new CloudRequestDataFactory(loggerFactory), httpClient, dataEndpoint, - resourceKey, - licenseKey, + trimmedResourceKey, + trimmedLicenseKey, propertiesEndpoint, evidenceKeysEndpoint, - timeoutMillis, - cloudRequestOrigin); + timeoutMillis, + cloudRequestOrigin, + requestedProperties); + } + + private static String trimmed(String value) { + return value == null ? "" : value.trim(); + } + + /** + * The property names to send as values, trimmed and lower-cased, as the + * cloud service answers in lower case, with blank names and repeats + * left out. + */ + private static List normalized(List names) { + List result = new ArrayList<>(); + for (String name : names) { + String normal = trimmed(name).toLowerCase(Locale.ROOT); + if (normal.isEmpty() == false && result.contains(normal) == false) { + result.add(normal); + } + } + return result; + } + + /** + * Refuse the two combinations of keys and requested properties that the + * cloud service refuses on every request, so that a deployment sees one + * configuration error when the engine is built. A blank value counts as + * absent. + */ + private static void checkCredentials( + String resourceKey, + String licenseKey, + List requestedProperties) { + boolean hasResourceKey = resourceKey.isEmpty() == false; + boolean hasLicenseKey = licenseKey.isEmpty() == false; + + if (hasResourceKey == false && hasLicenseKey == false) { + throw new PipelineConfigurationException( + Messages.NoCredential); + } + if (hasResourceKey == false && requestedProperties.isEmpty()) { + throw new PipelineConfigurationException( + Messages.LicenseKeyNeedsProperties); + } } private String getEnvironmentVariable(String name) { @@ -133,7 +181,8 @@ public CloudRequestEngineBuilder setEndpoint(String uri) { uri += '/'; } endPoint = uri; - return setDataEndpoint(uri + (resourceKey != null ? resourceKey + "." : "") + "json") + String key = trimmed(resourceKey); + return setDataEndpoint(uri + (key.isEmpty() ? "" : key + ".") + "json") .setPropertiesEndpoint(uri + "accessibleproperties") .setEvidenceKeysEndpoint(uri + "evidencekeys"); @@ -142,7 +191,8 @@ public CloudRequestEngineBuilder setEndpoint(String uri) { /** * The endpoint the CloudRequestEngine will query to get a processing result. *

- * By default, this is the endpoint value suffixed with the resourcekey and ".json" + * By default, this is the endpoint value suffixed with the resourcekey and + * ".json", or with "json" alone when no resource key is set * @param uri data endpoint * @return this builder */ @@ -183,11 +233,11 @@ public CloudRequestEngineBuilder setEvidenceKeysEndpoint(String uri) { /** * The resource key to query the endpoint with. *

- * No default, a value must be supplied + * No default. Either a resource key or a license key must be supplied. * @param resourceKey resource key * @return this builder */ - @DefaultValue("No default - a resource key must be supplied") + @DefaultValue("No default - a resource key or a license key must be supplied") public CloudRequestEngineBuilder setResourceKey(String resourceKey) { this.resourceKey = resourceKey; return this; @@ -196,6 +246,12 @@ public CloudRequestEngineBuilder setResourceKey(String resourceKey) { /** * The license key to query the endpoint with. *

+ * A license key used without a resource key needs the properties + * wanted, named with {@link #setProperty(String)} or + * {@link #setProperties(List)}, which are sent as values. Beside a + * resource key it adds the requested properties the license grants to + * what the resource key carries. + *

* There is no default * @param licenseKey license key * @return this builder diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java index 12ccc17f..5c563dcf 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java @@ -37,6 +37,7 @@ import fiftyone.pipeline.engines.services.HttpClient; import fiftyone.pipeline.exceptions.AggregateException; import org.json.JSONArray; +import org.json.JSONException; import org.json.JSONObject; import org.slf4j.Logger; @@ -50,6 +51,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -81,6 +83,9 @@ public class CloudRequestEngineDefault private String evidenceKeysEndpoint; private String cloudRequestOrigin; private Integer timeoutMillis; + private final List requestedProperties; + private final AtomicBoolean requestedPropertiesChecked = + new AtomicBoolean(false); private List propertyMetaData; private volatile Map publicProperties; @@ -133,7 +138,6 @@ public CloudRequestEngineDefault( cloudRequestOrigin); } - @SuppressWarnings("this-escape") public CloudRequestEngineDefault( Logger logger, ElementDataFactory aspectDataFactory, @@ -145,7 +149,42 @@ public CloudRequestEngineDefault( String evidenceKeysEndpoint, int timeoutMillis, String cloudRequestOrigin) { + this( + logger, + aspectDataFactory, + httpClient, + endPoint, + resourceKey, + licenseKey, + propertiesEndpoint, + evidenceKeysEndpoint, + timeoutMillis, + cloudRequestOrigin, + Collections.emptyList()); + } + + /** + * Constructor. + * @param requestedProperties the lower-case fully qualified names of the + * properties to ask for, sent as {@code values} + * when the list is not empty + */ + @SuppressWarnings("this-escape") + public CloudRequestEngineDefault( + Logger logger, + ElementDataFactory aspectDataFactory, + HttpClient httpClient, + String endPoint, + String resourceKey, + String licenseKey, + String propertiesEndpoint, + String evidenceKeysEndpoint, + int timeoutMillis, + String cloudRequestOrigin, + List requestedProperties) { super(logger, aspectDataFactory); + this.requestedProperties = Collections.unmodifiableList( + new ArrayList<>(requestedProperties)); this.endPoint = endPoint; this.resourceKey = resourceKey; @@ -238,6 +277,57 @@ protected void processEngine(FlowData data, CloudRequestData aspectData) throws ((CloudRequestDataInternal)aspectData).setJsonResponse(response); validateResponse(response, connection); + reportPropertiesNotCovered(response); + } + + /** + * Compare the properties asked for with the ones the answer carries, and + * warn once about any left out. The service leaves out a requested + * property it does not answer without naming it, and lower-cases the + * names it answers with. A property that is present but null counts as + * answered, as it carries a null reason beside it. The request is never + * failed, because the properties that did come back are usable. + */ + private void reportPropertiesNotCovered(String response) { + if (requestedProperties.isEmpty() || + requestedPropertiesChecked.compareAndSet(false, true) == false) { + return; + } + JSONObject answer; + try { + answer = new JSONObject(response); + } catch (JSONException e) { + return; + } + List notCovered = new ArrayList<>(); + for (String name : requestedProperties) { + String[] parts = name.split( + Pattern.quote(Constants.EVIDENCE_SEPERATOR), 2); + if (parts.length < 2 || hasKeyIgnoringCase( + answer.optJSONObject(keyIgnoringCase(answer, parts[0])), + parts[1]) == false) { + notCovered.add(name); + } + } + if (notCovered.isEmpty() == false) { + logger.warn(String.format( + PropertiesNotCovered, + stringJoin(notCovered, ", "))); + } + } + + private static String keyIgnoringCase(JSONObject object, String name) { + for (String key : object.keySet()) { + if (key.equalsIgnoreCase(name)) { + return key; + } + } + return name; + } + + private static boolean hasKeyIgnoringCase(JSONObject object, String name) { + return object != null && + object.has(keyIgnoringCase(object, name)); } /** @@ -257,9 +347,10 @@ private byte[] getContent(FlowData data) throws UnsupportedEncodingException { Map formData = getFormData(data); List formItems = new ArrayList<>(); - formItems.add("resource=" + resourceKey); - if(licenseKey != null && licenseKey.isEmpty() == false){ - formItems.add("license=" + licenseKey); + formItems.addAll(credentialItems()); + if (requestedProperties.isEmpty() == false) { + formItems.add("values=" + URLEncoder.encode( + stringJoin(requestedProperties, ","), "UTF-8")); } List formKeys = Arrays.asList(formData.keySet().toArray(new String[0])); @@ -274,6 +365,26 @@ private byte[] getContent(FlowData data) throws UnsupportedEncodingException { return string.getBytes(StandardCharsets.UTF_8); } + private boolean hasResourceKey() { + return resourceKey != null && resourceKey.isEmpty() == false; + } + + private boolean hasLicenseKey() { + return licenseKey != null && licenseKey.isEmpty() == false; + } + + /** The resource and license keys that are set, as form items. */ + private List credentialItems() throws UnsupportedEncodingException { + List items = new ArrayList<>(); + if (hasResourceKey()) { + items.add("resource=" + URLEncoder.encode(resourceKey, "UTF-8")); + } + if (hasLicenseKey()) { + items.add("license=" + URLEncoder.encode(licenseKey, "UTF-8")); + } + return items; + } + Map getFormData(FlowData flowData) { Map evidence = new HashMap<>( flowData.getEvidence().asKeyMap()); @@ -432,9 +543,24 @@ private void getCloudProperties() throws CloudRequestException, AggregateExcepti Map headers = new HashMap<>(); setCommonHeaders(headers); - HttpURLConnection connection = httpClient.connect(toUrl(propertiesEndpoint.trim() + (resourceKey != null ? "?Resource=" + resourceKey : ""))); - jsonResult = httpClient.getResponseString(connection, headers); - validateResponse(jsonResult, connection); + if (hasLicenseKey()) { + // The credentials go in the body, not the URL, so a license + // key stays out of request logs. + byte[] content = stringJoin(credentialItems(), "&") + .getBytes(StandardCharsets.UTF_8); + headers.put( + "Content-Type", "application/x-www-form-urlencoded"); + headers.put("Content-Length", Integer.toString(content.length)); + HttpURLConnection connection = + httpClient.connect(toUrl(propertiesEndpoint.trim())); + jsonResult = httpClient.postData(connection, headers, content); + validateResponse(jsonResult, connection); + } else { + HttpURLConnection connection = httpClient.connect(toUrl( + propertiesEndpoint.trim() + "?Resource=" + resourceKey)); + jsonResult = httpClient.getResponseString(connection, headers); + validateResponse(jsonResult, connection); + } JSONObject jsonObj = null; if (jsonResult.isEmpty() == false) { From 0fba7fe3f4b18e150c06011fdf8b289c73f20c19 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:45:45 +0000 Subject: [PATCH 03/10] Send only the properties named with setRequestedProperties The previous commit sent the properties set with setProperty and setProperties as 'values'. On rc that list reached newEngine and was never sent, so a configuration that set it, as the .NET cloud web examples do, would change the cloud service's answer. The .NET full build showed that answer losing device.deviceid. The engine builder now keeps its own list, set with setRequestedProperties as a list or as one comma separated string, and only that list is sent. setProperty and setProperties behave exactly as they did on rc, and the refusal for a license key with no requested properties names setRequestedProperties. --- .../cloudrequestengine/Constants.java | 6 +- .../CloudRequestEngineBuilder.java | 46 ++++++++++--- .../CloudRequestEngineLicenseKeyTests.java | 67 ++++++++++++++----- 3 files changed, 93 insertions(+), 26 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java index 538c0111..e733ef10 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java @@ -38,9 +38,9 @@ public class Messages { "The Cloud Request Engine has a license key and no requested " + "properties. A license key names no properties of its own, so " + "the cloud service answers 400 to every request made with one " - + "that does not list the properties it wants. Use setProperty " - + "or setProperties to name the fully qualified properties " - + "wanted, for example \"device.ismobile\" and " + + "that does not list the properties it wants. Use " + + "setRequestedProperties to name the fully qualified " + + "properties wanted, for example \"device.ismobile\" and " + "\"device.iscrawler\"."; public static final String PropertiesNotCovered = 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 27808486..5b407e64 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 @@ -38,6 +38,7 @@ import org.slf4j.ILoggerFactory; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Locale; import java.util.function.Function; @@ -57,6 +58,7 @@ public class CloudRequestEngineBuilder extends private String evidenceKeysEndpoint = null; private String resourceKey = null; private String licenseKey = null; + private final List requestedProperties = new ArrayList<>(); private String cloudRequestOrigin = null; private int timeoutMillis = Constants.DEFAULT_TIMEOUT_MILLIS; @@ -80,11 +82,8 @@ public CloudRequestEngineBuilder( protected CloudRequestEngine newEngine(List properties) throws Exception { String trimmedResourceKey = trimmed(resourceKey); String trimmedLicenseKey = trimmed(licenseKey); - List requestedProperties = normalized(properties); - checkCredentials( - trimmedResourceKey, - trimmedLicenseKey, - requestedProperties); + List requested = normalized(requestedProperties); + checkCredentials(trimmedResourceKey, trimmedLicenseKey, requested); /* * Check if endPoint has been explicitly set via setEndpoint. @@ -113,7 +112,7 @@ protected CloudRequestEngine newEngine(List properties) throws Exception evidenceKeysEndpoint, timeoutMillis, cloudRequestOrigin, - requestedProperties); + requested); } private static String trimmed(String value) { @@ -247,8 +246,7 @@ public CloudRequestEngineBuilder setResourceKey(String resourceKey) { * The license key to query the endpoint with. *

* A license key used without a resource key needs the properties - * wanted, named with {@link #setProperty(String)} or - * {@link #setProperties(List)}, which are sent as values. Beside a + * wanted, named with {@link #setRequestedProperties(List)}. Beside a * resource key it adds the requested properties the license grants to * what the resource key carries. *

@@ -262,6 +260,38 @@ public CloudRequestEngineBuilder setLicenseKey(String licenseKey) { return this; } + /** + * The properties each request asks the cloud service for, as fully + * qualified names such as {@code device.ismobile}, sent as values. + * Required with a license key alone. Beside a resource key the cloud + * service adds them to the properties the resource key carries. Names + * are trimmed, lower-cased and de-duplicated. The properties set with + * {@link #setProperty(String)} and {@link #setProperties(List)} are not + * sent. + *

+ * By default no properties are requested + * @param properties the fully qualified property names + * @return this builder + */ + @DefaultValue("None - required with a license key alone") + public CloudRequestEngineBuilder setRequestedProperties( + List properties) { + requestedProperties.addAll(properties); + return this; + } + + /** + * As {@link #setRequestedProperties(List)}, taking the names as one + * comma separated string, as a configuration file gives them. + * @param properties comma separated fully qualified property names + * @return this builder + */ + @DefaultValue("None - required with a license key alone") + public CloudRequestEngineBuilder setRequestedProperties( + String properties) { + return setRequestedProperties(Arrays.asList(properties.split(","))); + } + /** * Timeout in seconds for the request to the endpoint. *

diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java index 09163a9e..82c23efe 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -256,14 +256,14 @@ public void Build_AllThree_Accepted() throws Exception { assertNotNull(build(builder() .setResourceKey(RESOURCE_KEY) .setLicenseKey(LICENSE_KEY) - .setProperties(PROPERTY_LIST))); + .setRequestedProperties(PROPERTY_LIST))); } @Test public void Build_LicenseKeyAndProperties_Accepted() throws Exception { assertNotNull(build(builder() .setLicenseKey(LICENSE_KEY) - .setProperties(PROPERTY_LIST))); + .setRequestedProperties(PROPERTY_LIST))); } @Test @@ -271,20 +271,20 @@ public void Build_LicenseKeyWithoutProperties_Refused() { assertRefused( builder().setLicenseKey(LICENSE_KEY), "answers 400", - "setProperty"); + "setRequestedProperties"); } @Test public void Build_ResourceKeyWithProperties_Accepted() throws Exception { assertNotNull(build(builder() .setResourceKey(RESOURCE_KEY) - .setProperty("device.hardwarevendor"))); + .setRequestedProperties("device.hardwarevendor"))); } @Test public void Build_NoCredential_Refused() { assertRefused(builder(), "401", "resource key"); - assertRefused(builder().setProperties(PROPERTY_LIST), "401"); + assertRefused(builder().setRequestedProperties(PROPERTY_LIST), "401"); } @Test @@ -295,12 +295,12 @@ public void Build_BlankValuesCountAsAbsent() throws Exception { assertRefused( builder() .setLicenseKey(LICENSE_KEY) - .setProperties(Arrays.asList(" ", "")), + .setRequestedProperties(Arrays.asList(" ", "")), "answers 400"); assertNotNull(build(builder() .setResourceKey(RESOURCE_KEY) .setLicenseKey(" ") - .setProperty(" "))); + .setRequestedProperties(" "))); } @Test @@ -318,7 +318,7 @@ public void Process_LicenseKeyAlone_SendsLicenseAndValues() throws Exception { CloudRequestEngine engine = build(builder() .setLicenseKey(LICENSE_KEY + "\n") - .setProperties( + .setRequestedProperties( Arrays.asList(" Device.IsMobile", "device.iscrawler "))); assertEquals("", process(engine)); @@ -367,7 +367,7 @@ public void Process_ResourceKeyAndProperties_SendsResourceAndValues() "\"hardwarevendornullreason\":\"Paid feature\"}}"; CloudRequestEngine engine = build(builder() .setResourceKey(RESOURCE_KEY) - .setProperty("Device.HardwareVendor")); + .setRequestedProperties("Device.HardwareVendor")); assertEquals("", process(engine)); @@ -384,7 +384,7 @@ public void Process_AllThree_SendsResourceLicenseAndValues() CloudRequestEngine engine = build(builder() .setResourceKey(RESOURCE_KEY) .setLicenseKey(LICENSE_KEY) - .setProperty("IP.RegisteredName")); + .setRequestedProperties("IP.RegisteredName")); cloud.dataResponse = "{\"device\":{\"ismobile\":true}," + "\"ip\":{\"registeredname\":\"Example\"}}"; @@ -399,6 +399,43 @@ public void Process_AllThree_SendsResourceLicenseAndValues() } } + @Test + public void Process_RequestedPropertiesAsText_Sent() throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties("Device.IsMobile, device.iscrawler ,")); + + assertEquals("", process(engine)); + + Map fields = dataRequest("/api/v4/json"); + assertEquals(PROPERTIES, fields.get("values")); + } + + /** + * Properties set with setProperty or setProperties are not sent, as + * before, so a configuration that lists them keeps the answer it had. A + * license key with only those properties is refused. + */ + @Test + public void Process_SetProperties_NotSent() throws Exception { + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setProperties(Arrays.asList("IsMobile", "DeviceId"))); + + assertEquals("", process(engine)); + + Map fields = dataRequest(RESOURCE_KEY + ".json"); + assertEquals(RESOURCE_KEY, fields.get("resource")); + assertFalse(fields.containsKey("values")); + verify(logger, never()).warn(anyString()); + + assertRefused( + builder() + .setLicenseKey(LICENSE_KEY) + .setProperty("device.ismobile"), + "answers 400"); + } + // Discovery. @Test @@ -406,7 +443,7 @@ public void Discovery_LicenseKeyAlone_PostsCredentialsInTheBody() throws Exception { CloudRequestEngine engine = build(builder() .setLicenseKey(LICENSE_KEY) - .setProperties(PROPERTY_LIST)); + .setRequestedProperties(PROPERTY_LIST)); Map products = engine.getPublicProperties(); @@ -433,7 +470,7 @@ public void Discovery_ResourceKeyAndProperties_GetsWithTheKeyInTheQuery() throws Exception { CloudRequestEngine engine = build(builder() .setResourceKey(RESOURCE_KEY) - .setProperty("device.hardwarevendor")); + .setRequestedProperties("device.hardwarevendor")); engine.getPublicProperties(); @@ -468,7 +505,7 @@ public void Process_FailedRequest_LicenseKeyInNoUrlLogOrMessage() throws Exception { CloudRequestEngine engine = build(builder() .setLicenseKey(LICENSE_KEY) - .setProperties(Arrays.asList( + .setRequestedProperties(Arrays.asList( "device.ismobile", "device.other"))); assertEquals("", process(engine)); @@ -496,7 +533,7 @@ public void Process_PropertyDropped_ReportedOnceAndRequestSucceeds() cloud.dataResponse = "{\"device\":{\"ismobile\":true}}"; CloudRequestEngine engine = build(builder() .setLicenseKey(LICENSE_KEY) - .setProperties( + .setRequestedProperties( Arrays.asList("Device.IsMobile", "Device.IsCrawler"))); assertEquals("", process(engine)); @@ -516,7 +553,7 @@ public void Process_PropertyPresentButNull_CountsAsAnswered() "\"iscrawler\":null,\"iscrawlernullreason\":\"No data\"}}"; CloudRequestEngine engine = build(builder() .setLicenseKey(LICENSE_KEY) - .setProperties(PROPERTY_LIST)); + .setRequestedProperties(PROPERTY_LIST)); assertEquals("", process(engine)); From 9b987e6aaa36cd2318d09cd1866034835355c328 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 09:51:26 +0000 Subject: [PATCH 04/10] Count only the requests a refused build sends in the license key tests --- .../flowelements/CloudRequestEngineLicenseKeyTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java index 82c23efe..db11760a 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -225,6 +225,7 @@ private Map dataRequest(String pathEnd) private void assertRefused( CloudRequestEngineBuilder builder, String... expectedInMessage) { + int sent = cloud.calls.size(); PipelineConfigurationException e = assertThrows( PipelineConfigurationException.class, () -> build(builder)); @@ -234,7 +235,7 @@ private void assertRefused( "Message should contain '" + expected + "' but was: " + e.getMessage()); } - assertTrue(cloud.calls.isEmpty(), "A refused build sends nothing."); + assertEquals(sent, cloud.calls.size(), "A refused build sends nothing."); } // The rows of the specification's table of accepted combinations. From 46de8b5b177552508a668098b0d51a464d0dc805 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 12:37:16 +0000 Subject: [PATCH 05/10] Test that requested property names are sent as given, with two report texts As in .NET, the names sent as values keep the letter case they were given in, and a name given twice in different cases is sent once as first given. With a license key, names the answer leaves out are reported as not covered by the keys, and with a resource key, requested properties and no license key, as not properties, as cloud-request-engine.md says. A null list is refused with the parameter's name. --- .../CloudRequestEngineLicenseKeyTests.java | 77 +++++++++++++++++-- 1 file changed, 69 insertions(+), 8 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java index db11760a..e468eb59 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -60,7 +60,6 @@ public class CloudRequestEngineLicenseKeyTests { private static final String BASE = "https://cloud.example/api/v4/"; private static final String LICENSE_KEY = "abcdefghijkl1234"; private static final String RESOURCE_KEY = "resourcekey"; - private static final String PROPERTIES = "device.ismobile,device.iscrawler"; private static final List PROPERTY_LIST = Arrays.asList("device.ismobile", "device.iscrawler"); @@ -326,7 +325,7 @@ public void Process_LicenseKeyAlone_SendsLicenseAndValues() Map fields = dataRequest("/api/v4/json"); assertEquals(LICENSE_KEY, fields.get("license")); - assertEquals(PROPERTIES, fields.get("values")); + assertEquals("Device.IsMobile,device.iscrawler", fields.get("values")); assertFalse(fields.containsKey("resource")); assertFalse(fields.containsKey("licence")); assertEquals("iPhone", fields.get("User-Agent")); @@ -374,7 +373,7 @@ public void Process_ResourceKeyAndProperties_SendsResourceAndValues() Map fields = dataRequest(RESOURCE_KEY + ".json"); assertEquals(RESOURCE_KEY, fields.get("resource")); - assertEquals("device.hardwarevendor", fields.get("values")); + assertEquals("Device.HardwareVendor", fields.get("values")); assertFalse(fields.containsKey("license")); verify(logger, never()).warn(anyString()); } @@ -394,7 +393,7 @@ public void Process_AllThree_SendsResourceLicenseAndValues() Map fields = dataRequest(RESOURCE_KEY + ".json"); assertEquals(RESOURCE_KEY, fields.get("resource")); assertEquals(LICENSE_KEY, fields.get("license")); - assertEquals("ip.registeredname", fields.get("values")); + assertEquals("IP.RegisteredName", fields.get("values")); for (Call call : cloud.calls) { assertFalse(call.url.toString().contains(LICENSE_KEY)); } @@ -409,7 +408,7 @@ public void Process_RequestedPropertiesAsText_Sent() throws Exception { assertEquals("", process(engine)); Map fields = dataRequest("/api/v4/json"); - assertEquals(PROPERTIES, fields.get("values")); + assertEquals("Device.IsMobile,device.iscrawler", fields.get("values")); } /** @@ -541,12 +540,74 @@ public void Process_PropertyDropped_ReportedOnceAndRequestSucceeds() assertEquals("", process(engine)); verify(logger, times(1)).warn(argThat( - (String m) -> m.contains("device.iscrawler") && - m.contains("not covered") && - !m.contains("device.ismobile"))); + (String m) -> m.contains("Device.IsCrawler") && + m.contains("not covered by the keys") && + !m.contains("Device.IsMobile"))); verify(logger, times(1)).warn(anyString()); } + /** + * With a resource key, requested properties and no license key, a + * property no licence grants is answered as null, so a name the + * answer leaves out is not a property. The report says so, and names + * no license key, because none was given. + */ + @Test + public void Process_ResourceKeyNameNotAnswered_ReportedAsNotAProperty() + throws Exception { + cloud.dataResponse = "{\"device\":{\"ismobile\":true}}"; + CloudRequestEngine engine = build(builder() + .setResourceKey(RESOURCE_KEY) + .setRequestedProperties( + Arrays.asList("Device.IsMobile", "device.notaproperty"))); + + assertEquals("", process(engine)); + assertEquals("", process(engine)); + + verify(logger, times(1)).warn(argThat( + (String m) -> m.contains("device.notaproperty") && + m.contains("not properties") && + !m.contains("license") && + !m.contains("not covered"))); + verify(logger, times(1)).warn(anyString()); + } + + /** + * A name given twice in different letter cases is sent once, as it + * was first given, because the cloud service reads property names + * without regard to case. + */ + @Test + public void Process_NameGivenTwice_SentOnceAsFirstGiven() + throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties(Arrays.asList( + "Device.IsMobile", "device.ismobile", "device.iscrawler"))); + + assertEquals("", process(engine)); + + assertEquals("Device.IsMobile,device.iscrawler", + dataRequest("/api/v4/json").get("values")); + } + + /** + * A null list of requested properties is refused with the name of + * the parameter, as .NET refuses it, rather than failing inside the + * list. + */ + @Test + public void SetRequestedProperties_Null_Refused() { + IllegalArgumentException list = assertThrows( + IllegalArgumentException.class, + () -> builder().setRequestedProperties((List) null)); + assertTrue(list.getMessage().contains("properties")); + IllegalArgumentException text = assertThrows( + IllegalArgumentException.class, + () -> builder().setRequestedProperties((String) null)); + assertTrue(text.getMessage().contains("properties")); + } + @Test public void Process_PropertyPresentButNull_CountsAsAnswered() throws Exception { From fd7242afaad71277b1a562ff50116c78cf5da532 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 12:38:26 +0000 Subject: [PATCH 06/10] Send requested property names as given, and say when one is not a property The cloud service reads property names without regard to letter case, so the names sent as values keep the case they were given in, and a repeat in another case is dropped, as .NET now does. A null list of requested properties is refused with an IllegalArgumentException naming the parameter, rather than a NullPointerException from inside the list. With a resource key, requested properties and no license key, a property the resource key does not carry is answered as null with its reason, so a name the answer leaves out is not a property. The warning for that case says so and names no license key, as cloud-request-engine.md asks. --- .../cloudrequestengine/Constants.java | 8 ++++++ .../CloudRequestEngineBuilder.java | 26 ++++++++++++++----- .../CloudRequestEngineDefault.java | 10 ++++--- 3 files changed, 34 insertions(+), 10 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java index a802ae82..11e531de 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java @@ -49,6 +49,14 @@ public class Messages { + "they will be missing from every result. The other requested " + "properties came back and are usable."; + public static final String RequestedNamesNotProperties = + "The cloud service did not return the requested properties %s " + + "in its answer. With a resource key and no license key every " + + "property is answered, with a reason when it has no value, so " + + "these names are not properties. Check the spelling of each " + + "one. The other requested properties came back and are " + + "usable."; + public static final String EvidenceConflict = "'%s:%s' evidence conflicts with %s"; public static final String MessageNoDataInResponse = 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 e52b5fff..dd62e06b 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 @@ -41,7 +41,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; -import java.util.Locale; +import java.util.Set; +import java.util.TreeSet; import java.util.function.Function; /** @@ -160,15 +161,17 @@ private static String trimmed(String value) { } /** - * The property names to send as values, trimmed and lower-cased, as the - * cloud service answers in lower case, with blank names and repeats - * left out. + * The property names to send as values, trimmed and sent as given, + * with blank names and repeats left out. A repeat differs only in + * letter case, as the cloud service reads names without regard to + * case, and the first spelling given is kept. */ private static List normalized(List names) { List result = new ArrayList<>(); + Set seen = new TreeSet<>(String.CASE_INSENSITIVE_ORDER); for (String name : names) { - String normal = trimmed(name).toLowerCase(Locale.ROOT); - if (normal.isEmpty() == false && result.contains(normal) == false) { + String normal = trimmed(name); + if (normal.isEmpty() == false && seen.add(normal)) { result.add(normal); } } @@ -305,7 +308,8 @@ public CloudRequestEngineBuilder setLicenseKey(String licenseKey) { * qualified names such as {@code device.ismobile}, sent as values. * Required with a license key alone. Beside a resource key the cloud * service adds them to the properties the resource key carries. Names - * are trimmed, lower-cased and de-duplicated. The properties set with + * are trimmed and sent as given, and a name given again in another + * letter case is sent once. The properties set with * {@link #setProperty(String)} and {@link #setProperties(List)} are not * sent. *

@@ -316,6 +320,10 @@ public CloudRequestEngineBuilder setLicenseKey(String licenseKey) { @DefaultValue("None - required with a license key alone") public CloudRequestEngineBuilder setRequestedProperties( List properties) { + if (properties == null) { + throw new IllegalArgumentException( + "properties must not be null"); + } requestedProperties.addAll(properties); return this; } @@ -329,6 +337,10 @@ public CloudRequestEngineBuilder setRequestedProperties( @DefaultValue("None - required with a license key alone") public CloudRequestEngineBuilder setRequestedProperties( String properties) { + if (properties == null) { + throw new IllegalArgumentException( + "properties must not be null"); + } return setRequestedProperties(Arrays.asList(properties.split(","))); } diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java index e7de833e..627bb42c 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java @@ -212,7 +212,7 @@ public CloudRequestEngineDefault( /** * Constructor. - * @param requestedProperties the lower-case fully qualified names of the + * @param requestedProperties the fully qualified names of the * properties to ask for, sent as {@code values} * when the list is not empty */ @@ -478,7 +478,10 @@ protected void processEngine(FlowData data, CloudRequestData aspectData) throws * property it does not answer without naming it, and lower-cases the * names it answers with. A property that is present but null counts as * answered, as it carries a null reason beside it. The request is never - * failed, because the properties that did come back are usable. + * failed, because the properties that did come back are usable. With no + * license key, a property the resource key does not carry is answered + * as null, so a name left out is not a property, and the message says + * so. */ private void reportPropertiesNotCovered(String response) { if (requestedProperties.isEmpty() || @@ -503,7 +506,8 @@ private void reportPropertiesNotCovered(String response) { } if (notCovered.isEmpty() == false) { logger.warn(String.format( - PropertiesNotCovered, + hasLicenseKey() ? PropertiesNotCovered : + RequestedNamesNotProperties, stringJoin(notCovered, ", "))); } } From e99d627665d0f1ae194301128e4b2c64a524455b Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 12:45:18 +0000 Subject: [PATCH 07/10] Word the warning for a name that is not a property without the license key The warning for a resource key with requested properties and no license key names no license key, because none was given. --- .../fiftyone/pipeline/cloudrequestengine/Constants.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java index 11e531de..eebb82eb 100644 --- a/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java +++ b/pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/Constants.java @@ -51,11 +51,10 @@ public class Messages { public static final String RequestedNamesNotProperties = "The cloud service did not return the requested properties %s " - + "in its answer. With a resource key and no license key every " - + "property is answered, with a reason when it has no value, so " - + "these names are not properties. Check the spelling of each " - + "one. The other requested properties came back and are " - + "usable."; + + "in its answer. Every property is answered, with a reason " + + "when it has no value, so these names are not properties. " + + "Check the spelling of each one. The other requested " + + "properties came back and are usable."; public static final String EvidenceConflict = "'%s:%s' evidence conflicts with %s"; From f45eceda1f108a589cea9b497b735226c235581a Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 12:56:24 +0000 Subject: [PATCH 08/10] Record the license key tests' fake cloud calls in a synchronized list Discovery on rc fetches the evidence keys on another thread while it fetches the accessible properties, so the fake cloud records calls from two threads. A plain ArrayList could lose a call or hold a null, which failed the resource key discovery and data request tests depending on the order the tests ran in. rc's own discovery tests use a synchronized list for the same reason. --- .../flowelements/CloudRequestEngineLicenseKeyTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java index e468eb59..ab2cbbbf 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -78,7 +78,10 @@ private static class Call { /** Records every request and answers from fixed responses. */ private static class FakeCloud implements HttpClient { - final List calls = new ArrayList<>(); + // Discovery makes its two requests at once, so calls are + // recorded from two threads. + final List calls = + Collections.synchronizedList(new ArrayList<>()); String dataResponse = "{\"device\":{\"ismobile\":true," + "\"iscrawler\":false}}"; int dataStatus = 200; From 72faa2d5874df92355a1e399183bf9ef02edf62c Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 18:33:08 +0000 Subject: [PATCH 09/10] Answer the test cloud's POSTs by URL, as its GETs are rc's Cloud Request Engine tests build engines with a license key, and a license key makes the discovery requests POSTs, which the shared mock answered with the data response. The mock now answers a POST to the accessible properties or evidence keys endpoint as it answers a GET, and the three tests that count the data request count only that request. --- .../flowelements/CloudRequestEngineTests.java | 15 ++++++++++++--- .../CloudRequestEngineTestsBase.java | 16 +++++++++++++++- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java index 994408d1..d49c9c5d 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java @@ -623,7 +623,10 @@ public void process_PostsWholeEvidenceName() throws Exception { } ArgumentCaptor body = ArgumentCaptor.forClass(byte[].class); - verify(httpClient, times(1)).postData(any(), any(), body.capture()); + verify(httpClient, times(1)).postData( + argThat(c -> c.getURL().getPath().endsWith("properties") == false), + any(), + body.capture()); List items = Arrays.asList( new String(body.getValue(), StandardCharsets.UTF_8).split("&")); assertTrue(items.contains("id.usage=non-marketing"), @@ -849,7 +852,10 @@ public void process_RepeatedRequestHitsTheCache() throws Exception { engine.getEvidenceKeyFilter().include("header.sec-ch-ua-platform"), "The engine advertises a key the cloud service does not list."); ArgumentCaptor body = ArgumentCaptor.forClass(byte[].class); - verify(httpClient, times(1)).postData(any(), any(), body.capture()); + verify(httpClient, times(1)).postData( + argThat(c -> c.getURL().getPath().endsWith("properties") == false), + any(), + body.capture()); List items = Arrays.asList( new String(body.getValue(), StandardCharsets.UTF_8).split("&")); assertTrue(items.contains("sec-ch-ua-platform=Windows"), @@ -891,7 +897,10 @@ private List postedItems( } ArgumentCaptor body = ArgumentCaptor.forClass(byte[].class); - verify(httpClient, times(1)).postData(any(), any(), body.capture()); + verify(httpClient, times(1)).postData( + argThat(c -> c.getURL().getPath().endsWith("properties") == false), + any(), + body.capture()); return Arrays.asList( new String(body.getValue(), StandardCharsets.UTF_8).split("&")); } diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTestsBase.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTestsBase.java index 98d85411..dfbd106b 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTestsBase.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTestsBase.java @@ -122,7 +122,21 @@ else if (url.getPath().endsWith("evidencekeys")) { .when(httpClient) .getResponseString(any(HttpURLConnection.class), anyMap()); - doReturn(jsonResponse) + // A license key makes the discovery requests POSTs, so a POST is + // answered by its URL in the same way. + doAnswer(new Answer() { + @Override + public Object answer(InvocationOnMock invocationOnMock) throws Throwable { + URL url = ((HttpURLConnection)invocationOnMock.getArgument(0)).getURL(); + if (url.getPath().endsWith("properties")) { + return accessiblePropertiesResponse; + } + if (url.getPath().endsWith("evidencekeys")) { + return evidenceKeysResponse; + } + return jsonResponse; + } + }) .when(httpClient) .postData( any(HttpURLConnection.class), From 4656994f7a775dcf83a6fdb63ec339c8500c8f19 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Wed, 11 Nov 2026 18:40:53 +0000 Subject: [PATCH 10/10] Record the license key tests' fake connections in a concurrent map The fake cloud looked each request's URL up in a HashMap that both discovery threads wrote to, so a lost entry left a call with no URL and failed Process_ResourceKeyAndLicenseKey_SendsBoth in one run. --- .../flowelements/CloudRequestEngineLicenseKeyTests.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java index ab2cbbbf..e05e9bc0 100644 --- a/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -46,6 +46,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.anyString; @@ -78,8 +79,8 @@ private static class Call { /** Records every request and answers from fixed responses. */ private static class FakeCloud implements HttpClient { - // Discovery makes its two requests at once, so calls are - // recorded from two threads. + // Discovery makes its two requests at once, so connections and + // calls are recorded from two threads. final List calls = Collections.synchronizedList(new ArrayList<>()); String dataResponse = "{\"device\":{\"ismobile\":true," + @@ -90,7 +91,8 @@ private static class FakeCloud implements HttpClient { "[{\"Name\":\"IsMobile\",\"Type\":\"Boolean\"," + "\"Category\":\"Device\"}]}}}"; String evidenceKeysResponse = "[\"query.User-Agent\"]"; - private final Map urls = new HashMap<>(); + private final Map urls = + new ConcurrentHashMap<>(); @Override public HttpURLConnection connect(URL url) {