[jwrosewell]

Two places where the JSON Builder and the JavaScript Builder differ from the specification, found by a conformance check of this repository against specifications at 706de1f0.

1. A Property assigned no value was left out of the JSON

json-builder.md lines 47 to 53 say:

A Property that has been assigned 'no value' MUST be written as 'null', with the nullreason meta-Property in the table below. A Property whose value is null without having been assigned 'no value', which a Flow Element that is not an Engine can produce, MUST have no entry of its own in the Element Data's object, and no nullreason.

and lines 74 to 78 say the same of each item of a list of Element Data:

Where a Property's value is a list of Element Data, such as profiles in the hardware profile lookup, each item MUST be written as an object whose Properties follow the rules above for a Property that has been assigned 'no value' and for one whose value is null.

getValues puts a no-value Property into its map as null beside its nullreason, but buildJson hands each map to org.json, whose JSONObject(Map) leaves out every entry whose value is null. So the JSON carried <property>nullreason and no <property> at all, at the top level and in each item of a list of Element Data. That is how the specification has a plain null written, so a Property assigned no value looked like a plain null with a stray nullreason.

buildJson now writes each Element Data's object itself, with a null value written as JSON null, and toJsonValue does the same for any map inside it, such as an item of a list of Element Data. Every other value is wrapped as org.json wraps it, so nothing else in the JSON changes. A Property whose value is null without being assigned no value still has no entry, because getValues never puts one.

2. The JavaScript Builder's constructor accepted any protocol

javascript-builder.md lines 206 to 218 say:

1. The protocol MUST be http or https, compared ignoring ASCII case only [...] A configured protocol that is neither, including one made only of white space, MUST be refused when the element is built, using the language's usual configuration error. An empty configured protocol means that none is configured, which is the default under Configuration options, so step 1 takes the protocol from Evidence, but a builder method that sets the protocol MAY refuse an empty one.

JavaScriptBuilderElementBuilder.setProtocol refuses such a protocol, but the public constructors stored whatever they were given, and the element refused it only on each request. The constructor now throws PipelineConfigurationException with the builder's message for a protocol that is not null, not empty and not http or https in any case of its ASCII letters, white space included. Null or empty still means none is configured, and the builder, the constructor's only caller in the package, passes an empty string when no protocol is set. The constructors' Javadoc said "null or blank" means none is configured, and now says "null or empty".

Tests

Tests first, in 8f35be03.

• JsonBuilderTests.JsonBuilder_NoValue_WrittenAsNullWithReason gives the element a Property assigned no value with a message, a plain null and a string, and expects the first as null with its nullreason, the plain null with no entry and no nullreason, and the string unchanged.
• JsonBuilderTests.JsonBuilder_ListOfElementData_NoValueWrittenAsNull gives a list of one Element Data whose first Property has no value, and expects it written as null with its nullreason in the list's object.
• JavaScriptBuilderElementBuilderTests.JavaScriptBuilderElement_Constructor_InvalidProtocol_Refused expects htp, ftp, a space, a tab and https: refused by the constructor.
• JavaScriptBuilderElementBuilderTests.JavaScriptBuilderElement_Constructor_ValidOrNoProtocol_Accepted expects null, an empty string, http, https, HTTPS and Http accepted.

Red 43615866119 on 8f35be0 plus a throwaway workflow, Java 17, stopped after JsonBuilderTests failed exactly the two new tests (2 of 17), so the JavaScript Builder module did not run. A second red, 43616249012, on the same commit with test failures ignored so that both modules report, gives JsonBuilderTests 2 of 17 failing, the two new tests with "expected: but was: " for the null entry, and JavaScriptBuilderElementBuilderTests 5 of 25 failing, the five refused protocols with "Expected fiftyone.pipeline.core.exceptions.PipelineConfigurationException to be thrown, but nothing was thrown", while the accepted protocols pass.

Green 43615866171 on the fix 3099adef plus the same workflow. JsonBuilderTests 17 of 17 and JavaScriptBuilderElementBuilderTests 25 of 25.

Full build

Run 43617671777 built 3099ade merged with rc (b33daa0) on nine configurations, Ubuntu x64 with Java 8, 11, 17 and 21, Ubuntu arm64 with Java 17, and Windows with Java 8, 11, 17 and 21, and every job passed. JsonBuilderTests passed 17 of 17 and JavaScriptBuilderElementBuilderTests 25 of 25 in all nine jobs. Each of its ten Setting 'PullRequestSha' lines names 3099ade, and its Configure job printed "Left out 6 macOS entries".

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

Before you upgrade

• The JSON now has an entry "<property>": null beside each <property>nullreason, at the top level and in each item of a list of Element Data, where it had only the reason. Code that took a missing key to mean the Property has no value now finds the key with a null value, and should read the null, or the nullreason, as no value.
• A JavaScriptBuilderElement made with one of its public constructors and a protocol that is not http or https, white space included, now throws PipelineConfigurationException when it is made. Before, the element was made, and on each request it produced the script with no callback URL and logged an error the first time. JavaScriptBuilderElementBuilder already refused such a protocol, so only code that calls the constructors directly sees the change. A null or empty protocol still means none is configured.

What it releases

pipeline.jsonbuilder and pipeline.javascriptbuilder change, and both are in Monday's plan under version per package (#63) already.

Base branch

rc at b33daa0, and a trial merge of the head with rc at de74c8c is clean. #135 changes other lines of JsonBuilderElement.java and JsonBuilderTests.java, and a trial merge of the two at #135's head bb1d8da is clean.