diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/messages.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/messages.py index 5823f175..05f2996e 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/messages.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/messages.py @@ -33,7 +33,7 @@ class Messages(): # as format argument. WRONG_PROPERTY_FORMAT = \ - "Property Name '{}' is not in the expected format i.e. SetHeader[Component][HeaderName]. " + "Property Name '{}' is not in the expected format i.e. SetHeader[Identifier][HeaderName]. " # Element not found in flowData. This takes the element datakey # as format argument. diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipelinebuilder.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipelinebuilder.py index e208b686..2f0746d1 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipelinebuilder.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/pipelinebuilder.py @@ -28,6 +28,17 @@ from .setheaderelement import SetHeaderElement import importlib +# The data keys of the JSONBundler and the JavaScriptBuilder. +JSON_BUILDER_KEY = "jsonbundler" +JAVASCRIPT_BUILDER_KEY = "javascriptbuilder" + + +def _data_key(element): + + # Flow data keeps each element's results under its data key in lower + # case, so two keys that differ only in letters are the same key. + return str(element.datakey).lower() + class PipelineBuilder(object): """ @@ -40,6 +51,9 @@ class PipelineBuilder(object): def __init__(self, settings= {}): """ Pipeline Builder constructor. + The SequenceElement, JSONBundler, JavaScriptBuilder and + SetHeaderElement switches below each add their element only when the + caller has not added one with the same data key. @type settings: dictionary @param settings : settings for the pipeline builder including: `add_javascript_builder (Bool)` - Whether to add the JavaScriptBuilder, with the @@ -188,14 +202,52 @@ def build(self): """ - flow_elements = ( - self.get_sequence_elements() + - self.flow_elements + - self.get_javascript_elements() + - self.get_setheader_elements()) + flow_elements = self._add_web_elements(self.flow_elements) return Pipeline(flow_elements, logger=self.logger, suppress_process_exceptions=self.suppress_process_exceptions, data_file_update_service=self.data_file_update_service, auto_close_elements=self.auto_close_elements) + def _add_web_elements(self, flow_elements): + + """! + The caller's elements with each web integration element the settings + ask for, unless the caller added one with the same data key. The + SequenceElement goes first and the others after every other element, + except that a JSONBundler goes immediately before a JavaScriptBuilder + the caller added, because the JavaScriptBuilder reads the + JSONBundler's results. + + @type flow_elements: list + @param flow_elements: The FlowElements the caller added + @rtype: list + @return: Returns a new list of FlowElements + """ + + present = set(_data_key(element) for element in flow_elements) + + def absent(element): + return _data_key(element) not in present + + elements = [element for element in self.get_sequence_elements() + if absent(element)] + elements.extend(flow_elements) + + for element in self.get_javascript_elements(): + if not absent(element): + continue + builder = next( + (index for index, added in enumerate(elements) + if _data_key(added) == JAVASCRIPT_BUILDER_KEY), + None) + if _data_key(element) == JSON_BUILDER_KEY and builder is not None: + elements.insert(builder, element) + else: + elements.append(element) + + elements.extend(element for element in self.get_setheader_elements() + if absent(element)) + + return elements + def add_logger(self, logger): """ Add an instance of the logger class to the pipeline. diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py index 083c5617..4e9f2ca6 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py @@ -24,7 +24,10 @@ from .flowelement import FlowElement from .elementdata_dictionary import ElementDataDictionary from .messages import Messages -import re + +# The start of the name of every property whose value is for a response +# header. +SET_HEADER_PREFIX = "SetHeader" class SetHeaderElement(FlowElement): @@ -101,8 +104,9 @@ def get_setheader_properties_pipeline(self, pipeline): # Loop over each flowElement property for propertykey, propertymeta in properties.items(): - # Check if the property starts with SetHeader - if("setheader" in propertykey): + # Property keys are in lower case, and only a name that + # starts with SetHeader is for a response header. + if propertykey.startswith(SET_HEADER_PREFIX.lower()): setHeader_element_list.append(propertymeta["name"]) # Add SetHeader element list in dict against flowElement.datakey as key @@ -192,42 +196,27 @@ def get_property_value(self, flowData, element_key, property_key): def get_response_header_name(self, property_key): """! - Determines which response header the property value will be appended to by - stripping the 'SetHeader' string and the 'Component Name' from the property name. - - @param property_key: Key for SetHeaderAcceptCH property + The response header a SetHeader property's value is for. The + property name is SetHeader[Identifier][HeaderName]. The identifier + may start with either case and has no upper-case letters after its + first character, so the header name starts at the first upper-case + letter after that character. + + @param property_key: A property name, such as SetHeaderBrowserAccept-CH @rtype String - @return Response Header name + @return The response header name, such as Accept-CH """ - actual_property_name = property_key - response_header = "" - - # Check if property name starts with SetHeader. - # If Yes, Discard SetHeader from property name. - if ("SetHeader" not in property_key[0:9]): - raise Exception(Messages.PROPERTY_NOT_SET_HEADER.format(actual_property_name)) - else: - # Discard SetHeader from property_key - property_key = property_key.replace("SetHeader", "", 1) + if not property_key.startswith(SET_HEADER_PREFIX): + raise Exception( + Messages.PROPERTY_NOT_SET_HEADER.format(property_key)) - # Check if the first letter of Component name is in Uppercase - # If Yes, Split the propertyKey based on Uppercase letters - if(property_key[0].isupper() is False): - raise Exception(Messages.WRONG_PROPERTY_FORMAT.format(actual_property_name)) - else: - # Get the Component name string to be removed from the key - parts = re.findall(r'([A-Z][^A-Z]*)', property_key) - discard_letter = parts[0] - - # Check if property name contains the header name that starts with upper case - # If Yes, Remove the previously found Component Name to get the Header Name - if len(parts)<=1 : - raise Exception(Messages.WRONG_PROPERTY_FORMAT.format(actual_property_name)) + name = property_key[len(SET_HEADER_PREFIX):] - else: - # Replace the first letter with the empty string. - response_header = property_key.replace(discard_letter, "") + for index in range(1, len(name)): + if "A" <= name[index] <= "Z": + return name[index:] - return response_header + # There is no identifier, or no header name after it. + raise Exception(Messages.WRONG_PROPERTY_FORMAT.format(property_key)) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_basiclist_evidence_keyfilter.py b/pipeline/fiftyone_pipeline_core/tests/test_basiclist_evidence_keyfilter.py new file mode 100644 index 00000000..f320b32f --- /dev/null +++ b/pipeline/fiftyone_pipeline_core/tests/test_basiclist_evidence_keyfilter.py @@ -0,0 +1,91 @@ +# ********************************************************************* +# 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 basic list filter accepts the keys it was given, in any letters, and +nothing else. Its list attribute is read by other code, so it keeps holding +the keys in lower case, and the filter matches what the list holds when the +list is set after the filter is made.""" + +import unittest + +from fiftyone_pipeline_core.basiclist_evidence_keyfilter import \ + BasicListEvidenceKeyFilter + + +class SetsItsOwnList(BasicListEvidenceKeyFilter): + """A filter that sets its list without calling the base constructor.""" + + def __init__(self): + self.list = ["header.user-agent"] + + +class BasicListEvidenceKeyFilterTests(unittest.TestCase): + + def test_a_listed_key_is_accepted_in_any_letters(self): + key_filter = BasicListEvidenceKeyFilter( + ["Header.User-Agent", "query.sequence"]) + for key in ("header.user-agent", "HEADER.USER-AGENT", + "Header.User-Agent", "query.sequence", "Query.Sequence"): + self.assertTrue(key_filter.filter_evidence_key(key), key) + + def test_a_key_that_is_not_listed_is_refused(self): + key_filter = BasicListEvidenceKeyFilter(["header.user-agent"]) + for key in ("header.user", "header.user-agent2", "user-agent", + "cookie.user-agent", ""): + self.assertFalse(key_filter.filter_evidence_key(key), key) + + def test_an_empty_list_accepts_nothing(self): + key_filter = BasicListEvidenceKeyFilter([]) + self.assertFalse(key_filter.filter_evidence_key("header.user-agent")) + self.assertFalse(key_filter.filter_evidence_key("")) + + def test_filtering_keeps_listed_keys_as_they_were_given(self): + key_filter = BasicListEvidenceKeyFilter(["header.user-agent"]) + self.assertEqual( + {"Header.User-Agent": "a browser"}, + key_filter.filter_evidence({ + "Header.User-Agent": "a browser", + "header.accept": "text/html"})) + + def test_the_list_holds_the_keys_in_lower_case_in_order(self): + key_filter = BasicListEvidenceKeyFilter( + ["Query.Sequence", "header.user-agent", "Header.Host"]) + self.assertEqual( + ["query.sequence", "header.user-agent", "header.host"], + key_filter.list) + self.assertIsInstance(key_filter.list, list) + + def test_a_list_set_after_the_filter_is_made_is_matched(self): + key_filter = BasicListEvidenceKeyFilter(["header.user-agent"]) + key_filter.list = ["header.accept"] + self.assertTrue(key_filter.filter_evidence_key("Header.Accept")) + self.assertFalse(key_filter.filter_evidence_key("header.user-agent")) + + def test_a_subclass_that_sets_its_own_list_is_matched(self): + key_filter = SetsItsOwnList() + self.assertTrue(key_filter.filter_evidence_key("Header.User-Agent")) + self.assertFalse(key_filter.filter_evidence_key("header.accept")) + self.assertEqual(["header.user-agent"], key_filter.list) + + +if __name__ == "__main__": + unittest.main() diff --git a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py index 240080fb..a666e2e9 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py @@ -83,30 +83,60 @@ def test_set_response_header(self, data, expected_value): response = set_response_header(flowdata, response) self.assertEqual(expected_value, Constants.ACCEPTCH_HEADER in response.headers) - # Test get response header function for valid formats. + # Test get response header function for valid formats. The name is + # SetHeader[Identifier][HeaderName], and the identifier has no upper-case + # characters after its first character, which may be either case. @parameterized.expand([ ("SetHeaderBrowserAccept-CH", "Accept-CH"), + ("SetHeaderHardwareAccept-CH", "Accept-CH"), + ("SetHeaderPlatformAccept-CH", "Accept-CH"), ("SetHeaderBrowserCritical-CH", "Critical-CH"), - ("SetHeaderUnknownAccept-CH", "Accept-CH") + ("SetHeaderUnknownAccept-CH", "Accept-CH"), + ("SetHeaderbrowserAccept-CH", "Accept-CH"), + # The header name holds the identifier, which must be kept. + ("SetHeaderAcceptAccept-CH", "Accept-CH") ]) def test_get_response_header_name(self, property_key, expected_value): setHeaderElement = SetHeaderElement() actual_value = setHeaderElement.get_response_header_name(property_key) self.assertEqual(expected_value, actual_value) - # Test get response header function raises exception + # Test get response header function raises exception # for invalid property formats. @parameterized.expand([ ("TestBrowserAccept-CH", Messages.PROPERTY_NOT_SET_HEADER), - ("SetHeaderbrowserAccept-CH", Messages.WRONG_PROPERTY_FORMAT), - ("SetHeaderBrowseraccept-ch", Messages.WRONG_PROPERTY_FORMAT) + ("SetHeaderBrowseraccept-ch", Messages.WRONG_PROPERTY_FORMAT), + ("SetHeaderbrowser", Messages.WRONG_PROPERTY_FORMAT), + ("SetHeaderBrowser", Messages.WRONG_PROPERTY_FORMAT), + ("SetHeader", Messages.WRONG_PROPERTY_FORMAT) ]) - def test_get_response_header_name(self, property_value, expected_message): + def test_get_response_header_name_error( + self, property_value, expected_message): setHeaderElement = SetHeaderElement() - try: + with self.assertRaises(Exception) as caught: setHeaderElement.get_response_header_name(property_value) - except Exception as e: - message = str(e) - self.assertEqual(expected_message.format(property_value), message) + self.assertEqual( + expected_message.format(property_value), str(caught.exception)) + + # Only a property whose name starts with SetHeader is for a response + # header, so a name that holds SetHeader later on is left alone. + def test_get_setheader_properties_pipeline_needs_the_prefix(self): + + class Element(object): + datakey = "device" + + def get_properties(self): + return { + "setheaderbrowseraccept-ch": { + "name": "SetHeaderBrowserAccept-CH"}, + "notsetheaderbrowseraccept-ch": { + "name": "NotSetHeaderBrowserAccept-CH"}} + + class Pipeline(object): + flow_elements = [Element()] + + self.assertEqual( + {"device": ["SetHeaderBrowserAccept-CH"]}, + SetHeaderElement().get_setheader_properties_pipeline(Pipeline())) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_web_client_side.py b/pipeline/fiftyone_pipeline_core/tests/test_web_client_side.py index ddc07912..19c1164f 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_web_client_side.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_web_client_side.py @@ -118,6 +118,22 @@ def filter_evidence_key(self, key): return key.lower() == "header.user-agent" +class Counted(FlowElement): + """An element that counts the requests it processes.""" + + def __init__(self): + super(Counted, self).__init__() + self.datakey = "counted" + self.processed = 0 + + def process_internal(self, flowdata): + self.processed += 1 + flowdata.set_element_data(ElementDataDictionary(self, {})) + + def get_evidence_key_filter(self): + return BasicListEvidenceKeyFilter(["header.user-agent"]) + + def client(): """A test client of an application serving the two client-side endpoints.""" @@ -259,6 +275,76 @@ def test_revalidation_with_the_etag_is_answered_not_modified(self): self.assertEqual(good.headers.get(name), response.headers.get(name), name) + def test_a_matching_request_is_not_processed(self): + """The endpoints compare If-None-Match with the ETag before the + pipeline processes the request, and do not process a request that + matches, as the cache header detail of the web integration + specification asks.""" + + counted = Counted() + pipeline = PipelineBuilder({ + "javascript_builder_settings": {"endpoint": "/json"}, + }).add(counted).build() + app = Flask(__name__) + + @app.route("/51Degrees.core.js") + def javascript(): + return javascript_response(pipeline, request) + + @app.route("/json", methods=["POST"]) + def json_endpoint(): + return json_response(pipeline, request) + + client = app.test_client() + script = client.get( + "/51Degrees.core.js", headers={"User-Agent": CHROME}) + results = client.post("/json", headers={"User-Agent": CHROME}) + self.assertEqual(2, counted.processed) + + for method, path, good, status in ( + (client.get, "/51Degrees.core.js", script, 304), + (client.head, "/51Degrees.core.js", script, 304), + (client.post, "/json", results, 412)): + response = method(path, headers={ + "User-Agent": CHROME, + "If-None-Match": good.headers["ETag"]}) + self.assertEqual(status, response.status_code) + + self.assertEqual(2, counted.processed) + + def test_the_json_served_is_the_json_builders_json(self): + """The JavaScript Builder specification says the endpoint the + script calls back MUST return JSON in the same form as the JSON + within the script, which is the JSON Builder's. The flow data the + endpoint processes is kept, so the body can be compared with its + JSON.""" + + pipeline = PipelineBuilder({ + "javascript_builder_settings": {"endpoint": "/json"}, + }).add(Switchable()).build() + processed = [] + create_flowdata = pipeline.create_flowdata + + def create_and_keep(): + flowdata = create_flowdata() + processed.append(flowdata) + return flowdata + + app = Flask(__name__) + + @app.route("/json", methods=["POST"]) + def json_endpoint(): + return json_response(pipeline, request) + + with mock.patch.object(pipeline, "create_flowdata", create_and_keep): + response = app.test_client().post( + "/json", headers={"User-Agent": CHROME}) + + self.assertEqual(200, response.status_code) + self.assertEqual(1, len(processed)) + self.assertEqual(processed[0].jsonbundler.json, response.get_json()) + self.assertEqual(42, response.get_json()["switchable"]["answer"]) + def test_a_post_with_a_matching_etag_is_refused(self): """RFC 9110 section 13.1.2 allows a 304 for GET and HEAD only, and asks for 412 Precondition Failed for any other method.""" diff --git a/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py b/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py index b7e049ac..000d47fb 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py @@ -3,29 +3,43 @@ The elements the web integration adds (Sequence, JSON Builder, JavaScript Builder and Set Headers) each have a switch that defaults to on, and the Sequence Element comes first, as the web integration specification says. +Each is added only when the caller has not added one already. """ import unittest from fiftyone_pipeline_core.flowelement import FlowElement +from fiftyone_pipeline_core.javascriptbuilder import JavascriptBuilderElement +from fiftyone_pipeline_core.jsonbundler import JSONBundlerElement from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder +from fiftyone_pipeline_core.sequenceelement import SequenceElement +from fiftyone_pipeline_core.setheaderelement import SetHeaderElement class UserElement(FlowElement): - def __init__(self): + def __init__(self, datakey="user"): super(UserElement, self).__init__() - self.datakey = "user" + self.datakey = datakey def process_internal(self, flowdata): pass +def keys(pipeline): + return [element.datakey for element in pipeline.flow_elements] + + def datakeys(settings=None): builder = PipelineBuilder(settings or {}) builder.add(UserElement()) - pipeline = builder.build() - return [element.datakey for element in pipeline.flow_elements] + return keys(builder.build()) + + +# The data keys of a pipeline holding the user element and every element +# the web integration adds, in order. +ALL_KEYS = [ + "sequence", "user", "jsonbundler", "javascriptbuilder", "set-headers"] class WebIntegrationElementSwitchTests(unittest.TestCase): @@ -76,6 +90,84 @@ def testEverySwitchOff(self): "add_set_header_element": False})) +# The web integration specification says each of these elements MUST be +# present and is added only if it is not. An element the caller added is the +# one the pipeline uses, where the caller put it, and no other is added. +class CallerAddedWebElementTests(unittest.TestCase): + + def testCallersSequenceElementIsTheOnlyOneAndIsFirst(self): + sequence = SequenceElement() + pipeline = PipelineBuilder().add(sequence).add(UserElement()).build() + self.assertEqual(ALL_KEYS, keys(pipeline)) + self.assertIs(sequence, pipeline.flow_elements[0]) + + def testCallersSequenceElementCountsARequestOnce(self): + # With no session id the sequence is 1. A second Sequence Element + # would count the same request again and give 2. + pipeline = PipelineBuilder().add(SequenceElement()) \ + .add(UserElement()).build() + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertEqual(1, flowdata.get("sequence").get("sequence")) + self.assertEqual(1, flowdata.evidence.get("query.sequence")) + + def testCallersJsonBuilderIsTheOnlyOne(self): + json_builder = JSONBundlerElement() + pipeline = PipelineBuilder().add(UserElement()) \ + .add(json_builder).build() + self.assertEqual(ALL_KEYS, keys(pipeline)) + self.assertIs(json_builder, pipeline.flow_elements[2]) + + def testCallersJavaScriptBuilderIsTheOnlyOneAndWritesTheScript(self): + javascript_builder = JavascriptBuilderElement( + {"obj_name": "ownObject"}) + pipeline = PipelineBuilder().add(UserElement()) \ + .add(JSONBundlerElement()).add(javascript_builder).build() + self.assertEqual(ALL_KEYS, keys(pipeline)) + self.assertIs(javascript_builder, pipeline.flow_elements[3]) + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertIn("ownObject", flowdata.javascriptbuilder.javascript) + + def testJsonBuilderGoesImmediatelyBeforeCallersJavaScriptBuilder(self): + # The JavaScript Builder reads the JSON Builder's results, so the + # JSON Builder must run first. The element after the caller's + # JavaScript Builder shows that the JSON Builder is not simply added + # at the end. + javascript_builder = JavascriptBuilderElement( + {"obj_name": "ownObject"}) + pipeline = PipelineBuilder().add(UserElement()) \ + .add(javascript_builder).add(UserElement("after")).build() + self.assertEqual( + ["sequence", "user", "jsonbundler", "javascriptbuilder", + "after", "set-headers"], + keys(pipeline)) + self.assertIs(javascript_builder, pipeline.flow_elements[3]) + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertIn("ownObject", flowdata.javascriptbuilder.javascript) + + def testCallersSetHeadersElementIsTheOnlyOne(self): + set_headers = SetHeaderElement() + pipeline = PipelineBuilder().add(UserElement()) \ + .add(set_headers).build() + self.assertEqual( + ["sequence", "user", "set-headers", "jsonbundler", + "javascriptbuilder"], + keys(pipeline)) + self.assertIs(set_headers, pipeline.flow_elements[2]) + + def testElementWithTheSameDataKeyInOtherLettersCountsAsPresent(self): + # Flow data keeps each element's results under its data key in lower + # case, so another element with this key would replace them. + pipeline = PipelineBuilder().add(UserElement()) \ + .add(UserElement("Set-Headers")).build() + self.assertEqual( + ["sequence", "user", "Set-Headers", "jsonbundler", + "javascriptbuilder"], + keys(pipeline)) + + # The Sequence Element writes Element Data as well as Evidence. class SequenceElementDataTests(unittest.TestCase): @@ -184,6 +276,16 @@ def testSequenceThatIsNotWholeDigitsIsAbsent(self): {"query.session-id": "abc", "query.sequence": value}), repr(value)) + def testSequenceNumberThatIsNegativeOrHasAFractionIsAbsent(self): + # Given as numbers rather than as text. process() raises when an + # element records an error, so reaching the check means none was. + for value in (-5, 1.5): + self.assertEqual( + ("abc", 1), + self.process( + {"query.session-id": "abc", "query.sequence": value}), + repr(value)) + def testLeadingZerosAreAllowed(self): self.assertEqual( ("abc", 6),