Repository navigation
Add switches and paths to the web integration, and Element Data to the Sequence Element - #80
Merged
Merged
Conversation
…e sharing characters The tests cover the init parameters that set the JavaScript and JSON paths and switch the Sequence Element and the Set Headers Element, the Element Data the Sequence Element writes, how the JSON Builder Element shows that data, and the characters XML 1.0 allows in a shared Evidence value.
…e Sequence Element The filter takes init parameters for the JavaScript path, the JSON path, and whether the Sequence Element and the Set Headers Element are added. The defaults keep today's paths and today's behaviour. The Sequence Element writes session-id and sequence as Element Data as well as Evidence. Usage sharing keeps every character XML 1.0 allows instead of replacing everything above U+00FF.
… say what the Lite file lacks The MVC configuration takes the JavaScript path, the JSON path and the two element switches the filter takes, with defaults that keep today's behaviour. The example comments about the Lite data file say it has a small number of Properties, as the specification does.
Java 21 warns about it, and the build treats warnings as errors.
…th as well The specification's JSON path becomes the default in the filter and the MVC integration, and the JavaScript they serve calls it. The path 51Degrees.core.json is still answered, and json-path still sets the path.
…tion says A session id that is not a non-empty string, and a sequence that is not a whole number or text of decimal digits that fits in an int with room to add one, count as absent. The Evidence and the Element Data are written from the same values.
# Conflicts: # pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/SequenceElement.java
# Conflicts: # pipeline/pipeline.engines.fiftyone/src/main/java/fiftyone/pipeline/engines/fiftyone/flowelements/ShareUsageElement.java # pipeline/pipeline.engines.fiftyone/src/test/java/fiftyone/pipeline/engines/fiftyone/flowelements/ReplacedStringTest.java
…nt is refused The json-path test uses a path that is not the default.
The JSON Builder Element fails on every request without the sequence number, so the build says which option to change.
… properties are off
- FiftyOneJSServiceCoreTests checks that a configured path, with or without its leading slash, answers only that path inside the application, so /anything/json, /json/more and /notjson are not answered, and that the context path is not part of the path. - SequenceElementTests checks that a negative sequence of any whole number type, a sign and white space each count as no sequence, and that 0 gives 1.
…uence as absent FiftyOneJSServiceCore.Default answered any request whose whole URL ended with the configured path, so a configured path of json also answered /anything/json. The path is now compared with the whole path of the request inside the application, after the context path, ignoring case and with or without a leading slash, and a request with no URI is not answered. The old 51Degrees.core.json path is matched the same way. The Sequence Element took a negative whole number as a sequence. The Sequence Element specification counts a sequence only from 0 up to 2,147,483,646, so a negative one now counts as absent.
The weighted values test that rc adds at the end of JsonBuilderTests and this draft's Sequence Element test sit in the same place, and both are kept.
jwrosewell
marked this pull request as ready for review
November 11, 2026 12:17
This was referenced Nov 11, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
pipeline/Gaps-vs-spec.mdlists where the Java pipeline differed from the specification. Checked against the code onrcand against the web integration page, these are still true:PipelineFilterreads one init parameter (clientside-properties-enabled). The JavaScript path (51Degrees.core.js) and JSON path (51Degrees.core.json) are constants.SequenceElementwrites Evidence only and declares no properties. The specification lists Element Data as well.Item 10 (Drift as a sum) is already fixed by #60, so it is not repeated here.
The specification is quoted at commit ba079423 of
specificationsrc.What changes
W1. The filter takes two new init parameters,
javascript-pathandjson-path. Intercepted Urls: "These SHOULD be configurable, but the default values are:/51Degrees.core.js.../51dpipeline/json". The JavaScript path default stays51Degrees.core.js. The JSON path default becomes the specification's/51dpipeline/json, as .NET and Go use, and the JavaScript the filter serves calls it. The old path51Degrees.core.jsonis still answered, whateverjson-pathsays, so a site or a cached script that calls it keeps working.FiftyOneJSServiceCore.Defaultgains a constructor that takes the two paths, and the existing constructor uses the defaults.Constants.CORE_JSON_NAMEstays, for the old path, andConstants.DEFAULT_JSON_PATHis new.W2. The filter takes
sequence-element-enabledandset-headers-element-enabled, both true by default. Pipeline configuration: "For all of these elements, there MUST be configuration options to allow users to prevent them being added (or disable them) if they wish to do so." A false value stops the element being added when the configuration lacks it, and an element named in the configuration stays. With client-side properties on and the Sequence Element left out the build is refused, naming the option, as .NET does, because the JSON Builder Element fails on every request without the sequence number (json-builder.md).clientside-properties-enabledworks as before and still controls only the JSON Builder Element and the JavaScript Builder Element.StartupHelpers.buildFromConfigurationgains an overload with the JSON path and the two switches, and the existing overloads call it with the defaults.5b.
SequenceElementalso writes the Element Datasession-idandsequence, under the keysequence, as .NET, Go and Python do, and declares them as properties. The page's Element Data section gives the key: "The Element Data key issequence." Sequence Element: the Element Data table lists both, and "Add both output values to Evidence as well" keeps the Evidence writes, which are unchanged. New classSequenceData. Process treats the two values on their own, so a session id that is not a non-empty string counts as absent and gets a new id, and a sequence counts only if it is a whole number from 0 up, or text made only of the digits 0 to 9, up to 2,147,483,646 so that adding one fits in an int, and otherwise gives 1, with no error and no overflow. specifications#127, now merged, states both rules in that section: "Aquery.session-idvalue that is not a non-empty string is not present", so an empty session id gets a new one, and "Aquery.sequencevalue that is not a whole number, or a string made only of the digits 0 to 9 with no sign or spaces, from 0 up to 2,147,483,646 is not present". A negative whole number used to be taken as a sequence, and now counts as absent. An id of "0" is kept, a sequence with no session id counts up with a new id, and a session id with no sequence gives 1. The Evidence and the Element Data are written from the same values. Java used to cast the sequence with(int), which failed on text.MVC integration.
FiftyOneInterceptorConfiggainsgetJavaScriptPath,getJsonPath,getSequenceElementEnabledandgetSetHeadersElementEnabled, as default methods so existing implementations keep working, andFiftyOneInterceptorConfigDefaultgains the setters. The interceptor passes them toStartupHelpersand to the newFiftyOneJSService.setPaths, andFiftyOneInterceptor.enableClientsidePropertieshas an overload that maps the configured paths. The JSON default is the same new default as the filter's, the old path is answered and mapped too, and both elements are on.Example comments. The comments in
GettingStartedOnPrem.java,MetadataOnPrem.javaandOfflineProcessing.javathat said the Lite file has "limited accuracy" now say it has a small number of Properties, as required-examples.md says ("the Lite data file has a small number of Properties"), and so does the one inUpdateDataFile.java. These are comments only, so there is no test. A search found no other comment with that wording.Usage sharing filter.
EvidenceKeyFilterShareUsage.includereturns false forserver.http-requestbefore any other rule, as .NET's filter does, so the whole web request that draft Accept no Evidence for usage sharing, and give it the whole web request #68 added is never a value to share. The review of Accept no Evidence for usage sharing, and give it the whole web request #68 asked for this.Exact paths.
FiftyOneJSServiceCore.Defaultanswered any request whose whole URL ended with the configured path, so a configured path ofjsonalso answered/anything/json. The path is now compared with the whole path of the request inside the application, after the context path, ignoring case and with or without a leading slash, and a request with no URI is not answered. This compares the request path rather than the whole URL, as .NET comparesRequest.Path, which leaves out the application's base path. .NET still usesEndsWithon that path, so the comparison here is stricter than .NET's. Intercepted Urls names the two URLs the integration intercepts and says nothing of longer paths that end the same way. The old51Degrees.core.jsonpath is matched the same way, and the scriptrcserved called it at the root.No JSON path from the caller. The new overload of
StartupHelpers.buildFromConfigurationreadjsonPathwithout checking it, so a caller that passed null got a null pointer exception rather than a pipeline. A null or empty path is nowConstants.DEFAULT_JSON_PATH, which is what the filter and the MVC interceptor already pass in that case, so the script still calls back to a path the JSON is served from.Does 5b add a section to the JSON sent to browsers?
Yes. The JSON Builder Element skips only the element data keys
cloud-response,json-builderandset-headers, so the Sequence Element's data now appears:JsonBuilder_SequenceElementData_InJsoninJsonBuilderTestsruns a Sequence Element and a JSON Builder Element in one Pipeline and reads that section. This follows json-builder.md, whose Process section says "There MUST be a top-level entry for each Element Data" and whose exclusion list names only the Set Headers Element, the Cloud Request Engine, the Usage sharing element and the JSON Builder Element itself. The Sequence Element is not on that list, so its entry, the session id included, reaches the page as the specification says it should.Release note
The JSON that 51Degrees client-side properties read is now served from
/51dpipeline/json, the path the specification names. The old path/51Degrees.core.jsonstill works, so no change is needed to a page, a cached script or a proxy rule. A site that wants another path setsjson-pathin the filter, orgetJsonPathin the MVC configuration. The Sequence Element's data now appears in that JSON undersequence, and the filter and the MVC integration can leave the Sequence Element and the Set Headers Element out.Behaviour that changes for callers
/51dpipeline/json, and the script the filter and the MVC integration serve calls that path.51Degrees.core.jsonis still answered. A site that setsjson-pathkeeps its choice.sequencesection above.51Degrees.core.jsor51Degrees.core.json, so a page at/products/phones.htmlcarrying<script src="51Degrees.core.js">asked for/products/51Degrees.core.jsand got the script. The paths are now matched whole, after the context path, ignoring case and a leading slash, because the specification names them as paths, Intercepted Urls: "/51Degrees.core.js- Serves the JavaScript produced by JavaScript Builder Element." Such a page links/51Degrees.core.jsfrom the root instead, and this belongs in the release note.SequenceData, the extraStartupHelpers.buildFromConfigurationoverload, the extraFiftyOneJSServiceCore.Defaultconstructor, the init parameter names inConstants, and the two properties onSequenceElement.Other open drafts
Checked with
git merge-treeof this head against every open draft's head.SequenceElementis kept, and this draft'stoSequencereplaces itsgetSequence. Write the usage sharing record as the specification says #81'sReplacedStringis kept as it is. Write weighted values with the lower-case keys rawweighting and value #93 added a test at the end ofJsonBuilderTests, where this draft adds one, and the merge ofrc(dd3bd92,rcat ff6e4c0) keeps both.EvidenceKeyFilterShareUsage.include. Whichever merges second keeps Add the usage sharing options to share all Evidence and all query string parameters #83'stoLowerCase(Locale.ROOT)and this draft'sserver.http-requesttest ahead of Add the usage sharing options to share all Evidence and all query string parameters #83'sshareAllEvidencereturn, so that sharing all Evidence never sends the whole web request.PipelineFilter,Constants,FiftyOneInterceptorConfig,FiftyOneInterceptorConfigDefaultandFiftyOneInterceptorTests. Both add init parameters and options side by side, so whichever merges second keeps both. Cache the client-side endpoints' answers as the specification and RFC 9110 ask #67 also conflicts withrcitself.rcitself, not with this draft.How it was checked
Tests were written first and fail on
rc.PipelineFilterTestschecks the elements the filter adds with each init parameter, and the callback endpoint.FiftyOneJSServiceCoreTestsis new. It checks the default and configured paths. It does not compile onrcbecause it uses the new constructor, so the web module's tests fail to build there.SequenceElementTestsis new, andJsonBuilderTestsgains the JSON test above.FiftyOneInterceptorTestshas four new tests for the MVC elements and paths, andNoSequence.xmlis a configuration without a Sequence Element.SequenceElementTests, the newJsonBuilderTeststest, and 3 of 9PipelineFilterTestsfail. In the first run the web module does not compile because of the new constructor, which the second run avoids by leavingFiftyOneJSServiceCoreTestsoutFiftyOneInterceptorTests(7),FiftyOneJSServiceCoreTests(4) andPipelineFilterTests(9) pass on Ubuntu and WindowssequenceSequenceElementTestsand theJsonBuilderTeststest fail looking for the keysequenceSequenceElementTestsfailJsonBuilderTestsandShareUsageElementTestswebRequestIsNeverIncludedfailsSequenceElementTests(11) andShareUsageElementTests(22)SessionIdAndSequenceTestsexpected an empty session id to be kept, and the Sequence Element now creates a new one. The test expects that nowa6303a4crcwithout the fix. On Ubuntu and Windows with Java 8 and 21,SequenceNegativeSignedOrSpaced_CountsAsAbsent(1 of 13SequenceElementTests) andConfiguredPath_OnlyAnExactMatchIsAnsweredandContextPath_IsNotPartOfThePath(2 of 7FiftyOneJSServiceCoreTests) failSequenceElementTests, 7FiftyOneJSServiceCoreTests, 11PipelineFilterTests, 9FiftyOneInterceptorTests, 32SessionIdAndSequenceTests, 10EvidenceKeyFilterShareUsageTests, 7ClientsidePropertyServiceTestsand 5UsageSharingEvidenceTestspassStartupHelpers_NoJsonPath_IsTheDefaultfails with "java.lang.NullPointerException: Cannot invoke "String.startsWith(String)" because "jsonPath" is null", 1 error in 12PipelineFilterTestsPipelineFilterTests, 13SequenceElementTestsand 7FiftyOneJSServiceCoreTestspassSetting 'PullRequestSha'line, in Configure and in each of the 9 jobs, is fb4f384, and Configure printed "Left out 6 macOS entries"Setting 'PullRequestSha'line, in Configure and in each of the 9 jobs, is dd3bd92, the head, and Configure printed "Left out 6 macOS entries"Setting 'PullRequestSha'line matches. The Ubuntu Java 11 job passes the unit tests and the example tests, then fails at the Selenium contract tests against the cloud and on-premise examples ("the first page must call the json endpoint at least once"), which stopped the other jobs. See "What remains"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.
Every exit path of
SequenceElement.processInternal,SequenceElement.toSequence,StartupHelpers.buildFromConfiguration,FiftyOneJSServiceCore.Default.serveJSandFiftyOneJSServiceCore.Default.serveJsonwas read after the change.What remains
/51Degrees.core.jsonwhile the Java examples now call/51dpipeline/json. selenium-api-tests#23 accepts both paths for Java, and selenium-api-tests#24 has merged it into that repository'smain, which the full build runs the contract tests from.