From 76897c6e83cadb4cace1d1ed23d7edabd72db71c Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:13:04 +0000 Subject: [PATCH 1/7] Add failing tests for web elements the caller adds and for SetHeader names These tests fail on the code as it is and pass once the fixes that follow are in. PipelineBuilder.build() adds the Sequence Element, the JSON Builder, the JavaScript Builder and the Set Headers Element whatever the caller added. web-integration.md lines 30 to 45 at 706de1f0 says each MUST be present and is added only when it is not, with the JSON Builder immediately before the JavaScript Builder. The new tests in tests/test_web_integration_elements.py add each element themselves and expect it to be the only one of its kind, where the caller put it. A caller's own Sequence Element must give a sequence of 1 for a request with no session id, where today the request is counted twice and gives 2. A JSON Builder the caller lacks must go immediately before the caller's JavaScript Builder, and an element whose data key differs only in letter case counts as present. set-headers-element.md lines 10 to 20 at 706de1f0 says the identifier in SetHeader[Identifier][HeaderName] "MUST NOT have upper-case characters after the first character", so the first character may be either case. tests/test_setheader.py asserted that SetHeaderbrowserAccept-CH is refused, which pinned the wrong behaviour, so that name now expects Accept-CH. SetHeaderAcceptAccept-CH must give Accept-CH, where today every copy of the identifier is removed and it gives -CH. SetHeader with nothing after it must raise the wrong format message rather than an IndexError. The error test is renamed test_get_response_header_name_error, because it shared its name with the test of valid names, and it now fails when nothing is raised. --- .../tests/test_setheader.py | 29 +++-- .../tests/test_web_integration_elements.py | 100 +++++++++++++++++- 2 files changed, 115 insertions(+), 14 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py index 240080fb..c59eaf66 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py @@ -83,30 +83,39 @@ 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)) 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..5a9e1d85 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): From aee07976604857dfed771da87562c8bcc547edf6 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:13:51 +0000 Subject: [PATCH 2/7] Test that the basic list filter matches keys in any letters The lookup in BasicListEvidenceKeyFilter is about to change from a search of its list to a set, and these tests pass on the code as it is, so they show the change keeps what the filter does. A listed key is accepted in any letters and anything else is refused. The list attribute still holds the keys in lower case in the order given, because the web integration and the metadata examples read it. A list set after the filter is made, or by a subclass that never calls the base constructor, is the one matched. --- .../test_basiclist_evidence_keyfilter.py | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_core/tests/test_basiclist_evidence_keyfilter.py 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() From 33c3d114cacb428e28988c1c5db7ad476eed4929 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:14:38 +0000 Subject: [PATCH 3/7] Add the web elements only when the caller has not web-integration.md lines 30 to 45 at 706de1f0 says the Sequence Element, the JSON Builder, the JavaScript Builder and the Set Headers Element MUST be present, and that each is added only when it is not. An added Sequence Element goes first, an added JSON Builder immediately before the JavaScript Builder, and an added JavaScript Builder or Set Headers Element after all other elements. PipelineBuilder.build() added all four whatever the caller had added. A caller's own Sequence Element then ran twice, so each request moved the sequence on by two, and a caller's own JavaScript Builder ran before the JSON Builder added after it, so it had no JSON to read. build() now adds each element only when none of the caller's elements has the same data key, compared in any letters. The data key decides rather than the class because flow data keeps each element's results under its data key in lower case, and the JavaScript Builder and the web integration functions read these results by that key. A second element with the same key would replace the caller's results whatever its class, and a caller's own element in place of one of these, of any class, is the one those readers find. The Node.js pipeline builder decides the same way. When the caller added a JavaScript Builder but no JSON Builder, the JSON Builder goes immediately before the caller's JavaScript Builder. Otherwise the added elements go where they went before. The add_javascript_builder, add_sequence_element, add_json_builder_element, add_javascript_builder_element, use_setheader_properties and add_set_header_element settings work as before. build_from_configuration is unchanged. This makes CallerAddedWebElementTests in tests/test_web_integration_elements.py pass. --- .../fiftyone_pipeline_core/pipelinebuilder.py | 62 +++++++++++++++++-- 1 file changed, 57 insertions(+), 5 deletions(-) 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. From 6d3caf1387e3069cc974864c139c5332750670eb Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:15:50 +0000 Subject: [PATCH 4/7] Read the SetHeader identifier in either case and keep the header name whole set-headers-element.md lines 10 to 20 at 706de1f0 names these properties SetHeader[Identifier][HeaderName], and says the identifier "MUST NOT have upper-case characters after the first character", so its first character may be either case. get_response_header_name refused an identifier that starts in lower case, so SetHeaderbrowserAccept-CH raised an error instead of giving Accept-CH. It also took the identifier out with replace, which removes every copy of it, so SetHeaderAcceptAccept-CH gave -CH. A name of SetHeader alone raised an IndexError rather than the wrong format message. The header name is now everything from the first upper-case letter after the identifier's first character. A name with no identifier, or no header name after it, raises the wrong format message, and a name that does not start with SetHeader raises the same message as before. The upper-case letters are A to Z, as in the split this replaces. A name whose identifier starts with one of them, and does not appear again in the header name, gives the same header as before, which covers SetHeaderBrowserAccept-CH, SetHeaderHardwareAccept-CH and SetHeaderPlatformAccept-CH. This makes test_get_response_header_name and test_get_response_header_name_error in tests/test_setheader.py pass. --- .../setheaderelement.py | 54 ++++++++----------- 1 file changed, 21 insertions(+), 33 deletions(-) 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..1f4cce13 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): @@ -192,42 +195,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)) From 3fc38deba78963aad163a16f23bc6745d36b07b8 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:19:02 +0000 Subject: [PATCH 5/7] Test that a matching request is not processed, a numeric sequence, and the JSON served These tests pass on the code as it is. Each pins a rule the code already meets. web-integration.md lines 242 to 253 at 706de1f0 says the endpoint MUST compare the If-None-Match header with the ETag before the Pipeline processes the request, and MUST NOT process a request whose If-None-Match matches. The existing tests checked only the status code. test_a_matching_request_is_not_processed in tests/test_web_client_side.py counts the requests an element processes, and a matching GET, HEAD or POST leaves the count unchanged. sequence-element.md lines 39 to 44 at 706de1f0 says a query.sequence that is not a whole number, or a string of the digits 0 to 9, from 0 up to 2,147,483,646 is not present, and that no error is recorded. The existing test gave -5 and 5.0 only as text. testSequenceNumberThatIsNegativeOrHasAFractionIsAbsent in tests/test_web_integration_elements.py gives -5 and 1.5 as numbers and expects a sequence of 1 with no error. javascript-builder.md line 348 at 706de1f0 says the endpoint the script calls back "MUST return JSON data in the same form as the JSON embedded within the JavaScript". test_the_json_served_is_the_json_builders_json in tests/test_web_client_side.py keeps the flow data the JSON endpoint processes and checks that the body is that flow data's JSON Builder JSON. --- .../tests/test_web_client_side.py | 86 +++++++++++++++++++ .../tests/test_web_integration_elements.py | 10 +++ 2 files changed, 96 insertions(+) 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 5a9e1d85..000d47fb 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_web_integration_elements.py @@ -276,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), From a3a68639dc3cb7c0ccabff2c96b9d968f9192f61 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:32:38 +0000 Subject: [PATCH 6/7] Test that only a property whose name starts with SetHeader sets a response header --- .../tests/test_setheader.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py index c59eaf66..a666e2e9 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_setheader.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_setheader.py @@ -119,3 +119,24 @@ def test_get_response_header_name_error( 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())) From 94914bc9363f7f4db0df77e057600e2f9680aad8 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Sat, 14 Nov 2026 16:32:42 +0000 Subject: [PATCH 7/7] Take only properties whose names start with SetHeader, and name the identifier in the format message --- .../src/fiftyone_pipeline_core/messages.py | 2 +- .../src/fiftyone_pipeline_core/setheaderelement.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) 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/setheaderelement.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py index 1f4cce13..4e9f2ca6 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/setheaderelement.py @@ -104,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