[jwrosewell]

Problem

The JSON Builder wrote delayexecution for every property whose metadata sets the delay execution flag, before it read the value (jsonbundler.js line 168 at 24592acb). So a property with no value, and one that could not be read at all, were marked too. The cross-language audit found this, and PHP (51degrees-php#61), Go (51degrees-go#91) and Python (51degrees-python#92) have already merged the same change.

The bundler also kept each element's delayed properties under the wrong key. jsonbundler.js lines 157 and 163 at c1dd5683 stored and read them as this.propertyCache[flowElement.dataKey], but flowElement is the key itself, a string from pipeline.flowElements (pipeline.js line 85), so every element was stored under undefined. The first request was right, because each element wrote its list and read it straight back. From the second request every element read the last element's list, so every other element lost delayexecution and evidenceproperties.

What the specification says

JSON Builder, line 71 at 77dc1f62:

delayexecution: This meta-Property MUST be added with the value 'true' for each Property that has a value and whose metadata delay execution flag is true.

What changed

In jsonbundler.js lines 165 to 213 the value is read first, and delayexecution is written only when the property was read and its value is not null or undefined. The meta-properties are written in the same order as before (delayexecution, then evidenceproperties, then the value), so the JSON of a property that keeps delayexecution is unchanged.

evidenceproperties is still written for a property whose value could not be read. Line 72 says only when it MUST be added, being when the JSON includes delayed Properties from this Property's Evidence Properties list, and does not ask for it to be left out otherwise. Go writes it either way, "as the .NET and Java JSON Builders do, so the page can still run the JavaScript" (jsonbuilder.go lines 141 to 160 at 88a98b77), so Node keeps doing the same.

3cb500e2 keys the cache on the element's key, this.propertyCache[flowElement], in both places. c1dd5683 merges rc at 24592ac with no conflict.

Tests

tests/javascriptbuilder.test.js, "JSON bundler - delayexecution only for a property with a value". Three delayed properties are given a value, no value with a reason, and a null raw value. Only the first is marked, and the second keeps its nullreason. The existing delayed execution tests, which compare the JSON text in order, pass unchanged.

"JSON bundler - each element keeps its own delayed properties on later requests" builds one Pipeline with two elements, each with a delayed property and a property whose Evidence Properties name it, both named after the element's key, and processes two requests. Each request must mark each element's own delayed property and list its own evidence property.

• Red, the test on rc code at the time, 1 of 398 failed (twodelayexecution was true): run 42897495445.
• Green, lint and the core and cloud request engine tests on 26acbf9, 398 passed: run 42897530867.
• Red for the cache key, both tests on rc at 24592ac without either fix: run 43489758674, 2 of 1,144 failed. "each element keeps its own delayed properties on later requests" failed on request 2 for first, whose firstscriptdelayexecution was undefined, and "delayexecution only for a property with a value" failed as before.
• Green on 3cb500e: run 43489743052, 1,144 passed and lint passed. The run's conclusion is failure, from its last step, npm run tsc, which regenerates the types and compares them with the committed ones. That comparison fails on rc without this change too, because the committed types are older than the code (run 43485072238 found 23 type files that tsc writes differently on rc at 24592ac), so it is not a test of this change.
• Full build on the head 3cb500e: run 43490166786 passed. All seven Setting 'PullRequestSha' lines are 3cb500e, and each of the six Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 ran 1,358 unit tests passed with 2 skipped, and 132 integration tests passed with 4 skipped. 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.
• Full build on c1dd568, before the cache key fix: run 43477038884 passed. All seven Setting 'PullRequestSha' lines are c1dd568, and each of the six Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 ran 1,356 unit tests passed with 2 skipped, and 132 integration tests passed with 4 skipped.

What remains

Nothing in this change. No other language's JSON Builder has the same cache fault.

• Python (jsonbundler.py lines 124 and 163 at 32ffdcf1) and PHP (JsonBundlerElement.php lines 122 and 164 at a074012f) loop over the element objects and key the cache on each one's own data key, as the review of 13 November at 22:55 UTC found.
• .NET (JsonBuilderElement.cs lines 779 to 791 at d202a66a) and Java (JsonBuilderElement.java lines 334 to 344 at 30e48867) keep one configuration for each Pipeline, holding each delayed Property under its complete name, such as device.ismobile.
• Go (jsonbuilder.go lines 113 to 130 at 88a98b77) builds its index of the metadata on each request, under each Property's complete name.