diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py index 13b7f215..945b8a76 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/cloudrequestengine.py @@ -30,6 +30,7 @@ import requests from fiftyone_pipeline_core.basiclist_evidence_keyfilter import BasicListEvidenceKeyFilter +from fiftyone_pipeline_core.evidence import HTTP_REQUEST_KEY from fiftyone_pipeline_core.keymask import mask_licence_key from fiftyone_pipeline_core.keymask import mask_licence_keys_in_text from fiftyone_pipeline_engines.engine import Engine @@ -504,14 +505,19 @@ def report_properties_not_covered(self, result): def get_content(self, flowData): """! - Generate the Content to send in the POST request. The evidence keys - e.g. 'query.' and 'header.' have an order of precedence. These are - added to the evidence in reverse order, if there is conflict then - the queryData value is overwritten. - - 'query.' evidence should take precedence over all other evidence. - If there are evidence keys other than 'query.' that conflict then - this is unexpected so a warning will be logged. + Generate the Content to send in the POST request. One name under + several prefixes is sent once, with the value of the prefix that + evidence.md ranks highest, being query, header, cookie, server, + fiftyone and location, then any other prefix alphabetically. The + groups are added lowest precedence first, so a later value replaces + an earlier one, and a replacement by a key other than a query key + is logged as a warning. + + The engine sends every evidence value in the flow data, including + the values no element in the pipeline accepts, because what the + cloud service uses changes over time and knowing about the other + evidence is important to continued optimization. The reserved names + and the web request view are left out. @param: flowData: FlowData @return: Evidence Dictionary @@ -519,8 +525,8 @@ def get_content(self, flowData): queryData = {} - evidence = flowData.evidence.get_all() - # Add evidence in reverse alphabetical order, excluding special keys. + evidence = self.sendable_evidence(flowData.evidence) + # Add the other evidence lowest precedence first. self.add_query_data(queryData, evidence, self.get_selected_evidence(evidence, Constants.EVIDENCE_OTHER)) # Add cookie evidence. self.add_query_data(queryData, evidence, self.get_selected_evidence(evidence, Constants.EVIDENCE_COOKIE_PREFIX)) @@ -530,6 +536,28 @@ def get_content(self, flowData): self.add_query_data(queryData, evidence, self.get_selected_evidence(evidence, Constants.EVIDENCE_QUERY_PREFIX)) return queryData + def sendable_evidence(self, evidence): + + """! + The evidence the engine sends, being every value in the flow data, + whether an element accepted it or not, apart from the web request + view. The reserved names are left out by add_query_data, which + warns as it does so. An accepted value wins over a set aside one of + the same key. + + @param evidence: The flow data's Evidence + @return: The evidence keys and values to send + """ + + sendable = {} + # An older fiftyone_pipeline_core sets nothing aside. + unaccepted = getattr(evidence, "get_unaccepted", dict)() + for source in (unaccepted, evidence.get_all()): + for key, value in source.items(): + if str(key).lower() != HTTP_REQUEST_KEY: + sendable[key] = value + return sendable + def add_query_data(self, query_data, all_evidence, evidence): """! @@ -543,10 +571,14 @@ def add_query_data(self, query_data, all_evidence, evidence): for evidenceKey, evidenceValue in evidence.items(): - # Get the key parts - evidenceKeyParts = evidenceKey.split(Constants.EVIDENCE_SEPERATOR) - prefix = evidenceKeyParts[0].lower() - suffix = evidenceKeyParts[-1].lower() + # The prefix is the text before the first separator, and the + # name sent is the rest of the key in the letter case given, as + # .NET's AddQueryData takes it. A key with no separator is both. + prefix, separator, suffix = evidenceKey.partition( + Constants.EVIDENCE_SEPERATOR) + if not separator: + suffix = evidenceKey + prefix = prefix.lower() # The service reads its credentials and the list of requested # properties from these names, so evidence carrying one of them @@ -555,13 +587,19 @@ def add_query_data(self, query_data, all_evidence, evidence): # say so as the conflict case below does, so a caller who set # it expecting an effect can see why there was none. The value # is not logged, as it may be a credential. - if suffix in Constants.RESERVED_EVIDENCE_SUFFIXES: + if suffix.lower() in Constants.RESERVED_EVIDENCE_SUFFIXES: warnings.warn( Constants.WARNING_RESERVED_EVIDENCE.format(evidenceKey)) continue + # Evidence keys ignore letter case, so a name already added in + # other letters is the same name, and the later, higher + # precedence value replaces it under its own spelling. + existing = next((name for name in query_data + if name.lower() == suffix.lower()), None) + # Check and add the evidence to the query parameters. - if (not suffix in query_data.keys()): + if existing is None: query_data[suffix] = evidenceValue # If the queryParameter exists already. else: @@ -572,7 +610,7 @@ def add_query_data(self, query_data, all_evidence, evidence): if (prefix.lower() != Constants.EVIDENCE_QUERY_PREFIX): conflicts = {} for key, value in all_evidence.items(): - if(key.lower() != evidenceKey.lower() and suffix in key.lower()): + if(key.lower() != evidenceKey.lower() and suffix.lower() in key.lower()): conflicts[key] = value warningMessage = Constants.WARNING_MESSAGE \ @@ -585,6 +623,7 @@ def add_query_data(self, query_data, all_evidence, evidence): warnings.warn(warningMessage + conflictStr) # Overwrite the existing queryParameter value. + del query_data[existing] query_data[suffix] = evidenceValue def get_selected_evidence(self, evidence, type): @@ -604,15 +643,37 @@ def get_selected_evidence(self, evidence, type): not self.key_has_prefix(key, Constants.EVIDENCE_HTTPHEADER_PREFIX) and \ not self.key_has_prefix(key, Constants.EVIDENCE_COOKIE_PREFIX)): selected_evidence[key] = value - selected_evidence = dict(sorted(selected_evidence.items(), reverse=True)) - + # Lowest precedence first, so the value added last wins. + selected_evidence = dict(sorted( + selected_evidence.items(), + key=lambda item: self.other_prefix_rank(item[0]), + reverse=True)) + else: for key, value in evidence.items(): if self.key_has_prefix(key, type): selected_evidence[key] = value return selected_evidence - + + def other_prefix_rank(self, key): + + """! + The sort key of an evidence key that is not query, header or cookie + evidence, where a lower one wins. server, fiftyone and location + come first, in the order evidence.md lists them, then any other + prefix alphabetically, all ignoring letter case. + + @param key: The evidence key + @return: A tuple that sorts higher precedence first + """ + + prefix = key.partition(Constants.EVIDENCE_SEPERATOR)[0].lower() + if prefix in Constants.OTHER_PREFIX_PRECEDENCE: + return (Constants.OTHER_PREFIX_PRECEDENCE.index(prefix), "", + key.lower()) + return (len(Constants.OTHER_PREFIX_PRECEDENCE), prefix, key.lower()) + def key_has_prefix(self, itemKey, prefix): """! diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py index f53bee2a..5b198db4 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/src/fiftyone_pipeline_cloudrequestengine/constants.py @@ -53,6 +53,11 @@ class Constants: # processing. EVIDENCE_QUERY_PREFIX = "query" + # The prefixes after query, header and cookie, highest precedence + # first, as evidence.md lists them. Any other prefix ranks after these, + # alphabetically. + OTHER_PREFIX_PRECEDENCE = ("server", "fiftyone", "location") + # other evidence constant EVIDENCE_OTHER = "other" diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_all_evidence_sent.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_all_evidence_sent.py new file mode 100644 index 00000000..ddb6e847 --- /dev/null +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_all_evidence_sent.py @@ -0,0 +1,299 @@ +# ********************************************************************* +# 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. +# ********************************************************************* + +"""The Cloud Request Engine sends every Evidence value in the Flow Data, +apart from the reserved names and the web request view, as the Processing +section of cloud-request-engine.md says at e972d136.""" + +import warnings + +from fiftyone_pipeline_cloudrequestengine.cloudrequestengine import \ + CloudRequestEngine +from fiftyone_pipeline_cloudrequestengine.constants import Constants +from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder + +from .classes.cloudrequestengine_testbase import CloudRequestEngineTestsBase + +RESOURCE_KEY = "resource_key" +LICENSE_KEY = "a-licence-key" + + +class TestAllEvidenceSent(CloudRequestEngineTestsBase): + """! + A cloud-only pipeline, so the Cloud Request Engine is the only element + and its accepted Evidence is the list the stand-in service gives, which + holds query.User-Agent alone. Everything else is Evidence no element in + the pipeline accepts, and the engine still sends it. + """ + + def send(self, evidence, settings=None): + """ + Process one flow data holding the evidence given, through a + pipeline whose only element is the Cloud Request Engine, and give + back the body of the data request. + """ + + settings = dict(settings or {}) + settings.setdefault("resource_key", RESOURCE_KEY) + settings["http_client"] = self.mock_http() + engine = CloudRequestEngine(settings) + pipeline = PipelineBuilder().add(engine).build() + data = pipeline.create_flowdata() + for key, value in evidence.items(): + data.evidence.add(key, value) + + with warnings.catch_warnings(): + warnings.simplefilter("ignore") + data.process() + + return engine.http_client.requests[-1][2] + + def test_evidence_the_service_does_not_list_is_sent(self): + """ + The stand-in service lists none of these three names, so a store + that keeps only the accepted Evidence loses all three before the + engine reads them. The specification says the engine makes the + request "with every Evidence value in the Flow Data, apart from the + reserved names and the web request view". + """ + + content = self.send({ + "query.my-own-thing": "a value", + "header.sec-ch-ua-platform": "Windows", + "cookie.51D_experiment": "b", + }) + + self.assertEqual("a value", content["my-own-thing"]) + self.assertEqual("Windows", content["sec-ch-ua-platform"]) + self.assertEqual("b", content["51D_experiment"]) + + def test_evidence_the_service_lists_is_still_sent(self): + """ + An accepted key is still sent, so reading the set aside Evidence as + well has not displaced the Evidence the engine sent before. + """ + + content = self.send({ + "query.User-Agent": "a browser", + "query.my-own-thing": "a value", + }) + + self.assertEqual("a browser", content["User-Agent"]) + self.assertEqual("a value", content["my-own-thing"]) + + def test_query_evidence_wins_whether_listed_or_not(self): + """ + Query evidence takes precedence over header and cookie evidence of + the same name, whether the service lists the name or not. + """ + + content = self.send({ + "cookie.my-own-thing": "from the cookie", + "header.my-own-thing": "from the header", + "query.my-own-thing": "from the query", + }) + + self.assertEqual("from the query", content["my-own-thing"]) + + def test_each_prefix_wins_over_every_later_one(self): + """ + evidence.md lists query, header, cookie, server, fiftyone and + location from highest to lowest precedence, and ranks any other + prefix alphabetically after them. Each pair of prefixes gives one + name, added higher first so the order added cannot decide it, and + the higher prefix's value is the one sent. + """ + + order = ("query", "header", "cookie", "server", "fiftyone", + "location", "apple", "zebra") + evidence = {} + for i, higher in enumerate(order): + for lower in order[i + 1:]: + name = "n-" + higher + "-" + lower + evidence[higher + "." + name] = higher + evidence[lower + "." + name] = lower + + content = self.send(evidence) + + wrong = [] + for i, higher in enumerate(order): + for lower in order[i + 1:]: + name = "n-" + higher + "-" + lower + if content.get(name) != higher: + wrong.append(name + " sent " + str(content.get(name))) + self.assertEqual([], wrong) + + def test_a_prefix_ranks_the_same_in_any_letter_case(self): + """ + Evidence keys ignore letter case, so Location ranks as location and + SERVER as server. + """ + + content = self.send({ + "fiftyone.b": "fiftyone", + "Location.b": "location", + "SERVER.c": "server", + "Location.c": "location", + }) + + self.assertEqual("fiftyone", content["b"]) + self.assertEqual("server", content["c"]) + + def test_other_prefixes_rank_by_the_prefix_alone(self): + """ + Other prefixes rank alphabetically by the prefix, so ab comes + before ab-c, although the key ab-c.n sorts before ab.n because a + hyphen sorts before a dot. + """ + + content = self.send({"ab.n": "ab", "ab-c.n": "ab-c"}) + + self.assertEqual("ab", content["n"]) + + def test_a_reserved_name_is_never_sent_from_evidence(self): + """ + The service reads its credentials and the property list from these + names, so Evidence carrying one of them would sit beside the + engine's own values with no way to know which one the service + applied. The specification says Evidence under any prefix whose key + strips to values, resource, license or licence, in any letter case, + MUST be left out. + """ + + evidence = { + "query.resource": "evidence_resource", + "query.License": "evidence_license", + "header.LICENCE": "evidence_licence", + "cookie.values": "evidence_values", + "query.my-own-thing": "a value", + } + + # A resource key and a licence key, with no property list, so + # nothing the engine sends is called values. + content = self.send(evidence, {"license_key": LICENSE_KEY}) + self.assertEqual(RESOURCE_KEY, content[Constants.RESOURCE_PARAMETER]) + self.assertEqual(LICENSE_KEY, content[Constants.LICENSE_PARAMETER]) + self.assertNotIn(Constants.VALUES_PARAMETER, content) + self.assertNotIn(Constants.LICENCE_SPELLING, content) + for name, value in content.items(): + self.assertNotIn("evidence_", str(value), name) + self.assertEqual("a value", content["my-own-thing"]) + + # A licence key alone, with the property list it needs, so nothing + # the engine sends is called resource. + content = self.send(evidence, { + "resource_key": "", + "license_key": LICENSE_KEY, + "requested_properties": ["device.ismobile"], + }) + self.assertNotIn(Constants.RESOURCE_PARAMETER, content) + self.assertEqual(LICENSE_KEY, content[Constants.LICENSE_PARAMETER]) + self.assertEqual( + "device.ismobile", content[Constants.VALUES_PARAMETER]) + self.assertNotIn(Constants.LICENCE_SPELLING, content) + for name, value in content.items(): + self.assertNotIn("evidence_", str(value), name) + self.assertEqual("a value", content["my-own-thing"]) + + def test_the_web_request_view_is_never_sent(self): + """ + A web integration adds the web request itself under + server.http-request. It is a view of the whole request and not a + value to send, so the specification says it MUST be left out. + """ + + content = self.send({ + "server.http-request": {"headers": {"user-agent": "a browser"}}, + "query.my-own-thing": "a value", + }) + + self.assertEqual( + [], [name for name in content if "http-request" in name.lower()]) + self.assertEqual("a value", content["my-own-thing"]) + + def test_a_reserved_name_is_left_out_under_any_prefix_and_case(self): + """ + The letter case of the key does not change which name the service + reads, so a reserved name under any prefix is left out whatever its + case. + """ + + content = self.send({ + "header.ReSoUrCe": "evidence_resource", + "cookie.LiCeNsE": "evidence_license", + "query.VALUES": "evidence_values", + }) + + for name, value in content.items(): + self.assertNotIn("evidence_", str(value), name) + + def test_the_name_sent_is_everything_after_the_first_dot(self): + """ + The prefix is the text before the first dot, and removing it leaves + the rest of the key, dots included. So a.license is not the + reserved name license, and a key with no dot is sent whole. + """ + + content = self.send({ + "query.a.license": "after the first dot", + "header.x.y.z": "three parts", + "nodot": "no prefix", + }) + + self.assertEqual("after the first dot", content["a.license"]) + self.assertEqual("three parts", content["x.y.z"]) + self.assertEqual("no prefix", content["nodot"]) + self.assertNotIn("z", content) + + def test_the_name_sent_keeps_the_letter_case_given(self): + """ + The name is sent in the letter case of the evidence key, as .NET + sends it, rather than lower-cased. + """ + + content = self.send({ + "cookie.51D_Experiment": "b", + "header.Sec-CH-UA-Platform": "Windows", + }) + + self.assertEqual("b", content["51D_Experiment"]) + self.assertEqual("Windows", content["Sec-CH-UA-Platform"]) + self.assertNotIn("51d_experiment", content) + self.assertNotIn("sec-ch-ua-platform", content) + + def test_one_name_in_two_letter_cases_is_sent_once(self): + """ + Evidence keys ignore letter case, so header.User-Agent and + query.user-agent name the same value, and the query value wins + under the precedence order. The name is sent once, spelled as the + winning key spells it. + """ + + content = self.send({ + "header.User-Agent": "from the header", + "query.user-agent": "from the query", + }) + + self.assertEqual( + ["user-agent"], + [name for name in content if name.lower() == "user-agent"]) + self.assertEqual("from the query", content["user-agent"]) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_evidence.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_evidence.py index ba9f157a..748a4b59 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_evidence.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudengine_evidence.py @@ -21,8 +21,8 @@ # ********************************************************************* """A cloud engine reads only the cloud request engine's results, so it -accepts no evidence, and a cloud pipeline sends the service only the -evidence the service lists and the elements the builder adds read.""" +accepts no evidence, and a cloud pipeline still sends the service every +evidence value apart from the reserved names.""" from fiftyone_pipeline_cloudrequestengine.cloudengine import CloudEngine from fiftyone_pipeline_cloudrequestengine.cloudrequestengine import \ @@ -44,10 +44,10 @@ def test_a_cloud_engine_accepts_no_evidence(self): "server.client-ip"): self.assertFalse(engine.filter_evidence_key(key), key) - def test_a_cloud_pipeline_sends_only_the_evidence_the_service_lists(self): + def test_a_cloud_pipeline_sends_every_evidence_value(self): """ - The stand-in service lists query.User-Agent alone, so of this - evidence only the user agent reaches it. + The stand-in service lists query.User-Agent alone, and no element + accepts the header or the cookie, yet all three reach it. """ cloud = CloudRequestEngine({ "license_key": LICENSE_KEY, @@ -65,6 +65,6 @@ def test_a_cloud_pipeline_sends_only_the_evidence_the_service_lists(self): data.process() request_type, url, content = cloud.http_client.requests[-1] - self.assertEqual("iPhone", content["user-agent"]) - self.assertNotIn("x-test", content) - self.assertNotIn("other", content) + self.assertEqual("a header", content["x-test"]) + self.assertEqual("a cookie", content["other"]) + self.assertEqual("iPhone", content["User-Agent"]) diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py index 1c39b024..4e246298 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_cloudrequestengine.py @@ -202,7 +202,7 @@ def test_get_content(self, evidence, expected_value): result = engine.get_content(data) - self.assertEqual(expected_value, result["user-agent"]) + self.assertEqual(expected_value, result["User-Agent"]) def test_no_exception_on_cloud_request_engine_init(self): """ diff --git a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py index 09f01bfe..75b08646 100644 --- a/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py +++ b/pipeline/fiftyone_pipeline_cloudrequestengine/tests/test_credentials.py @@ -86,7 +86,7 @@ def test_resource_key_alone_is_accepted(self): self.assertEqual(RESOURCE_KEY, content[Constants.RESOURCE_PARAMETER]) self.assertNotIn(Constants.LICENSE_PARAMETER, content) self.assertNotIn(Constants.VALUES_PARAMETER, content) - self.assertEqual("iPhone", content["user-agent"]) + self.assertEqual("iPhone", content["User-Agent"]) def test_resource_key_with_licence_key_is_accepted(self): """ @@ -291,7 +291,7 @@ def test_reserved_evidence_is_left_out_of_the_body(self): self.assertEqual( "device.ismobile,device.istablet", content[Constants.VALUES_PARAMETER]) - self.assertEqual("iPhone", content["user-agent"]) + self.assertEqual("iPhone", content["User-Agent"]) # Each piece of evidence left out is named once, without its # value, as the value may be a credential. @@ -389,11 +389,11 @@ def test_evidence_never_supplies_values(self): Constants.LICENSE_PARAMETER, Constants.VALUES_PARAMETER)]) self.assert_other_evidence_sent(content) - def test_evidence_no_element_accepts_is_not_sent(self): + def test_evidence_no_element_accepts_is_sent(self): """ - A default pipeline keeps only the evidence one of its elements - accepts, so a header or cookie the service does not ask for is not - sent to it. + The engine sends every evidence value in the flow data apart from + the reserved names, so a header and a cookie that no element in a + default pipeline accepts are sent as well. """ engine = CloudRequestEngine({ "license_key": LICENSE_KEY, @@ -409,9 +409,9 @@ def test_evidence_no_element_accepts_is_not_sent(self): data.process() request_type, url, content = engine.http_client.requests[-1] - self.assertEqual("iPhone", content["user-agent"]) - self.assertNotIn("x-test", content) - self.assertNotIn("other", content) + self.assertEqual("a header", content["x-test"]) + self.assertEqual("a cookie", content["other"]) + self.assertEqual("iPhone", content["User-Agent"]) def test_properties_added_later_are_normalised_and_checked(self): """