From d7d792f08af88c87b377d4b748ed917811018932 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 21:01:46 +0000 Subject: [PATCH 1/3] Test that the JSON Builder needs a sequence number --- .../test_jsonbundler_sequence_required.py | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 pipeline/fiftyone_pipeline_core/tests/test_jsonbundler_sequence_required.py diff --git a/pipeline/fiftyone_pipeline_core/tests/test_jsonbundler_sequence_required.py b/pipeline/fiftyone_pipeline_core/tests/test_jsonbundler_sequence_required.py new file mode 100644 index 0000000..06396cf --- /dev/null +++ b/pipeline/fiftyone_pipeline_core/tests/test_jsonbundler_sequence_required.py @@ -0,0 +1,78 @@ + +""" +The JSON Builder fails with an error, and sets no JSON, when no Sequence +Element has produced a sequence number, as the JSON Builder specification +says. +""" + +import unittest + +from fiftyone_pipeline_core.jsonbundler import JSONBundlerElement +from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder + + +def bundler_only(suppress=False): + builder = PipelineBuilder({ + "add_javascript_builder": False, + "use_setheader_properties": False, + "suppress_process_exceptions": suppress}) + return builder.add(JSONBundlerElement()).build() + + +class JsonBuilderSequenceTests(unittest.TestCase): + + def test_no_sequence_number_is_an_error_that_names_the_sequence_element( + self): + pipeline = bundler_only() + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + with self.assertRaises(Exception) as caught: + flowdata.process() + self.assertIn("Sequence Element", str(caught.exception)) + + def test_no_json_is_set_when_the_error_is_suppressed(self): + pipeline = bundler_only(suppress=True) + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertEqual(1, len(flowdata.errors)) + self.assertNotIn("jsonbundler", flowdata.data) + + def test_a_sequence_number_from_the_query_string_does_not_count(self): + for value in ("3", "", "abc", True, 3.0): + pipeline = bundler_only(suppress=True) + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + flowdata.evidence.add("query.sequence", value) + flowdata.process() + self.assertEqual(1, len(flowdata.errors), repr(value)) + self.assertNotIn("jsonbundler", flowdata.data, repr(value)) + + def test_an_integer_sequence_number_in_the_evidence_counts(self): + pipeline = bundler_only() + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + flowdata.evidence.add("query.sequence", 3) + flowdata.process() + self.assertIn("javascriptProperties", flowdata.jsonbundler.json) + + def test_the_default_pipeline_has_a_sequence_number(self): + pipeline = PipelineBuilder({}).build() + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + flowdata.process() + self.assertEqual(0, len(flowdata.errors)) + self.assertIn("javascriptProperties", flowdata.jsonbundler.json) + + def test_a_sequence_of_ten_or_more_leaves_out_the_javascript_properties( + self): + pipeline = bundler_only() + self.addCleanup(pipeline.close) + flowdata = pipeline.create_flowdata() + flowdata.evidence.add("query.sequence", 10) + flowdata.process() + self.assertEqual([], flowdata.jsonbundler.json["javascriptProperties"]) + + +if __name__ == "__main__": + unittest.main() From d1294e25fe04d5cebbaf5517f005e06032846518 Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 21:02:44 +0000 Subject: [PATCH 2/3] Fail the JSON Builder when no Sequence Element has produced a sequence number --- .../src/fiftyone_pipeline_core/jsonbundler.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/jsonbundler.py b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/jsonbundler.py index 9f2a8ca..201decf 100644 --- a/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/jsonbundler.py +++ b/pipeline/fiftyone_pipeline_core/src/fiftyone_pipeline_core/jsonbundler.py @@ -57,6 +57,16 @@ def process_internal(self, flowdata): """ + # The sequence number is the one the Sequence Element has put in the + # Evidence as a whole number. Text in the query string does not + # count, and without a number no JSON is set. + sequence = flowdata.evidence.get("query.sequence") + if isinstance(sequence, bool) or not isinstance(sequence, int): + raise Exception( + "The JSON Builder needs the sequence number that a Sequence " + "Element produces. A Sequence Element needs to be placed " + "before it in the Pipeline.") + # Get every property on every FlowElement # Storing JavaScript properties in an extra section @@ -178,9 +188,7 @@ def process_internal(self, flowdata): if value is None: output[flow_element.datakey.lower()][propertykey.lower() + "nullreason"] = null_reason - sequence = flowdata.evidence.get("query.sequence") - - if sequence is None or sequence < 10: + if sequence < 10: if "type" in propertymeta and propertymeta["type"].lower() == "javascript": From 841d84c3f1903f048c64d1ae76d18fc0cc39913e Mon Sep 17 00:00:00 2001 From: James Rosewell Date: Mon, 9 Nov 2026 21:43:07 +0000 Subject: [PATCH 3/3] Build the JavaScript script tests on a pipeline with a Sequence Element The JSON Builder fails without a sequence number, so a pipeline without a Sequence Element no longer reaches the script. The builder's reading of text values stays covered by the get_sequence tests. --- .../tests/test_javascriptbuilder_sequence.py | 93 +++---------------- 1 file changed, 13 insertions(+), 80 deletions(-) diff --git a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py index 2e42c30..db05784 100644 --- a/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py +++ b/pipeline/fiftyone_pipeline_core/tests/test_javascriptbuilder_sequence.py @@ -41,25 +41,19 @@ from parameterized import parameterized from fiftyone_pipeline_core import javascriptbuilder -from fiftyone_pipeline_core.javascriptbuilder import JavascriptBuilderElement -from fiftyone_pipeline_core.jsonbundler import JSONBundlerElement from fiftyone_pipeline_core.pipelinebuilder import PipelineBuilder NODE = shutil.which("node") -def _render(evidence=None, minify=False, sequence_element=False): - """Render the script. With sequence_element False the pipeline has only - the JSON bundler and the JavaScript builder.""" +def _render(evidence=None, minify=False, sequence_element=True): + """Render the script. The JSON builder fails without a sequence number, + so the pipeline always has the SequenceElement.""" - settings = {"javascript_builder_settings": {"minify": minify}} - if not sequence_element: - settings["add_javascript_builder"] = False - builder = PipelineBuilder(settings) if not sequence_element: - builder.add(JSONBundlerElement()) - builder.add(JavascriptBuilderElement({"minify": minify})) - pipeline = builder.build() + raise ValueError("The JSON builder needs the SequenceElement") + settings = {"javascript_builder_settings": {"minify": minify}} + pipeline = PipelineBuilder(settings).build() flowdata = pipeline.create_flowdata() for key, value in (evidence or {}).items(): flowdata.evidence.add(key, value) @@ -92,54 +86,15 @@ def assert_parses(self, script): "the rendered script does not parse: " + result.stderr) @parameterized.expand([["unminified", False], ["minified", True]]) - def test_script_parses_without_sequence_element(self, _, minify): - """With no SequenceElement and no query evidence the script is - rendered with sequence 1 and an empty session id, and parses.""" + def test_script_parses(self, _, minify): + """With no query evidence the script is rendered with sequence 1, and + parses.""" script = _render(minify=minify) self.assert_parses(script) if not minify: self.assertEqual(["var sequence = 1;"], _line(script, "sequence")) - self.assertEqual( - ['var sessionId = "";'], _line(script, "sessionId")) - - @parameterized.expand([ - ["text", "abc"], - ["negative", "-1"], - ["zero", "0"], - ["too_large", "99999999999"], - ["empty", ""], - ["statement", "1;b"], - ["decimal", "1.5"], - ["underscore", "1_0"], - ["just_too_large", "2147483648"], - ["file_separator", chr(0x1C) + "5"], - ["group_separator", chr(0x1D) + "5"], - ["record_separator", chr(0x1E) + "5"], - ["unit_separator", chr(0x1F) + "5"], - ["trailing_file_separator", "5" + chr(0x1C)], - ["null", chr(0) + "5"], - ["no_break_space", chr(0xA0) + "5"], - ["line_separator", chr(0x2028) + "5"], - ["arabic_indic_digit", chr(0x665)], - ["full_width_digit", chr(0xFF15)], - ["five_thousand_digits", "9" * 5000], - ["plus_alone", "+"], - ["spaces_alone", " "], - ["zeros", "0000"], - ]) - def test_unusable_sequence_from_query_becomes_one(self, _, value): - """A query.sequence that is not a positive 32 bit integer is rendered - as 1, as the pipeline specification says, so the script parses and - nothing else is written in its place.""" - - script = _render({ - "query.session-id": "abc", - "query.sequence": value}) - - self.assertEqual(["var sequence = 1;"], _line(script, "sequence")) - self.assert_parses(script) @parameterized.expand([ ["text", "abc"], @@ -171,30 +126,8 @@ def test_unusable_sequence_with_sequence_element_becomes_one( ['var sessionId = "abc";'], _line(script, "sessionId")) self.assert_parses(script) - @parameterized.expand([ - ["text", "7", 7], - ["spaces", " 7 ", 7], - ["number", 12, 12], - ["largest", "2147483647", 2147483647], - ]) - def test_usable_sequence_from_query_is_rendered(self, _, value, expected): - script = _render({ - "query.session-id": "abc", - "query.sequence": value}) - - self.assertEqual( - ["var sequence = " + str(expected) + ";"], - _line(script, "sequence")) - self.assert_parses(script) - - @parameterized.expand([ - ["without_sequence_element", False], - ["with_sequence_element", True], - ]) - def test_session_id_from_query_is_rendered(self, _, sequence_element): - script = _render( - {"query.session-id": "abc-123"}, - sequence_element=sequence_element) + def test_session_id_from_query_is_rendered(self): + script = _render({"query.session-id": "abc-123"}) self.assertEqual( ['var sessionId = "abc-123";'], _line(script, "sessionId")) @@ -223,7 +156,7 @@ def test_longest_session_id_is_rendered(self): ["space", "a b", False], ["new_line", "ab\n", False], ] - for element in [False, True] + for element in [True] ]) def test_unsafe_session_id_is_rendered_empty( self, _, value, check_text, sequence_element): @@ -331,7 +264,7 @@ def test_any_sequence_value_reaches_the_script(self): chr(0x1F) + "5", "5" + chr(0x1C), chr(0) + "5", "9" * 5000, "+", " "] for value in values: - for element in [False, True]: + for element in [True]: with self.subTest(value=repr(value), element=element): script = _render( {"query.session-id": "abc",