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 1ca5d11c..2e257108 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 @@ -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 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 conflicts = new HashMap<>(); for (String key : allEvidence.keySet()) { if (key.equals(evidenceKey) == false && key.contains(suffix)) { @@ -607,13 +627,15 @@ Map getSelectedEvidence(Map 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 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 9e9f3a00..994408d1 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 @@ -631,20 +631,55 @@ 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 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 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 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 items = postedItems(null, evidence); @@ -652,10 +687,64 @@ public void process_SendsOnlyAcceptedEvidence() throws Exception { 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 evidence = new HashMap<>(); + evidence.put("Query.User-Agent", "query-value"); + evidence.put("header.User-Agent", "header-value"); + + List 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 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 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 body = ArgumentCaptor.forClass(byte[].class); + verify(httpClient, times(1)).postData(any(), any(), body.capture()); + List 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