From a2692e6808f1fd528c05b6c6efaceb1220d5f228 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 20:54:16 +0000 Subject: [PATCH 1/4] Test that the JavaScript Builder renders sequence 1 and no session id with no Sequence Element --- .../test_javascriptbuilder_no_sequence.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py new file mode 100644 index 00000000..1f62af51 --- /dev/null +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py @@ -0,0 +1,79 @@ +# ********************************************************************* +# 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. +# ********************************************************************* + +"""With no Sequence Element in the pipeline, the JavaScript Builder renders +the sequence as 1 and the session id as an empty string, as step 5 of the +JavaScript Builder specification's process says, whatever the evidence +holds.""" + +import re +import unittest + +from fiftyone_pipeline_core.elementdata_dictionary import ElementDataDictionary +from fiftyone_pipeline_core.flowelement import FlowElement +from fiftyone_pipeline_core.javascriptbuilder import JavascriptBuilderElement +from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder + + +class StandInJsonBuilder(FlowElement): + """Gives the JavaScript Builder its JSON without needing a sequence.""" + + def __init__(self): + super(StandInJsonBuilder, self).__init__() + self.datakey = "jsonbundler" + self.properties = {"json": {"type": "dict"}} + + def process_internal(self, flowdata): + flowdata.set_element_data(ElementDataDictionary( + self, {"json": {"javascriptProperties": []}})) + + +def render(evidence): + pipeline = PipelineBuilder({ + "add_javascript_builder": False, + "use_setheader_properties": False}) \ + .add(StandInJsonBuilder()) \ + .add(JavascriptBuilderElement({"minify": False})) \ + .build() + flowdata = pipeline.create_flowdata() + for key, value in evidence.items(): + flowdata.evidence.add(key, value) + flowdata.process() + return flowdata.javascriptbuilder.javascript + + +def line(script, name): + return [text.strip() for text in script.splitlines() + if re.match(r"\s*var " + name + r"\s*=", text)] + + +class NoSequenceElementTests(unittest.TestCase): + + def test_the_sequence_is_one_and_the_session_id_empty(self): + script = render({"query.sequence": "5", + "query.session-id": "abc-123"}) + self.assertEqual(["var sequence = 1;"], line(script, "sequence")) + self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + + +if __name__ == "__main__": + unittest.main() From 77dc7528c9e9f607c568c84657ecd2783721c3a0 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Tue, 10 Nov 2026 20:54:18 +0000 Subject: [PATCH 2/4] Render sequence 1 and an empty session id with no Sequence Element, as step 5 says --- .../src/fiftyone_pipeline_core/javascriptbuilder.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py index 59bdf9bd..0e5d7c7e 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py @@ -416,9 +416,16 @@ def process_internal(self, flowdata): # Both are written into the script, so each always has a safe value. # The session id is written inside quotes and is empty when absent or # not safe, and the sequence is written as bare code, so it is always - # a positive number. - variables["_sessionId"] = get_session_id(query_params.get("query.session-id")) - variables["_sequence"] = get_sequence(query_params.get("query.sequence")) + # a positive number. With no Sequence Element the sequence is 1 and + # the session id empty, whatever the evidence holds. + if "sequence" in flowdata.pipeline.flow_elements_list: + variables["_sessionId"] = get_session_id( + query_params.get("query.session-id")) + variables["_sequence"] = get_sequence( + query_params.get("query.sequence")) + else: + variables["_sessionId"] = "" + variables["_sequence"] = 1 # The session id and the sequence are left out, because the script # appends both to its own request after it has taken the record of From 6c142df24e7c8f8eb489d70419f955d6afddb4d6 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 07:30:25 +0000 Subject: [PATCH 3/4] Pin both sides of the branch, and say in the older tests where the no Sequence Element case is covered --- .../test_javascriptbuilder_no_sequence.py | 42 +++++++++++++++++-- .../tests/test_javascriptbuilder_sequence.py | 8 ++-- 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py index 1f62af51..17f3be39 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py @@ -23,7 +23,8 @@ """With no Sequence Element in the pipeline, the JavaScript Builder renders the sequence as 1 and the session id as an empty string, as step 5 of the JavaScript Builder specification's process says, whatever the evidence -holds.""" +holds. The same pipeline with a Sequence Element renders that element's +values, which is the other side of the same branch.""" import re import unittest @@ -32,6 +33,7 @@ from fiftyone_pipeline_core.flowelement import FlowElement from fiftyone_pipeline_core.javascriptbuilder import JavascriptBuilderElement from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder +from fiftyone_pipeline_core.sequenceelement import SequenceElement class StandInJsonBuilder(FlowElement): @@ -47,10 +49,17 @@ def process_internal(self, flowdata): self, {"json": {"javascriptProperties": []}})) -def render(evidence): - pipeline = PipelineBuilder({ +def render(evidence, sequence_element=False): + """Render the script from a pipeline that differs only in whether it + holds a SequenceElement. The real JSON bundler refuses a pipeline with no + sequence number, so a stand-in gives the builder its JSON.""" + + builder = PipelineBuilder({ "add_javascript_builder": False, - "use_setheader_properties": False}) \ + "use_setheader_properties": False}) + if sequence_element: + builder = builder.add(SequenceElement()) + pipeline = builder \ .add(StandInJsonBuilder()) \ .add(JavascriptBuilderElement({"minify": False})) \ .build() @@ -74,6 +83,31 @@ def test_the_sequence_is_one_and_the_session_id_empty(self): self.assertEqual(["var sequence = 1;"], line(script, "sequence")) self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + def test_an_empty_request_gives_the_same_defaults(self): + script = render({}) + self.assertEqual(["var sequence = 1;"], line(script, "sequence")) + self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + + def test_a_session_id_the_builder_would_accept_is_still_left_out(self): + """A session id that passes the builder's own check is rendered + empty, so the empty string comes from there being no Sequence + Element rather than from that check.""" + + script = render({"query.session-id": "a" * 64}) + self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + + def test_with_a_sequence_element_its_values_are_rendered(self): + """The same pipeline with a SequenceElement renders what that element + produced, which is the request's sequence counted on by one and the + session id it was given.""" + + script = render({"query.sequence": "5", + "query.session-id": "abc-123"}, + sequence_element=True) + self.assertEqual(["var sequence = 6;"], line(script, "sequence")) + self.assertEqual( + ['var sessionId = "abc-123";'], line(script, "sessionId")) + if __name__ == "__main__": unittest.main() diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py index db057848..f828458f 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py @@ -22,10 +22,10 @@ """ The script must parse whatever the pipeline holds. The sequence is written -into the script as bare code and the session id inside quotes, and a pipeline -built without a SequenceElement, for example from a configuration file that -does not list one, passes the query string's values, or nothing at all, -straight to the builder. +into the script as bare code and the session id inside quotes, and both come +from the SequenceElement, so every pipeline here has one. A pipeline without +a SequenceElement renders sequence 1 and an empty session id, which +test_javascriptbuilder_no_sequence.py covers. The script is checked with "node --check", which needs Node on the path. Every GitHub hosted runner has it. From 7fbda45929665419410e44bff13d0e3894f5a920 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Thu, 12 Nov 2026 08:41:22 +0000 Subject: [PATCH 4/4] Take the session id and the sequence from the Sequence Element Step 5 of the JavaScript Builder's process reads both values from the Sequence Element's Element Data, and renders 1 and an empty session id where there is no Sequence Element. This draft had the second half and read both values from Evidence, which is the half PHP, .NET and Java already have. The Sequence Element now publishes the pair it writes to Evidence as its own Element Data, and the builder reads it from there. The Evidence writes stay, because the JSON Builder and the Sequence Element's own evidence key filter read them. The case-folded Evidence lookup the builder used for these two keys goes with them, having no other reader. A key given in other letters is still read, because Evidence.get folds the key it is given and the Sequence Element reads through it. --- .../javascriptbuilder.py | 55 +++++------ .../fiftyone_pipeline_core/sequenceelement.py | 17 +++- .../test_javascriptbuilder_no_sequence.py | 93 +++++++++++++++++-- 3 files changed, 122 insertions(+), 43 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py index 844413ab..1b57a3f9 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/javascriptbuilder.py @@ -52,9 +52,11 @@ EXCLUDED_PARAMETERS = ["query.session-id", "query.sequence"] # The data key of the SequenceElement, which is the element that produces the -# session id and the sequence. A pipeline that does not hold it renders the -# defaults below. +# session id and the sequence, and the two names its Element Data holds them +# under. A pipeline without that element renders the defaults below. SEQUENCE_ELEMENT_DATA_KEY = "sequence" +SESSION_ID_NAME = "session-id" +SEQUENCE_NAME = "sequence" # The object name is written into the script as the name of a global # variable, a session storage key and a property name, all without any @@ -140,7 +142,7 @@ def parse_sequence(value): A sequence as a positive 32 bit integer. @type value: object - @param value: The query.sequence evidence, or None + @param value: A sequence value, from Evidence or Element Data, or None @rtype: int @return: The sequence, or None when the value is not a whole number from 1 to 2147483647. Any value at all can be passed, including one a page @@ -177,13 +179,11 @@ def get_sequence(value): The template writes the sequence as bare code (var sequence = ...;), so anything other than a whole number would stop the script parsing, or - change what it does. A pipeline without a SequenceElement passes the - query string's value, or nothing at all, straight through. As the - pipeline specification says, a value that is not a positive 32 bit - integer becomes 1. + change what it does. As the pipeline specification says, a value that is + not a positive 32 bit integer becomes 1. @type value: object - @param value: The query.sequence evidence, or None + @param value: The SequenceElement's sequence, or None @rtype: int @return: The sequence to render """ @@ -199,11 +199,10 @@ def get_session_id(value): The template writes the session id inside quotes without any escaping. As the pipeline specification says, a session id that is not 1 to 64 - ASCII letters, digits and hyphens is rendered as an empty string, whether - it came from the SequenceElement or from the query string. + ASCII letters, digits and hyphens is rendered as an empty string. @type value: object - @param value: The query.session-id evidence, or None + @param value: The SequenceElement's session id, or None @rtype: str @return: The session id to render, or an empty string """ @@ -421,28 +420,22 @@ def process_internal(self, flowdata): + self.settings["_objName"] + "' is used.") query_params = self.get_evidence_key_filter().filter_evidence(flowdata.evidence.get_all()) - # Evidence keys ignore letter case, and get_all gives each key in - # the letters it was added with, so a key is matched here in lower - # case and keeps those letters where it is sent on. - folded = {str(key).lower(): key for key in query_params} - - def query_param(name): - key = folded.get(name) - return None if key is None else query_params[key] - - # Both are written into the script, so each always has a safe value. - # The session id is written inside quotes and is empty when absent or - # not safe, and the sequence is written as bare code, so it is always - # a positive number. The Sequence Element writes both values, so - # with no Sequence Element in the pipeline the sequence is 1 and the - # session id empty, whatever the request asked for. - if SEQUENCE_ELEMENT_DATA_KEY in flowdata.pipeline.flow_elements_list: - variables["_sessionId"] = get_session_id( - query_param("query.session-id")) - variables["_sequence"] = get_sequence(query_param("query.sequence")) - else: + + # Both come from the Sequence Element's Element Data, and with no + # Sequence Element in the pipeline the sequence is 1 and the session + # id empty, whatever the request asked for. Both are written into the + # script, so each always has a safe value. The session id is written + # inside quotes and is empty when absent or not safe, and the sequence + # is written as bare code, so it is always a positive number. + sequence_data = flowdata.data.get(SEQUENCE_ELEMENT_DATA_KEY) + if sequence_data is None: variables["_sessionId"] = "" variables["_sequence"] = DEFAULT_SEQUENCE + else: + variables["_sessionId"] = get_session_id( + sequence_data.get(SESSION_ID_NAME)) + variables["_sequence"] = get_sequence( + sequence_data.get(SEQUENCE_NAME)) # The session id and the sequence are left out, because the script # appends both to its own request after it has taken the record of diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/sequenceelement.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/sequenceelement.py index 2aaf8283..ef61e62b 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/sequenceelement.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/sequenceelement.py @@ -23,6 +23,7 @@ from .flowelement import FlowElement from .evidence_keyfilter import EvidenceKeyFilter +from .elementdata_dictionary import ElementDataDictionary from .javascriptbuilder import parse_sequence import uuid @@ -71,13 +72,14 @@ def get_evidence_key_filter(self): def process_internal(self, flowdata): """! - The SequenceElement stores session data for requests for JavaScript - + The SequenceElement stores session data for requests for JavaScript, + in Evidence and in its own Element Data + @type flowdata: FlowData @param flowdata: The FlowData """ - + if flowdata.evidence.get("query.session-id"): # Get current sequence number. A value that is absent or is not @@ -96,5 +98,12 @@ def process_internal(self, flowdata): "query.session-id", str(uuid.uuid4()) ) - + flowdata.evidence.add("query.sequence", 1) + + # The JavaScript Builder takes both values from here rather than from + # Evidence, which is what step 5 of its specification asks for. The + # values are the ones just written, so the two carry the same pair. + flowdata.set_element_data(ElementDataDictionary(self, { + "session-id": flowdata.evidence.get("query.session-id"), + "sequence": flowdata.evidence.get("query.sequence")})) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py index 17f3be39..82917e2b 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_no_sequence.py @@ -20,11 +20,12 @@ # such notice(s) shall fulfill the requirements of that article. # ********************************************************************* -"""With no Sequence Element in the pipeline, the JavaScript Builder renders -the sequence as 1 and the session id as an empty string, as step 5 of the -JavaScript Builder specification's process says, whatever the evidence -holds. The same pipeline with a Sequence Element renders that element's -values, which is the other side of the same branch.""" +"""Step 5 of the JavaScript Builder specification's process takes the session +id and the sequence from the Sequence Element's Element Data, and where there +is no Sequence Element renders the sequence as 1 and the session id as an +empty string, whatever the evidence holds. Both sides of that branch are +tested here, and so is the source the values are read from, because the +builder used to read them from Evidence instead.""" import re import unittest @@ -49,9 +50,28 @@ def process_internal(self, flowdata): self, {"json": {"javascriptProperties": []}})) -def render(evidence, sequence_element=False): - """Render the script from a pipeline that differs only in whether it - holds a SequenceElement. The real JSON bundler refuses a pipeline with no +class StandInSequenceElement(FlowElement): + """Takes the place of the SequenceElement, writing one pair into its + Element Data and a different pair into Evidence, so that a test can tell + which of the two the builder read. Element Data of None stands for an + element that is in the pipeline and produced nothing.""" + + def __init__(self, contents): + super(StandInSequenceElement, self).__init__() + self.datakey = "sequence" + self.contents = contents + + def process_internal(self, flowdata): + flowdata.evidence.add("query.session-id", "from-evidence") + flowdata.evidence.add("query.sequence", 99) + if self.contents is not None: + flowdata.set_element_data( + ElementDataDictionary(self, self.contents)) + + +def render(evidence, sequence_element=False, stand_in=None): + """Render the script from a pipeline that differs only in which Sequence + Element it holds, if any. The real JSON bundler refuses a pipeline with no sequence number, so a stand-in gives the builder its JSON.""" builder = PipelineBuilder({ @@ -59,6 +79,8 @@ def render(evidence, sequence_element=False): "use_setheader_properties": False}) if sequence_element: builder = builder.add(SequenceElement()) + if stand_in is not None: + builder = builder.add(stand_in) pipeline = builder \ .add(StandInJsonBuilder()) \ .add(JavascriptBuilderElement({"minify": False})) \ @@ -109,5 +131,60 @@ def test_with_a_sequence_element_its_values_are_rendered(self): ['var sessionId = "abc-123";'], line(script, "sessionId")) +class WhereTheValuesComeFromTests(unittest.TestCase): + + def test_the_element_data_is_read_and_the_evidence_is_not(self): + """The stand-in puts one pair in its Element Data and a different + pair in Evidence. What is rendered says which of the two was read.""" + + script = render({}, stand_in=StandInSequenceElement( + {"session-id": "from-element-data", "sequence": 7})) + self.assertEqual(["var sequence = 7;"], line(script, "sequence")) + self.assertEqual( + ['var sessionId = "from-element-data";'], + line(script, "sessionId")) + + def test_an_element_that_produced_no_element_data_gives_the_defaults(self): + """A Sequence Element that is in the pipeline but set no Element + Data, because it stopped or failed, is the same as no Sequence + Element at all, so the script still parses.""" + + script = render({}, stand_in=StandInSequenceElement(None)) + self.assertEqual(["var sequence = 1;"], line(script, "sequence")) + self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + + def test_an_unusable_pair_in_the_element_data_is_rendered_safely(self): + """The session id is written inside quotes and the sequence as bare + code, so a session id that does not match the pattern is rendered + empty and a sequence that is not a positive 32 bit integer is 1.""" + + script = render({}, stand_in=StandInSequenceElement( + {"session-id": 'a" + alert(1) + "', "sequence": "not a number"})) + self.assertEqual(["var sequence = 1;"], line(script, "sequence")) + self.assertEqual(['var sessionId = "";'], line(script, "sessionId")) + + def test_the_sequence_element_publishes_the_pair_it_wrote(self): + """The real SequenceElement's Element Data holds the same session id + and sequence it put in Evidence, which is what the builder reads.""" + + builder = PipelineBuilder({ + "add_javascript_builder": False, + "use_setheader_properties": False}) + pipeline = builder.add(SequenceElement()).build() + flowdata = pipeline.create_flowdata() + flowdata.evidence.add("query.session-id", "abc-123") + flowdata.evidence.add("query.sequence", "5") + flowdata.process() + + self.assertEqual("abc-123", flowdata.sequence.get("session-id")) + self.assertEqual(6, flowdata.sequence.get("sequence")) + self.assertEqual( + flowdata.evidence.get("query.session-id"), + flowdata.sequence.get("session-id")) + self.assertEqual( + flowdata.evidence.get("query.sequence"), + flowdata.sequence.get("sequence")) + + if __name__ == "__main__": unittest.main()