[jwrosewell]

The problem

pipeline/Gaps-vs-spec.md lists where the Java pipeline differed from the specification. Checked against the code on rc and against the data update page, four data update items are still true:

Item	What the code did
D1, no published date	DataUpdateServiceDefault sent the epoch as If-Modified-Since when the data file had no published date.
D3, update on start-up alone	OnPremiseAspectEngineBuilderBase.preCreateEngine registered the data file with the update service only when auto update was on, so updateOnStartup with autoUpdate off did nothing.
D6, logging	Only debug lines for most actions, and error text that differed from the specification's table.
D8, licence key	An engine with the Distributor's URL built without a licence key, then failed on every update.

The same checks found D2, D4 and the programmatic update already met, and D5, D7 and D9 not required by the text, so those are not touched. The usage sharing items (U1 to U5) wait for the record format decision and are not touched either.

The specification is quoted at commit ba079423 of specifications rc.

What changes

Each change cites the specification sentence it follows.

1. D1. If-Modified-Since is the data file's published date, or its last modified time on disk when it has none. Update polling: "If the data file does not have a publication date then the file system last modified date/time can be used instead." An in-memory data file with no date still sends nothing.
2. D3. The builder registers the data file with the update service when auto update or update on start-up is on, as .NET does. Update on start-up: "This option can be used to bootstrap an Aspect Engine that does not yet have any local data." The service now creates the file system watcher only when auto update is on, because the master switch "is the master switch for all automatic update activity", file system watching included. Without that, registering for update on start-up alone would also have started the watcher.
3. D6. The info messages and the error messages in the Logging tables, with their text and their levels (the specification gives no level for the info tables, so they are info):
   • "Updating on start-up", "Creating file system watcher", "Checking for update", "Attempting to refresh Engine '...' with new data", "Checking for update from '...' for Engine '...'", "Downloaded new data from '...' for Engine '...'" and "No data newer than ... found at '...' for Engine '...'" (a 304 reply, which was debug).
   • Warnings for a failed connection, a failed download (HTTP 429, 403 and any other failing status included, as the table's rows say) and a failed integrity check, with "Update will be attempted again later." only when auto update is on and the update is not a manual one. A failed connection and a failing HTTP status were errors, and a failed integrity check logged nothing.
   • Errors for a failed write, a failed refresh (one error after the retries, where there were ten warnings), a missing temporary file path and a failed copy of the data file.
   • Not added: the row for a locked data file with "use temp file" off, because no code path matches it.
   • These messages and levels are the tables as they read at ba079423. specifications#60 changes the Errors table to carry .NET's text and levels, so that every language says the same thing for the same event. A separate Java draft moves these messages onto .NET's wording once that change is merged, and is part of the same release.
4. Engine name before the engine exists. Update on start-up runs before the engine is built, so its log lines said Engine 'No Engine'. The builder now gives the data file its engine's type name, taken from the builder's own name (DeviceDetectionHashEngineBuilder gives DeviceDetectionHashEngine), and the update service logs it until the engine is set. AspectEngineDataFileDefault gains getEngineTypeName and setEngineTypeName.
5. D8. The build throws PipelineConfigurationException when auto update or update on start-up is on, the update URL is the Distributor's and no licence key is set. The message names setAutoUpdate(false) or setDataUpdateOnStartup(false), whichever is on. Defaults: the licence key is "required before the Engine builds with autoUpdate or updateOnStartup on ... The build fails with a configuration error that names the option to turn off." A key is not required when the URL is not the Distributor's, which is how an update source that serves the file without one is configured. OnPremiseAspectEngineBuilderBase gains a protected isLicenseKeyRequired that returns false, and FiftyOneOnPremiseAspectEngineBuilderBase returns true for the Distributor's URL. Every data file is checked before any is registered, so a builder with several data files never registers the first, or runs its update on start-up, before refusing the build for a later one.
6. Licence keys masked in every message. The data update URL carries the licence keys in its query, and the new info and warning messages named it in full. Logging: "Wherever a message below names the <url>, it is the URL with each license key masked to its last 4 characters", and logging.md: "The same applies to the message of an error raised by an HTTP library, which often includes the URL it requested." Every message in downloadFile now names the URL through one loggedUrl, made with LicenseKeyMask.maskIn from #85, and the error detail and the exception logged with a failed connection or download are masked with LicenseKeyMask.maskIn as well. This is the part of #10 that this draft's new messages reopened. The getIdForLogging helper, which only the replaced messages used, is removed.
7. The write error. A failure writing the download to the temporary file was logged as the "Error downloading file" warning. The download stream now records whether a read failed, so a failure that is not a read logs the Errors table's row for "Error writing to data file location or temp file location", "An error occurred when writing to <file path>. Error detail: <error>", at error level, and the update returns AUTO_UPDATE_ERR_READING_STREAM, whose documentation is "Used when IO operations with input or output stream failed". A failed read is still the download warning.
8. Programmatic update reads the file on disk. checkForUpdate compared the data file with the engine's temporary file only when the file system watcher flag was off. The watcher now exists only when auto update is on, so with auto update off and the flag at its default of true, a programmatic update never looked at the file on disk. Programmatic update: "If automatic update via HTTP is not enabled and a file data source is specified then it causes immediate refresh of the Engine using the file at the configured location, irrespective of whether Automatic Update from File is enabled." A programmatic update now always compares the files, and the timer compares them whenever no watcher exists, where it used to go by the flag.
9. The message for a missing data update service. Commit 10a8a5d corrects the grammar of the PipelineConfigurationException the builder throws when auto update or update on start-up is on and no data update service was given, in OnPremiseAspectEngineBuilderBase.java lines 229 and 232 to 233. "an DataUpdateService" becomes "a DataUpdateService" in both places and "to the the pipeline builder" loses the doubled word. No test asserts that text, and a caller who reads the message sees the corrected wording.
10. The refresh failure message (commits 5365b94 and d8f75d7). The message logged when a refresh fails is now the Errors table's, at specifications 8aa2de01. A refresh from the file, which is tried 10 times, logs "An error occurred when applying a data update to engine '<Engine type name>' after <tries> tries.", and a refresh from data held in memory logs "An error occurred when applying a data update to engine '<Engine type name>'.". The second row covers updateFromMemory and the download that checkForUpdateFromUrl keeps in memory when there is no temporary file, because both pass the data to the engine as bytes with refreshData(identifier, byte[]) and try once, rather than pointing it at a file and trying 10 times. Both are at error level and carry the exception, as .NET and Go word them. Before, Java logged "An error occurred while applying a data file update to <type>. Error detail: <message>".

Behaviour that changes for callers

• A pipeline built from the free Lite data file with the defaults now fails to build, because auto update is on by default and no licence key is set. The caller turns auto update off with setAutoUpdate(false), as the specification's defaults say. The tests that built with the defaults now do this or give a key, and the examples already did. This belongs in the release note.
• setDataUpdateOnStartup(true) now works with setAutoUpdate(false).
• The data update service no longer creates a file system watcher for a data file whose auto update is off.
• Log messages name the data update URL with each licence key masked, for example LicenseKeys=********************3LPA.
• A failure writing the temporary file is an error with the write text, and checkForUpdate returns AUTO_UPDATE_ERR_READING_STREAM for it, where it was the download warning and AUTO_UPDATE_HTTPS_ERR.
• checkForUpdate called by the application refreshes the engine from the file on disk when that file is newer, whatever the watcher flag says.
• No public method, option or default value is removed or changed. The one addition is the protected isLicenseKeyRequired.

Other open drafts

#85 is merged into rc, and its change to downloadFile conflicted with this draft's. The first merge of rc (06e3a21) keeps this draft's messages, and the next commits mask them. The second merge of rc (76eb957, rc at ff6e4c0, which holds #92) had no text conflict, but #92 added DataUpdateService_CheckForUpdate_FileRefreshFails_TimerSetAgain, which allowed for a warning on each of the 10 refresh tries. This draft logs each try at debug and one error after the last, the Errors table's "Error when calling RefreshData on Engine" row, so 0c7e639 makes the test allow that one error. git merge-tree of this head against every other open draft's head finds conflicts only with #97, #67, #63 and #43, and each of those conflicts with rc itself.

The third merge of rc (3a3cfe4, rc at 1c2c578) brings in #98, which bounds the wait for a watched data file at ten minutes and registers the file again after a watched refresh, and moves the cxx submodule to c6d4e4d9. Git merged DataUpdateServiceDefault.java with no conflict, because #98 changes only the watched file branch of checkForUpdate, which this draft does not touch, and its new DataUpdateServiceWatchedFileTests sit beside this draft's tests. Commit 031a329 merges rc again at ce406b2, which holds #80, #99 and #88, and the head 546696d merges rc at 528e0df, which adds #59, #61, #62, #101 and #108. Neither touches any of this draft's files.

How it was checked

Tests were written first and fail on rc.

• DataUpdateServiceLoggingTests is new. It reads each message and level from the specification and checks If-Modified-Since for a data file with and without a published date. Six of its tests give the data file a real-looking licence key in its URL and check, at every level, that each message and each exception logged with one holds only the masked key, for a 304, a 200, a failed connection, a failed download, a 403 and no response. One checks the write error text.
• DataUpdateServiceTests now expects no file system watcher when auto update is off, a warning, not an error, for a failed connection or a 429, and a programmatic update that refreshes from a newer file on disk with auto update off and the watcher flag at its default.
• FiftyOneDegreesOnPremiseAspectEngineBuilderBaseTests checks registration with update on start-up alone, and the licence key refusal and its exceptions. OnPremiseAspectEngineBuilderBaseTest checks that a builder with two data files, the second without a key, refuses the build before it registers either. A single file builder refuses two data files before it reaches this check, so the test uses the base builder.
• UpdateOnStartup_AutoUpdateOff_StartsNoTimer in DataUpdateServiceLoggingTests passes on rc as well, so it guards the behaviour and does not prove the change. D3 is proved by UpdateOnStartup_AutoUpdateOff_DataFileRegistered in FiftyOneDegreesOnPremiseAspectEngineBuilderBaseTests and by the auto update off cases of DataUpdateService_Register_FileSystemWatcher in DataUpdateServiceTests, which both fail in the first red run below.

Run	Link	Result
Red, tests only	run	12 of 14 logging tests, 10 of 108 service tests and 3 of 9 builder tests fail on Ubuntu and Windows
Green, with the fix	run	all 14, 104 and 9 pass on Ubuntu and Windows
Red, HTTP statuses	run	3 new logging tests and the 429 test fail on Ubuntu and Windows
Green, HTTP statuses	run	all 17 logging tests and 104 service tests pass on Ubuntu and Windows
Red, engine name	run	The tests do not compile on rc without setEngineTypeName
Green, engine name	run	18 logging tests and 10 builder tests pass
Red, masking, write error, file comparison, every key first	run	The tests on the first merge of rc, without the fixes. On Ubuntu and Windows, 7 of 25 logging tests (the six licence key tests and the write error), all 7 DataUpdateServiceLicenseKeyTests from #85, the 4 cases of the file comparison test and LaterDataFileHasNoLicenseKey_NothingRegistered fail
Green, masking, write error, file comparison, every key first	run	On Ubuntu and Windows, all 25 logging tests, 7 licence key tests, 108 service tests, 5 builder base tests (3 are Unix only and skip on Windows) and 10 builder tests pass
Earlier full build, head 98f3a9a	run	Every Ubuntu and Windows job passed
Full build of the merge, head 76eb957	run	Ubuntu Java 11 failed at the 4 cases of DataUpdateService_CheckForUpdate_FileRefreshFails_TimerSetAgain ("1 errors occurred during test", the refresh error), and the other jobs were cancelled. Fixed in 0c7e639
Earlier full build, head 0c7e639	run	Head 0c7e639. All 9 jobs without macOS passed: Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21. Every Setting 'PullRequestSha' line, in Configure and in each of the 9 jobs, is 0c7e639, the head, and Configure printed "Left out 6 macOS entries"

| Red, the refresh failure message | run 43034146958 | The tests (5365b94) without the fix. On Java 8 and 21, the 4 rows of DataUpdateService_CheckForUpdate_FileRefreshFails_TimerSetAgain of 132 DataUpdateServiceTests and UpdateFromMemory_RefreshFails_LogsTheTableMessage of 25 DataUpdateServiceLoggingTests fail |
| Green, the refresh failure message | run 43034177333 | d8f75d7. On Java 8 and 21, all 132 and all 25 pass |
| Full pull request build | run 43071987865 | Head 546696d, which merges rc at 528e0df. All 9 jobs without macOS passed, being Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21. Every Setting 'PullRequestSha' line, in Configure and in each of the 9 jobs, is 546696d, and Configure printed "Left out 6 macOS entries" |
| Earlier attempt on 031a329 | run 43051980968 | Failed at "Pipeline developer examples" for want of a resource key, which #108 fixed in rc |
| Earlier full build | run 43015715793 | Head 3a3cfe4, before the refresh failure message. Every job passed |
| Earlier full build | run | Head 10a8a5d, before the merge of rc that brings in #98. All 9 jobs without macOS passed, being Ubuntu Java 8, 11, 17 and 21, Ubuntu ARM Java 17, and Windows Java 8, 11, 17 and 21. Every Setting 'PullRequestSha' line, in Configure and in each of the 9 jobs, is 10a8a5d, the head, and Configure 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.

Every exit path of downloadFile, downloadOkResponse, checkForUpdate, checkForUpdateFromUrl, updatedFileAvailable, updateFromMemory and preCreateEngine was read after the change. The new temporary file path check returns AUTO_UPDATE_REFRESH_FAILED where a missing path used to throw a NullPointerException out of the timer thread.

What remains

• The messages and levels realign to .NET's. specifications#60 makes the data update Logging tables follow .NET's text and levels for each event, including reporting a skipped or not needed check as NotNeeded, and Java takes that wording in a separate draft in this release, stacked on this one.
• D2 and D4 need no change. D5, D7 and D9 are not required by the text, and the usage sharing items wait for the record format decision.
• The pipeline/Gaps-vs-spec.md file is not edited here. Its removal is for whoever closes the list.