Send every Evidence value but the reserved names in the cloud request

pipeline/pipeline.cloudrequestengine/src/main/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineDefault.java

@@ -470,7 +470,11 @@ private byte[] getContent(FlowData data) throws UnsupportedEncodingException {
     /**
      * The Evidence to send, keyed by the name the cloud service expects.
      * Only Evidence that {@link #isSent(String)} allows is included, and
-     * where two prefixes give the same name the precedence order decides.
+     * where two prefixes give the same name the precedence order decides,
+     * query first, then header, then cookie, then any other prefix.
+     * Evidence with no prefix ranks with the other prefixes, unless the
+     * whole key is query, header or cookie, as {@link #keyPrefix(String)}
+     * gives the whole key as its prefix.
      * @param flowData the FlowData
      * @return the form fields to send
      */
@@ -497,22 +501,43 @@ Map<String, Object> getFormData(FlowData flowData) {
     }
 
     /**
-     * Whether Evidence is sent to the cloud service. Only keys the service
-     * accepts are sent, never one that strips to a name in
-     * {@link #ENGINE_FIELDS}, compared without regard to case, and never
-     * the web request.
+     * Whether Evidence is sent to the cloud service. Every key is sent,
+     * including keys the service does not list as accepted and keys with no
+     * prefix, apart from the web request and a key whose
+     * {@link #keySuffix(String)} is in {@link #ENGINE_FIELDS}, compared
+     * without regard to case.
      * @param key the Evidence key
      * @return true if the Evidence is sent
      */
     private boolean isSent(String key) {
-        if (HTTP_REQUEST_KEY.equalsIgnoreCase(key) ||
-            evidenceKeyFilter.include(key) == false) {
-            return false;
-        }
-        String[] parts = key.split(
-            Pattern.quote(Constants.EVIDENCE_SEPERATOR), 2);
-        return parts.length == 2 &&
-            ENGINE_FIELDS.contains(parts[1].toLowerCase(Locale.ROOT)) == false;
+        return HTTP_REQUEST_KEY.equalsIgnoreCase(key) == false &&
+            ENGINE_FIELDS.contains(
+                keySuffix(key).toLowerCase(Locale.ROOT)) == false;
+    }
+
+    /**
+     * The prefix of an Evidence key, being the part before its first
+     * separator, or the whole key where it has none.
+     * @param key the Evidence key
+     * @return the prefix
+     */
+    private static String keyPrefix(String key) {
+        int separator = key.indexOf(Constants.EVIDENCE_SEPERATOR);
+        return separator >= 0 ? key.substring(0, separator) : key;
+    }
+
+    /**
+     * The name an Evidence key is sent under, being the part after its
+     * first separator, or the whole key where it has none. The name can
+     * hold a dot itself, as id.usage does in query.id.usage.
+     * @param key the Evidence key
+     * @return the name to send
+     */
+    private static String keySuffix(String key) {
+        int separator = key.indexOf(Constants.EVIDENCE_SEPERATOR);
+        return separator >= 0 ?
+            key.substring(separator + Constants.EVIDENCE_SEPERATOR.length()) :
+            key;
     }
 
     /**
@@ -531,13 +556,8 @@ private void addFormData(
         Collections.sort(evidenceKeys, Collections.reverseOrder());
 
         for (String evidenceKey : evidenceKeys) {
-            // Split at the first separator only. Everything after the prefix
-            // is the name the cloud service expects, and that name can hold
-            // a dot itself, as id.usage does in query.id.usage.
-            String[] evidenceKeyParts = evidenceKey.split(
-                Pattern.quote(Constants.EVIDENCE_SEPERATOR), 2);
-            String prefix = evidenceKeyParts[0];
-            String suffix = evidenceKeyParts[1];
+            String prefix = keyPrefix(evidenceKey);
+            String suffix = keySuffix(evidenceKey);
 
             // Check and add the evidence to the query parameters.
             if (formData.containsKey(suffix) == false) {
@@ -549,7 +569,7 @@ private void addFormData(
                 // warning, if the evidence prefix is not query. Otherwise a
                 // warning is not needed as query evidence is expected
                 // to overwrite any existing evidence with the same suffix.
-                if (prefix.equals("query") == false) {
+                if (prefix.equalsIgnoreCase("query") == false) {
                     Map<String, Object> conflicts = new HashMap<>();
                     for (String key : allEvidence.keySet()) {
                         if (key.equals(evidenceKey) == false && key.contains(suffix)) {
@@ -607,13 +627,15 @@ Map<String, Object> getSelectedEvidence(Map<String, Object> evidence, String typ
     }
 
     /**
-     * Check that the key of a KeyValuePair has the given prefix.
+     * Check that an Evidence key has the given prefix, being the part
+     * before its first separator, or the whole key where it has none,
+     * compared without regard to case.
      * @param itemKey key to check
      * @param prefix the prefix to check for
      * @return true if the key has the prefix
      */
     private boolean hasKeyPrefix(String itemKey, String prefix) {
-        return itemKey.startsWith(prefix + ".");
+        return keyPrefix(itemKey).equalsIgnoreCase(prefix);
     }
 
     @Override

pipeline/pipeline.cloudrequestengine/src/test/java/fiftyone/pipeline/cloudrequestengine/flowelements/CloudRequestEngineTests.java

@@ -631,31 +631,120 @@ public void process_PostsWholeEvidenceName() throws Exception {
     }
 
     /**
-     * Only the Evidence whose keys the cloud service accepts is sent, with
-     * the keys compared without regard to case.
+     * Every Evidence value is sent, including Evidence the cloud service
+     * does not list as accepted, apart from the reserved names under any
+     * prefix.
      * See the Processing section of the cloud request engine specification.
      */
     @Test
-    public void process_SendsOnlyAcceptedEvidence() throws Exception {
-        acceptEvidence(Arrays.asList(
-            "query.User-Agent", "header.Accept-Language"));
+    public void process_SendsEveryEvidenceValueButTheReservedNames()
+        throws Exception {
+        acceptEvidence(Arrays.asList("query.User-Agent"));
         Map<String, Object> evidence = new HashMap<>();
         evidence.put("query.User-Agent", "iPhone");
         evidence.put("header.accept-language", "en-GB");
         evidence.put("header.X-Other", "other");
         evidence.put("cookie.session", "abc");
         evidence.put("server.client-ip", "1.2.3.4");
+        evidence.put("other.VALUES", "ismobile");
+        evidence.put("server.licence", "evidence-licence");
+
+        List<String> items = postedItems(null, evidence);
+
+        assertEquals(
+            new HashSet<>(Arrays.asList(
+                "resource=resourcekey",
+                "User-Agent=iPhone",
+                "accept-language=en-GB",
+                "X-Other=other",
+                "session=abc",
+                "client-ip=1.2.3.4")),
+            new HashSet<>(items),
+            "The request did not carry every Evidence value apart from " +
+                "the reserved names: " + items);
+    }
+
+    /**
+     * Evidence whose key has no prefix is sent under its whole key, unless
+     * the key is a reserved name in any letter case. Keys only usually
+     * carry a prefix, as the Overview of the Evidence specification says.
+     */
+    @Test
+    public void process_SendsEvidenceWithNoPrefixUnderItsWholeKey()
+        throws Exception {
+        acceptEvidence(Arrays.asList("query.User-Agent"));
+        Map<String, Object> evidence = new HashMap<>();
+        evidence.put("query.User-Agent", "iPhone");
+        evidence.put("bare", "value");
+        evidence.put("Values", "ismobile");
+        evidence.put("license", "evidence-license");
+        evidence.put("RESOURCE", "evidence-resource");
+        evidence.put("licence", "evidence-licence");
 
         List<String> items = postedItems(null, evidence);
 
         assertEquals(
             new HashSet<>(Arrays.asList(
                 "resource=resourcekey",
                 "User-Agent=iPhone",
-                "accept-language=en-GB")),
+                "bare=value")),
             new HashSet<>(items),
-            "The request carried Evidence the cloud service does not " +
-                "accept: " + items);
+            "The request did not carry the Evidence with no prefix, or " +
+                "carried a reserved name: " + items);
+    }
+
+    /**
+     * Prefixes are compared without regard to case, so query Evidence
+     * takes precedence over header Evidence of the same name, with no
+     * warning, whatever the case of its prefix. Keys are case-insensitive,
+     * as the Overview of the Evidence specification says.
+     */
+    @Test
+    public void process_ComparesPrefixesWithoutRegardToCase()
+        throws Exception {
+        loggerFactory.loggers.clear();
+        acceptEvidence(Arrays.asList("query.User-Agent"));
+        Map<String, Object> evidence = new HashMap<>();
+        evidence.put("Query.User-Agent", "query-value");
+        evidence.put("header.User-Agent", "header-value");
+
+        List<String> items = postedItems(null, evidence);
+
+        assertEquals(
+            new HashSet<>(Arrays.asList(
+                "resource=resourcekey",
+                "User-Agent=query-value")),
+            new HashSet<>(items),
+            "The query Evidence did not take precedence: " + items);
+        loggerFactory.assertMaxWarnings(0);
+    }
+
+    /**
+     * Evidence with no prefix ranks with Evidence of any prefix other than
+     * query, header and cookie, so it has the lowest precedence, unless the
+     * whole key is one of those prefixes, which puts it with that prefix's
+     * Evidence. The precedence order of the Evidence specification orders
+     * prefixes only, so this follows .NET.
+     */
+    @Test
+    public void process_RanksEvidenceWithNoPrefix() throws Exception {
+        acceptEvidence(Arrays.asList("query.User-Agent"));
+        Map<String, Object> evidence = new HashMap<>();
+        evidence.put("X-Thing", "bare");
+        evidence.put("header.X-Thing", "header");
+        evidence.put("query", "bare-query");
+        evidence.put("header.query", "header-query");
+
+        List<String> items = postedItems(null, evidence);
+
+        assertEquals(
+            new HashSet<>(Arrays.asList(
+                "resource=resourcekey",
+                "X-Thing=header",
+                "query=bare-query")),
+            new HashSet<>(items),
+            "Evidence with no prefix was not ranked as .NET ranks it: " +
+                items);
     }
 
     /**
@@ -727,6 +816,47 @@ public void process_LeavesOutEvidenceNamedLicence(
         assertEquals(expected, items);
     }
 
+    /**
+     * A cache key is built from the Evidence the engine advertises, which is
+     * still the keys the cloud service lists, so a second request that
+     * differs only in Evidence the service does not list finds the entry
+     * the first one wrote, and one request is posted. That request carries
+     * the Evidence the service does not list all the same.
+     * See the Generation of keys section of the caching specification.
+     */
+    @Test
+    public void process_RepeatedRequestHitsTheCache() throws Exception {
+        acceptEvidence(Arrays.asList("query.User-Agent"));
+        configureMockedClient();
+        CloudRequestEngine engine =
+            new CloudRequestEngineBuilder(loggerFactory, httpClient)
+                .setResourceKey("resourcekey")
+                .setCacheSize(10)
+                .build();
+
+        try (Pipeline pipeline = new PipelineBuilder(loggerFactory)
+                .addFlowElement(engine).build()) {
+            for (String platform : Arrays.asList("Windows", "macOS")) {
+                try (FlowData data = pipeline.createFlowData()) {
+                    data.addEvidence("query.User-Agent", "iPhone");
+                    data.addEvidence("header.sec-ch-ua-platform", platform);
+                    data.process();
+                }
+            }
+        }
+
+        assertFalse(
+            engine.getEvidenceKeyFilter().include("header.sec-ch-ua-platform"),
+            "The engine advertises a key the cloud service does not list.");
+        ArgumentCaptor<byte[]> body = ArgumentCaptor.forClass(byte[].class);
+        verify(httpClient, times(1)).postData(any(), any(), body.capture());
+        List<String> items = Arrays.asList(
+            new String(body.getValue(), StandardCharsets.UTF_8).split("&"));
+        assertTrue(items.contains("sec-ch-ua-platform=Windows"),
+            "The request did not carry the Evidence the cloud service does " +
+                "not list: " + items);
+    }
+
     /**
      * Answer the evidencekeys request with the given keys.
      * @param keys the Evidence keys the cloud service accepts