[kinyoklion]

Summary

This change moves the file data source onto the shared file loading code that #361 adds, so that it and the file-based override source that a later change adds share one reading, reloading, merging, and change detection implementation. It targets dotnet-server-sdk-v9, the branch that prepares the next major version, so the data source takes the shared code's file format rules instead of keeping its own. The data source keeps its options, its default file reader, and its data semantics; what is particular to it stays in FileDataSource, and the loading machinery it had of its own (FlagFileParser, FlagFileDataMerger, FileWatchingReloader) is removed.

File format changes for the next major version

• FileDataSourceBuilder.Parser takes the file extension the parser is for. The parser for a file is chosen by its extension; a file whose extension has no registered parser is parsed as JSON, and the content is no longer inspected to guess its format. A configured file with a .yaml or .yml extension and no registered parser can never load, so it is reported when the client is created.
• A flag or segment is listed under its key. A key property in the definition is optional and must be equal to the key it is listed under; before, the definition's own key was kept even when it differed. A version property is optional; before, a definition without one failed the load. The data source assigns the version of each load to every item, as it always did.
• A definition that is marked as deleted fails the load. A file with no content is a document with no entries, where before it failed to parse. Content after the end of the document fails the load, where before it was ignored.

The shared code gains these additions, each with a default that leaves the override source unchanged:

• FileDataReloaderConfig.MaxLoadAttempts, a bound on the consecutive failed attempts after which the reloader stops retrying until the next detected change. The file data source uses it to keep its documented promise of giving up after repeated failures. The line that ends the budget names the failure.
• FileDataReloaderConfig.FailOnMissingDirectory, which makes a path whose directory does not exist fail the load even when missing files are skipped. The file data source uses it to keep its rule that SkipMissingPaths covers a file that is missing from a directory that exists.
• A failure that repeats is demoted to debug level only on an automatic retry that fails the same way as the attempt before it. A load that a change notification or Start begins is a new event and is always logged at error level. Before, the demotion compared messages regardless of what started the load.
• A retry whose timer fires while a reload that then succeeds is in progress applies nothing afterwards: nothing has failed since, so there is nothing to retry. Before, it loaded the same data again.

What stays with the file data source

FileDataSource configures a FileDataReloader and, with auto-update, a FileDataWatcher, and supplies what the shared code does not decide:

• Every item of a successful load is stamped with one version that counts the successful loads, and the full data set is applied with Init on every successful load, including when the content is unchanged and on a repeated Start.
• Start loads synchronously and returns a completed task that reports whether any load has ever succeeded; Initialized stays true afterwards. After Dispose, Start does nothing.
• DuplicateKeysHandling.Throw fails the whole load and Ignore keeps the first occurrence across files; SkipMissingPaths skips a file that is missing from a directory that exists, and a path whose directory does not exist fails the load with or without it, so a directory that disappears cannot empty the store; the registered parsers and the configured FileReader are passed through; without auto-update, files are loaded once, with no retry and no watching.
• The flagValues expansion is the same flag as before: on, one variation, served by fallthrough.
• Each change notification still logs detected file modification, reloading.

Robustness the file data source gains

The observable data semantics are unchanged: the same documents produce the same flags and versions, Start reports the same result, and the last good data stays in place when a load fails. With auto-update on, the data source now behaves differently in these ways:

• Any failed load is retried, not only a parse failure: a read error, a duplicate key, or a failure to apply the data is retried too, after the shared code's one second delay rather than 600 ms, up to 5 attempts, and then given up until the next detected change. The budget covers the run of attempts, where it was counted per file before. Previously a read error on a load started by a change notification was not retried at all, and the data source waited for another notification that was not guaranteed to come.
• Change notifications settle for 100 ms before a reload, and notifications that arrive during a reload coalesce into one follow-up, so the burst of notifications one edit produces loads once. A stream of notifications that never settles reloads no later than one second after its first notification.
• The default reader is the shared code's. It opens a file while allowing other processes to write, rename, or delete it, so a load in progress never fails the editor or tool that is updating the file on Windows, and it does not wait for a process that denies reading: that load fails, and with auto-update it is retried after a second. Before, the reader held the file exclusively while reading and waited up to 30 seconds for a process that held it. FlagFileReader is removed.
• A deleted configured file is detected. Without SkipMissingPaths the reload fails and the data is kept, as a missing file always was; with SkipMissingPaths the remaining files are applied, so the deleted file's flags leave the store.
• A directory that does not exist disables nothing: each directory is watched when it exists and attempted again every second, where before one such path turned auto-update off for every file. The load of a path in a missing directory fails as before until the directory and the file exist; then the watcher picks them up. A watched directory that is deleted has its watch dropped and set up again when it returns, with one reload then. A watcher error is handled the same way. Configured files are compared by their metadata, read through symbolic links, so a replaced link is detected while other files in the directory cost nothing, and a change that arrives without a notification is found by a check once a second.
• The log messages are the shared code's. For failed loads: Unable to load flags: unable to read file: ... [path], Unable to load flags: error parsing file: ... [path], Unable to load flags: flag "key" is specified by multiple files; they carry the exception message without the exception type name, and no debug-level stack trace follows them. The first failure of a run is logged at error level, a retry that fails the same way as the attempt before it at debug level, and the end of the retry budget once at error level with the reason; a load started by a change notification or Start is always logged at error level. Before, every attempt logged a warning. New lines: Reloading file data after detecting a change at info level on each reload, in addition to the kept line; Retrying file data load after earlier failure at debug level; File {path} does not exist; it contributes no data at debug level for a skipped file, replacing {path}: File not found. The watcher logs its own messages (Unable to watch directory {dir}; will retry, Now watching directory {dir}, Directory {dir} no longer exists; its watch is set up again when it appears, Error from file system watcher for directory {dir}), which replace Unable to watch files for auto-updating for a directory that is missing; that line remains for a watcher that cannot be constructed at all, and is logged once. The debug line that logged the full content of every file read is gone.
• The version counter advances only on successful loads, so versions no longer skip numbers for failed attempts. Each applied load still has one version, the first is 1, and they increase.

Documented-only differences

• Under DuplicateKeysHandling.Ignore, a key that appears in both flags and flagValues of one file resolves to the flags entry. Before, the entry that came first in the document won. With Throw both fail the load, as before. The FileData docs already call an in-file duplicate an error.
• FileReader(null) means the default reader. Before, every load failed with a null reference.
• An exception from Init is a load failure: it is logged as Unable to load flags: error applying file data: ..., retried with auto-update, and makes Start return false, where before it escaped from Start or the watcher thread. It does not consume a version.
• The failure to construct the watcher is logged when Start runs, once, instead of when the client is built.
• A rename that replaces a configured file logs the detected-change line twice, once for each name, and reloads once.
• With auto-update, the reload after a change runs on a timer thread after the settle window, not on the file system watcher's thread.

The only public API change is the Parser signature described above. The doc comment of FileDataSourceBuilder.AutoUpdate describes the directory watching and the retained data, and the FileData docs name the flagValues property correctly and describe the key and version rules.

Tests

The baseline tests pass with these changes, each marked in the test with its reason. In FileDataSourceExistingBehaviorTest: three failed-load pins and the missing-directory pin keep their behavior assertions and get the shared code's message texts; the pin of a warning per failed parse attempt is replaced by the error-then-debug rule and the end of the budget. In FileDataSourceTest: the two tests of the per-file retry budget are replaced by one for the per-run budget; the test that loads good data after a bad start sees version 1, because a failed load no longer consumes a version; the test that a modified file is not reloaded while another is missing waits for the failed reload before it observes, because its observation window equaled the settle window; the tests whose scripted reader ignores the path use a path in a directory that does not exist, so that activity in the shared temp directory cannot reach the watcher on Windows and macOS, and the one that needs a notification uses a private directory; two comments that described the old mechanism are updated. FileDataSourceBuilderTest covers the parser registration by extension. FlagFileDataMergerTest goes with the merger; the shared FileDataMergerTest covers the same semantics. New tests pin the configuration error for a YAML file without a parser, that files with other extensions need none, and that the retry applies to a file read with a registered parser.

New tests in FileDataSourceTest pin the version stamping, that an Init that throws consumes no version, the application of identical content, the flagValues shape, Ignore across files, auto-update off meaning no watcher and no retry, Start after Dispose, that Dispose stops the watcher and a Start after it creates none, that a change made during the initial load is detected, the empty data set with both kinds, deletion with and without SkipMissingPaths, a directory created after start, the one second retry delay, the coalescing of a burst of notifications, that a retry firing during a successful Start does not apply again, and the per-run retry budget. New tests in FileDataSourceExistingBehaviorTest pin that every failed Start is logged at error level. New tests in FileDataReloaderTest pin MaxLoadAttempts, FailOnMissingDirectory, the repeat rule, the reason on the give-up line, no give-up without retries, and the stale retry. Each new test was run against a one-line mutation of the code that makes it fail; the mutations are listed in the commit message.

SDK-3248

---

Note

Overview
Moves the server file data source onto the shared FileLoading stack (FileDataReloader, FileDataWatcher, FileDataParser, FileDataReader) and deletes the old in-source pieces (FlagFileParser, FlagFileReader, FlagFileDataMerger, FileWatchingReloader). FileDataSource now configures the reloader/watcher and stamps versions in Apply; load, merge, debounce, retry, and watch behavior come from the shared engine.

Public API (v9): FileDataSourceBuilder.Parser is now Parser(extension, parseFn) with a dictionary of parsers by extension (JSON when none registered). Build fails if a .yaml/.yml path has no parser. Default reader is FileDataReader; FileReader(null) resets to default. Docs fix flagValues naming and describe optional key/version in files.

Shared reloader additions used by the data source: MaxLoadAttempts (5 with auto-update), FailOnMissingDirectory (with SkipMissingPaths), per-run retry budget, stale-retry skip after success, and updated failure logging (error on new loads, debug on identical retries, give-up line with reason).

Observable behavior shifts with auto-update (called out in tests/docs): 1s retry delay vs 600ms; debounced reloads; broader retry on read/merge/apply failures; deletion/skip-missing-path semantics; directory watching when paths don’t exist yet; unified log message prefixes; versions advance only on successful loads; several edge cases around Dispose/Start/watcher setup.

^{Reviewed by Cursor Bugbot for commit 656c726. Bugbot is set up for automated code reviews on this repo. Configure here.}

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[kinyoklion]

bugbot review

[cursor]

Stale Bugbot comment from a previous run.

[cursor]

Stale Bugbot comment from a previous run.

[kinyoklion]

bugbot review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 840c5e4. Configure here.}

[keelerm84]

An application that pairs the file data source with a persistent store gets told the client initialized when the store never received the flags, and auto-update never tries again. DataSourceUpdatesImpl.InitWithHeaders catches a store exception and returns false rather than throwing (DataSourceUpdatesImpl.cs:563-566), and this line ignores the result, so the new "an exception from Init is a load failure" path never runs with the SDK's own updates sink.

With a real DataSourceUpdatesImpl over a store whose Init throws:

• Start() returns true and Initialized is true.
• With AutoUpdate(true), no second Init happens within 3 seconds, so the store stays empty until someone edits a file.

The old code ignored the result too, but this PR is where the data source gains a retry path for exactly this case. Treating false as a failure routes it there:

if (!_dataSourceUpdates.Init(allData))
{
    throw new InvalidOperationException("the data store did not accept the data");
}

[keelerm84]

This DisarmRetry runs after Reload has released _reloadLock, so it can cancel a retry that a different reload armed in the meantime. If a change notification arrives while Start is loading, the triggered reload waits on the lock. If it then fails, it arms a retry, and this line can disarm it. The reloader is left with a failed load and nothing scheduled.

A partial write recovers anyway, because finishing the write changes the file and the watcher reloads. A failure that leaves the file untouched does not, for example a scanner or backup tool holding a deny-read lock. The application then serves the data from Start until the next edit, which is the case the timed retry is there to cover.

The window is narrow: a stress test that holds the triggered reload on the lock lost the retry once in 3000 runs. It is cheap to close by making the decision with the lock held, for example by disarming only when _failedAttempts == 0 under _reloadLock, so a success never cancels a retry for a failure that came after it.