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 e957273e..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 @@ -27,6 +27,35 @@ 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 " + + "setRequestedProperties 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 RequestedNamesNotProperties = + "The cloud service did not return the requested properties %s " + + "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"; 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 b0d420a0..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 @@ -25,6 +25,7 @@ import fiftyone.pipeline.annotations.DefaultValue; import fiftyone.pipeline.annotations.ElementBuilder; +import fiftyone.pipeline.cloudrequestengine.Constants.Messages; import fiftyone.pipeline.cloudrequestengine.CloudDiscoveryState; import fiftyone.pipeline.cloudrequestengine.data.CloudRequestData; import fiftyone.pipeline.core.data.FlowData; @@ -37,7 +38,11 @@ import fiftyone.pipeline.engines.services.HttpClientDefault; import org.slf4j.ILoggerFactory; +import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.Set; +import java.util.TreeSet; import java.util.function.Function; /** @@ -55,6 +60,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; private CloudDiscoveryState discoveryState = null; @@ -78,12 +84,10 @@ 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 requested = normalized(requestedProperties); + checkCredentials(trimmedResourceKey, trimmedLicenseKey, requested); /* * Check if endPoint has been explicitly set via setEndpoint. @@ -106,12 +110,13 @@ protected CloudRequestEngine newEngine(List properties) throws Exception new CloudRequestDataFactory(loggerFactory), httpClient, dataEndpoint, - resourceKey, - licenseKey, + trimmedResourceKey, + trimmedLicenseKey, propertiesEndpoint, evidenceKeysEndpoint, - timeoutMillis, - cloudRequestOrigin); + timeoutMillis, + cloudRequestOrigin, + requested); if (discoveryState != null) { engine.useDiscoveryState(discoveryState); } @@ -150,6 +155,51 @@ public CloudDiscoveryState getDiscoveryState() { CloudRequestEngineDefault engine = built; return engine == null ? null : engine.getDiscoveryState(); } + + private static String trimmed(String value) { + return value == null ? "" : value.trim(); + } + + /** + * 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); + if (normal.isEmpty() == false && seen.add(normal)) { + 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) { return System.getenv(name); @@ -173,7 +223,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"); @@ -182,7 +233,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 */ @@ -223,11 +275,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; @@ -236,6 +288,11 @@ 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 #setRequestedProperties(List)}. 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 @@ -246,6 +303,47 @@ 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 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. + *

+ * 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) { + if (properties == null) { + throw new IllegalArgumentException( + "properties must not be null"); + } + 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) { + if (properties == null) { + throw new IllegalArgumentException( + "properties must not be null"); + } + return setRequestedProperties(Arrays.asList(properties.split(","))); + } + /** * Timeout in seconds for each request to the cloud service, the two * discovery requests when the engine is built and each data request. 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 2e257108..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 @@ -53,6 +53,7 @@ import java.net.URLEncoder; import java.nio.charset.StandardCharsets; import java.util.*; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.regex.Pattern; @@ -97,6 +98,9 @@ public class CloudRequestEngineDefault private String evidenceKeysEndpoint; private String cloudRequestOrigin; private Integer timeoutMillis; + private final List requestedProperties; + private final AtomicBoolean requestedPropertiesChecked = + new AtomicBoolean(false); /** * The lowest HTTP status of a client error. A discovery request answered @@ -181,7 +185,6 @@ public CloudRequestEngineDefault( cloudRequestOrigin); } - @SuppressWarnings("this-escape") public CloudRequestEngineDefault( Logger logger, ElementDataFactory aspectDataFactory, @@ -193,7 +196,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 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; @@ -431,6 +469,61 @@ 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. 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() || + 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( + hasLicenseKey() ? PropertiesNotCovered : + RequestedNamesNotProperties, + 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)); } /** @@ -450,9 +543,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])); @@ -467,6 +561,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; + } + /** * The Evidence to send, keyed by the name the cloud service expects. * Only Evidence that {@link #isSent(String)} allows is included, and @@ -700,10 +814,26 @@ private void fetchPublicProperties() throws CloudRequestException, AggregateExce Map headers = new HashMap<>(); setCommonHeaders(headers); - HttpURLConnection connection = httpClient.connect(toUrl(propertiesEndpoint.trim() + (resourceKey != null ? "?Resource=" + resourceKey : ""))); - setTimeouts(connection); - 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())); + setTimeouts(connection); + jsonResult = httpClient.postData(connection, headers, content); + validateResponse(jsonResult, connection); + } else { + HttpURLConnection connection = httpClient.connect(toUrl( + propertiesEndpoint.trim() + "?Resource=" + resourceKey)); + setTimeouts(connection); + jsonResult = httpClient.getResponseString(connection, headers); + validateResponse(jsonResult, connection); + } if (jsonResult.isEmpty() == false) { setPublicProperties(jsonResult); 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..e05e9bc0 --- /dev/null +++ b/pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineLicenseKeyTests.java @@ -0,0 +1,640 @@ +/* ********************************************************************* + * 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 java.util.concurrent.ConcurrentHashMap; + +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 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 { + // 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," + + "\"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 ConcurrentHashMap<>(); + + @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) { + int sent = cloud.calls.size(); + PipelineConfigurationException e = assertThrows( + PipelineConfigurationException.class, + () -> build(builder)); + for (String expected : expectedInMessage) { + assertTrue( + e.getMessage().contains(expected), + "Message should contain '" + expected + "' but was: " + + e.getMessage()); + } + assertEquals(sent, cloud.calls.size(), "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) + .setRequestedProperties(PROPERTY_LIST))); + } + + @Test + public void Build_LicenseKeyAndProperties_Accepted() throws Exception { + assertNotNull(build(builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties(PROPERTY_LIST))); + } + + @Test + public void Build_LicenseKeyWithoutProperties_Refused() { + assertRefused( + builder().setLicenseKey(LICENSE_KEY), + "answers 400", + "setRequestedProperties"); + } + + @Test + public void Build_ResourceKeyWithProperties_Accepted() throws Exception { + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setRequestedProperties("device.hardwarevendor"))); + } + + @Test + public void Build_NoCredential_Refused() { + assertRefused(builder(), "401", "resource key"); + assertRefused(builder().setRequestedProperties(PROPERTY_LIST), "401"); + } + + @Test + public void Build_BlankValuesCountAsAbsent() throws Exception { + assertRefused( + builder().setResourceKey(" ").setLicenseKey("\n"), + "401"); + assertRefused( + builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties(Arrays.asList(" ", "")), + "answers 400"); + assertNotNull(build(builder() + .setResourceKey(RESOURCE_KEY) + .setLicenseKey(" ") + .setRequestedProperties(" "))); + } + + @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") + .setRequestedProperties( + Arrays.asList(" Device.IsMobile", "device.iscrawler "))); + + assertEquals("", process(engine)); + + Map fields = dataRequest("/api/v4/json"); + assertEquals(LICENSE_KEY, fields.get("license")); + assertEquals("Device.IsMobile,device.iscrawler", 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) + .setRequestedProperties("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) + .setRequestedProperties("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)); + } + } + + @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("Device.IsMobile,device.iscrawler", 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 + public void Discovery_LicenseKeyAlone_PostsCredentialsInTheBody() + throws Exception { + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties(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) + .setRequestedProperties("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) + .setRequestedProperties(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) + .setRequestedProperties( + 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 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 { + cloud.dataResponse = "{\"device\":{\"ismobile\":true," + + "\"iscrawler\":null,\"iscrawlernullreason\":\"No data\"}}"; + CloudRequestEngine engine = build(builder() + .setLicenseKey(LICENSE_KEY) + .setRequestedProperties(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()); + } +} 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),