From 79250a7133b968a4b563d9cd0c5ac15202b04591 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Mon, 28 Sep 2026 13:38:12 -0700 Subject: [PATCH 1/9] feat: Add reload infrastructure for file-based flag overrides The flag overrides feature needs a file source that reloads reliably: coalesced change notifications, retry after a failed read, retention of the last good data, sequential reloads, and support for files that do not exist yet. This adds that infrastructure to the integrations package as a purely additive change. The existing file data source keeps its current behavior. - FileDataReloader serializes reloads, debounces change signals, keeps the last good data on failure by not applying, retries a failed load after a bounded delay, reports an identical repeated failure once, and skips a reload whose raw content did not change. Close never waits for an in-flight read. - FileDataPoller detects changes by examining each file's modification time and size on a fixed interval, including a file that appears or disappears. - FileDataWatcher watches parent directories and signals once after start so a change between an initial load and the start of watching is not missed. - OverrideFileLoader reads a list of files with the override source's rules: a missing file contributes no entries, entries keep the versions that the documents specify, a value-only entry becomes a flag that is off and serves the value, a document that the model rejects is a file data error, and the result carries a per-file summary and a content digest. Two additive FlagFactory methods keep document versions. FileDataSourceImpl, FileSynchronizer, FileInitializer, FileDataSourceBase, and FileDataException are unchanged. Two new test classes pin the file data source's reload and error behavior: a reload on every watch event with no debounce, no retry, and no skip, every failure reported and logged as the plain description at error level, a model-rejected document escaping as a SerializationException, and a description that requires a cause. --- .../server/integrations/FileDataPoller.java | 134 +++++ .../server/integrations/FileDataReloader.java | 324 +++++++++++ .../integrations/FileDataSourceParsing.java | 33 ++ .../server/integrations/FileDataWatcher.java | 155 ++++++ .../integrations/OverrideFileLoader.java | 247 +++++++++ .../FileDataLoadingBehaviorTest.java | 80 +++ .../integrations/FileDataPollerTest.java | 182 +++++++ .../integrations/FileDataReloaderTest.java | 512 ++++++++++++++++++ .../integrations/FileDataWatcherTest.java | 166 ++++++ .../FileSynchronizerReloadBehaviorTest.java | 204 +++++++ .../integrations/OverrideFileLoaderTest.java | 290 ++++++++++ 11 files changed, 2327 insertions(+) create mode 100644 lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java create mode 100644 lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java create mode 100644 lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java create mode 100644 lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataLoadingBehaviorTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileSynchronizerReloadBehaviorTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java new file mode 100644 index 00000000..2cc5e488 --- /dev/null +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java @@ -0,0 +1,134 @@ +package com.launchdarkly.sdk.server.integrations; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.time.Duration; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Detects changes to a set of files by examining them on a fixed interval. Use it where file + * system change notifications are not available or not reliable, alone or together with them. + * A change to the modification time or the size of any file invokes the change callback. A file + * that appears or disappears is also a change. A file whose attributes cannot be read counts as + * absent. + *

+ * The poller samples the files once per interval and compares only the modification time and the + * size. A rewrite that keeps both values is not detected. + *

+ * Detection is generous. The callback can run for a change that does not alter the effective + * data. Feed it into a {@link FileDataReloader}, whose debouncing and skip-unchanged handling + * absorb the excess. + */ +final class FileDataPoller implements Closeable { + private final List paths; + private final Runnable onChange; + private final ScheduledThreadPoolExecutor executor; + private final AtomicBoolean closed = new AtomicBoolean(false); + private List last; + + /** + * Creates a started poller. It examines the files once before it returns, so only later changes + * invoke the callback. Call {@link #close()} to stop it. + * + * @param paths the files to examine + * @param interval the time between examinations + * @param onChange called when any file changed since the previous examination + */ + FileDataPoller(List paths, Duration interval, Runnable onChange) { + this.paths = new ArrayList<>(paths); + this.onChange = onChange; + this.last = observeAll(this.paths); + ThreadFactory threadFactory = runnable -> { + Thread t = new Thread(runnable, "LaunchDarkly-FileDataPoller"); + t.setDaemon(true); + return t; + }; + this.executor = new ScheduledThreadPoolExecutor(1, threadFactory); + this.executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); + long millis = Math.max(interval.toMillis(), 1); + this.executor.scheduleWithFixedDelay(this::examine, millis, millis, TimeUnit.MILLISECONDS); + } + + /** + * Stops the poller. It does not wait for an examination or a callback that is in progress. A + * file system that does not respond must not block shutdown. As a result, the callback can run + * one more time shortly after close returns. Consumers tolerate a late call, as they do for a + * late reload. + */ + @Override + public void close() { + if (closed.getAndSet(true)) { + return; + } + executor.shutdownNow(); + } + + private void examine() { + if (closed.get()) { + return; + } + List current = observeAll(paths); + boolean changed = !current.equals(last); + last = current; + if (changed && !closed.get()) { + onChange.run(); + } + } + + static List observeAll(List paths) { + List states = new ArrayList<>(paths.size()); + for (Path path : paths) { + states.add(observe(path)); + } + return states; + } + + private static FileState observe(Path path) { + try { + BasicFileAttributes attributes = Files.readAttributes(path, BasicFileAttributes.class); + return new FileState(true, attributes.lastModifiedTime().toMillis(), attributes.size()); + } catch (IOException | RuntimeException e) { + return FileState.ABSENT; + } + } + + /** + * The observed state of one file, or its absence. + */ + static final class FileState { + static final FileState ABSENT = new FileState(false, 0, 0); + + final boolean exists; + final long modifiedTimeMillis; + final long size; + + FileState(boolean exists, long modifiedTimeMillis, long size) { + this.exists = exists; + this.modifiedTimeMillis = modifiedTimeMillis; + this.size = size; + } + + @Override + public boolean equals(Object other) { + if (other instanceof FileState) { + FileState o = (FileState) other; + return exists == o.exists && modifiedTimeMillis == o.modifiedTimeMillis && size == o.size; + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(exists, modifiedTimeMillis, size); + } + } +} diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java new file mode 100644 index 00000000..259ee185 --- /dev/null +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java @@ -0,0 +1,324 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogValues; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; +import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; + +import java.io.Closeable; +import java.time.Duration; +import java.util.Arrays; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; + +/** + * Owns the reload cycle for the files of an override source. It serializes reloads, debounces + * change signals, retains the last good result on failure by not calling the apply callback, + * retries after failures, and skips applications whose content did not change. + *

+ * The reload work itself is delegated to a {@link Loader}. The loader reads every configured file + * and returns the merged result, or throws {@link FileDataException} when a file cannot be read or + * parsed or when the files cannot be merged. + *

+ * The file data source does not use this class. It keeps its own reload behavior. + */ +final class FileDataReloader implements Closeable { + /** + * A settle window long enough to coalesce the burst of change notifications produced by a + * single file edit, and short enough to stay responsive. + */ + static final Duration DEFAULT_DEBOUNCE_DELAY = Duration.ofMillis(100); + + /** + * Bounds how long a failed reload can go uncorrected when no further change notification + * arrives, for example when the failure came from reading a file mid-write. Reading a local + * file is cheap, so this can be short. + */ + static final Duration DEFAULT_RETRY_DELAY = Duration.ofSeconds(1); + + /** + * Performs one full load of all configured files. + */ + interface Loader { + LoadResult load() throws FileDataException; + } + + /** + * Receives the outcome of each reload. + *

+ * Both methods are called with the reload lock held, so calls never overlap with each other + * or with another reload. Neither method may call back into {@link FileDataReloader#close()}. + */ + interface Handler { + /** + * Called with each successfully merged result. + * + * @param result the merged data + */ + void apply(LoadResult result); + + /** + * Called when a reload fails, once per distinct failure. With automatic retries, repeats of an + * identical failure do not call this again. A success re-arms it. The reloader logs failures + * itself, so implementations only need to update their own state. + * + * @param e the failure + */ + void onError(FileDataException e); + } + + private final Loader loader; + private final Handler handler; + private final LDLogger logger; + private final long debounceDelayMillis; + private final long retryDelayMillis; + private final boolean skipUnchanged; + + // Serializes the load work between reloadNow and the worker thread. + private final Object reloadLock = new Object(); + private byte[] lastGoodHash = null; + private String lastErrorMessage = null; + + // Guards the worker and the two timers. + private final Object timerLock = new Object(); + private ScheduledThreadPoolExecutor worker = null; + private ScheduledFuture debounceFuture = null; + private ScheduledFuture retryFuture = null; + + private final AtomicBoolean closed = new AtomicBoolean(false); + + /** + * Creates a reloader. The caller performs the initial load with {@link #reloadNow()}, routes + * change signals to {@link #trigger()}, and calls {@link #close()} when finished. + *

+ * The worker thread starts on the first {@link #trigger()} call or the first failed load that + * arms a retry, not here. Construction alone does not start a thread. + * + * @param loader performs each full load + * @param handler receives results and failures + * @param logger receives log output about reloads and failures + * @param debounceDelay how long to wait after a trigger for further triggers to settle before + * reloading. Zero or negative reloads on every trigger without waiting. + * @param retryDelay how long to wait after a failed reload before retrying without a trigger. + * Zero or negative disables the automatic retry. + * @param skipUnchanged if true, a successful load whose raw file contents are identical to the + * last applied contents does not call the apply callback + */ + FileDataReloader( + Loader loader, + Handler handler, + LDLogger logger, + Duration debounceDelay, + Duration retryDelay, + boolean skipUnchanged + ) { + this.loader = loader; + this.handler = handler; + this.logger = logger; + this.debounceDelayMillis = debounceDelay == null ? 0 : debounceDelay.toMillis(); + this.retryDelayMillis = retryDelay == null ? 0 : retryDelay.toMillis(); + this.skipUnchanged = skipUnchanged; + } + + /** + * Synchronously loads the files and applies the result or reports the failure. Use it for the + * initial load. A failure here arms the same automatic retry as a failed triggered reload. + */ + void reloadNow() { + if (!reload() && retryDelayMillis > 0) { + armRetry(); + } + } + + /** + * Signals that the files may have changed and a reload should happen after the debounce delay. + * It never blocks. Signals that arrive while a reload is already pending extend the settle + * window, so a burst of signals produces one reload after the burst ends. + */ + void trigger() { + synchronized (timerLock) { + if (closed.get()) { + return; + } + ScheduledThreadPoolExecutor executor = ensureWorker(); + if (debounceFuture != null) { + debounceFuture.cancel(false); + } + debounceFuture = executor.schedule(() -> runReload(false), Math.max(debounceDelayMillis, 0), + TimeUnit.MILLISECONDS); + } + } + + /** + * Stops the reloader. It does not wait for a reload that is already in progress. A reload + * wedged in a blocking file read must not be able to wedge shutdown. Such a reload may still + * deliver its result shortly after close returns, and consumers tolerate that. A reload that + * has not yet reached its callbacks when close is called does not invoke them. + */ + @Override + public void close() { + if (closed.getAndSet(true)) { + return; + } + synchronized (timerLock) { + if (debounceFuture != null) { + debounceFuture.cancel(false); + debounceFuture = null; + } + if (retryFuture != null) { + retryFuture.cancel(false); + retryFuture = null; + } + if (worker != null) { + worker.shutdownNow(); + } + } + } + + /** + * Reports whether the worker thread has been started. Visible for tests. + * + * @return true if a worker exists + */ + boolean hasWorker() { + synchronized (timerLock) { + return worker != null; + } + } + + /** + * Formats a failure for logging. The exception's own description requires a cause, and a + * duplicate key failure has none, so this builds the text from the parts that are present. + * + * @param e the failure + * @return the message, followed by the cause in brackets when there is one + */ + static String describe(FileDataException e) { + StringBuilder s = new StringBuilder(); + if (e.getMessage() != null) { + s.append(e.getMessage()); + } + if (e.getCause() != null) { + if (s.length() > 0) { + s.append(" "); + } + s.append("[").append(e.getCause().toString()).append("]"); + } + return s.toString(); + } + + private ScheduledThreadPoolExecutor ensureWorker() { + if (worker == null) { + ThreadFactory threadFactory = runnable -> { + Thread t = new Thread(runnable, "LaunchDarkly-FileDataReloader"); + t.setDaemon(true); + return t; + }; + ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1, threadFactory); + executor.setRemoveOnCancelPolicy(true); + executor.setExecuteExistingDelayedTasksAfterShutdownPolicy(false); + worker = executor; + } + return worker; + } + + private void armRetry() { + synchronized (timerLock) { + if (closed.get()) { + return; + } + // An already armed retry keeps its earlier deadline. + if (retryFuture != null && !retryFuture.isDone()) { + return; + } + retryFuture = ensureWorker().schedule(() -> runReload(true), retryDelayMillis, TimeUnit.MILLISECONDS); + } + } + + // Runs on the worker thread for a debounced or a retried reload. + private void runReload(boolean isRetry) { + try { + if (isRetry) { + logger.debug("Retrying flag data load after earlier failure"); + } else { + logger.info("Reloading flag data after detecting a change"); + } + synchronized (timerLock) { + // This reload supersedes a pending retry. It either succeeds, or it fails and arms a fresh + // retry below. + if (retryFuture != null) { + retryFuture.cancel(false); + retryFuture = null; + } + } + if (!reload() && retryDelayMillis > 0) { + armRetry(); + } + } catch (RuntimeException e) { + // The executor would otherwise swallow the exception and the worker would go quiet. + logger.error("Unexpected error while reloading flag data: {}", LogValues.exceptionSummary(e)); + logger.debug(LogValues.exceptionTrace(e)); + } + } + + // Performs one full load of all configured files. It reports whether the load succeeded, which + // decides whether a retry gets armed. A skipped no-op application counts as success. The whole + // set is re-read on every reload because entries are combined across files in order, so a + // change to one file can alter which file wins for a key. + private boolean reload() { + synchronized (reloadLock) { + // A trigger already queued when close was called can still reach here. + if (closed.get()) { + return true; + } + + LoadResult result; + try { + result = loader.load(); + } catch (FileDataException e) { + return fail(e); + } + + // Close may have happened while the files were being read. Deliver nothing in that case. + if (closed.get()) { + return true; + } + + // A success right after a failure must apply even when the content is unchanged since the + // last success. The consumer heard about the failure and may have moved to an interrupted + // state. Only apply tells it that things are good again. + boolean recovering = lastErrorMessage != null; + lastErrorMessage = null; + byte[] hash = result.getContentHash(); + if (skipUnchanged && !recovering && hash != null && Arrays.equals(hash, lastGoodHash)) { + return true; + } + lastGoodHash = hash; + handler.apply(result); + return true; + } + } + + private boolean fail(FileDataException e) { + // Close may have happened while the files were being read. Deliver nothing and report success + // so that no retry is armed. + if (closed.get()) { + return true; + } + // With automatic retries, a persistent failure would repeat the same log entry and the same + // callback on every attempt. Repeats of an identical failure are logged at debug level and do + // not call onError again. A consumer therefore sees one report per distinct failure. + String description = describe(e); + if (description.equals(lastErrorMessage)) { + logger.debug("Unable to load flags: {}", description); + return false; + } + lastErrorMessage = description; + logger.error("Unable to load flags: {}", description); + handler.onError(e); + return false; + } +} diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java index 81ff8cdc..bae5309d 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java @@ -193,6 +193,31 @@ private FlagFactory() {} static ItemDescriptor flagFromJson(LDValue jsonTree, int version) { return FEATURES.deserialize(replaceVersion(jsonTree, version).toJsonString()); } + + /** + * Constructs a flag from raw JSON and keeps the version that the document specifies. The + * override source uses this. The file data source keeps assigning its load version. + */ + static ItemDescriptor flagFromJson(LDValue jsonTree) { + return FEATURES.deserialize(jsonTree.toJsonString()); + } + + /** + * Constructs a flag that is off and serves the given value as its single variation for every + * context. The document supplies no version, so the flag has version zero. This is the shape + * that the override source uses for value-only entries. The file data source keeps + * {@link #flagWithValue(String, LDValue, int)}. + */ + static ItemDescriptor offFlagWithValue(String key, LDValue jsonValue) { + LDValue o = LDValue.buildObject() + .put("key", key) + .put("version", 0) + .put("on", false) + .put("offVariation", 0) + .put("variations", LDValue.buildArray().add(jsonValue).build()) + .build(); + return FEATURES.deserialize(o.toJsonString()); + } /** * Constructs a flag that always returns the same value. This is done by giving it a single @@ -214,6 +239,14 @@ static ItemDescriptor flagWithValue(String key, LDValue jsonValue, int version) static ItemDescriptor segmentFromJson(LDValue jsonTree, int version) { return SEGMENTS.deserialize(replaceVersion(jsonTree, version).toJsonString()); } + + /** + * Constructs a segment from raw JSON and keeps the version that the document specifies. The + * override source uses this. + */ + static ItemDescriptor segmentFromJson(LDValue jsonTree) { + return SEGMENTS.deserialize(jsonTree.toJsonString()); + } private static LDValue replaceVersion(LDValue objectValue, int version) { ObjectBuilder b = LDValue.buildObject(); diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java new file mode 100644 index 00000000..172f1a0f --- /dev/null +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java @@ -0,0 +1,155 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogValues; + +import java.io.Closeable; +import java.io.IOException; +import java.nio.file.ClosedWatchServiceException; +import java.nio.file.FileSystems; +import java.nio.file.Path; +import java.nio.file.WatchEvent; +import java.nio.file.WatchKey; +import java.nio.file.WatchService; +import java.nio.file.Watchable; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_DELETE; +import static java.nio.file.StandardWatchEventKinds.ENTRY_MODIFY; +import static java.nio.file.StandardWatchEventKinds.OVERFLOW; + +/** + * Watches a set of files for changes on a worker thread and invokes a callback when one of them + * is created, modified, or deleted. + *

+ * The Java file system API reports changes at the directory level, so the watcher registers the + * parent directory of every file. A file that does not exist yet is reported when it appears, as + * long as its directory exists when the watcher is created. A directory that does not exist is an + * error from {@link #create(Iterable, LDLogger)}. + *

+ * The callback is invoked for every relevant notification, including the burst of notifications + * that a single edit can produce, and once more right after the watcher starts so that a change + * made between an initial load and the start of watching is not missed. Feed it into a + * {@link FileDataReloader}, which coalesces the burst and skips no-op reloads. + */ +final class FileDataWatcher implements Closeable, Runnable { + private final WatchService watchService; + private final Set watchedFilePaths; + private final LDLogger logger; + private final Thread thread; + private final AtomicBoolean stopped = new AtomicBoolean(false); + private volatile Runnable onChange; + + /** + * Creates a watcher for the given files. Nothing is watched until {@link #start(Runnable)}. + * + * @param filePaths the files to watch, absolute or relative to the working directory + * @param logger the logger + * @return the watcher + * @throws IOException if the watch service cannot be created or a parent directory cannot be + * registered, for example because it does not exist + */ + static FileDataWatcher create(Iterable filePaths, LDLogger logger) throws IOException { + Set directoryPaths = new HashSet<>(); + Set absoluteFilePaths = new HashSet<>(); + for (Path p : filePaths) { + Path absolutePath = p.toAbsolutePath().normalize(); + absoluteFilePaths.add(absolutePath); + directoryPaths.add(absolutePath.getParent()); + } + WatchService ws = FileSystems.getDefault().newWatchService(); + try { + for (Path d : directoryPaths) { + d.register(ws, ENTRY_CREATE, ENTRY_MODIFY, ENTRY_DELETE); + } + } catch (IOException | RuntimeException e) { + ws.close(); + throw e; + } + return new FileDataWatcher(ws, absoluteFilePaths, logger); + } + + private FileDataWatcher(WatchService watchService, Set watchedFilePaths, LDLogger logger) { + this.watchService = watchService; + this.watchedFilePaths = watchedFilePaths; + this.logger = logger; + this.thread = new Thread(this, "LaunchDarkly-FileDataWatcher"); + this.thread.setDaemon(true); + } + + /** + * Starts the worker thread and then signals one change, because a file may have changed between + * the caller's initial load and the registration of the watches. + * + * @param onChange called when a watched file changed + */ + void start(Runnable onChange) { + this.onChange = onChange; + thread.start(); + signal(); + } + + @Override + public void run() { + while (!stopped.get()) { + WatchKey key; + try { + key = watchService.take(); // blocks until a change is available or the thread is interrupted + } catch (InterruptedException e) { + continue; // if stopped, the loop condition ends the thread + } catch (ClosedWatchServiceException e) { + return; + } + boolean watchedFileWasChanged = false; + for (WatchEvent event : key.pollEvents()) { + if (event.kind() == OVERFLOW) { + // Notifications were dropped, so a watched file may have changed. + watchedFileWasChanged = true; + break; + } + Watchable w = key.watchable(); + Object context = event.context(); + if (w instanceof Path && context instanceof Path) { + Path absolutePath = ((Path) w).resolve((Path) context); + if (watchedFilePaths.contains(absolutePath)) { + watchedFileWasChanged = true; + break; + } + } + } + key.reset(); // without this, the watch on this key stops working + if (watchedFileWasChanged && !stopped.get()) { + signal(); + } + } + } + + private void signal() { + try { + onChange.run(); + } catch (RuntimeException e) { + // COVERAGE: there is no way to simulate this condition in a unit test + logger.warn("Unexpected exception when reloading file data: {}", LogValues.exceptionSummary(e)); + } + } + + /** + * Stops watching and releases the watch service. It does not wait for the worker thread. + */ + @Override + public void close() { + if (stopped.getAndSet(true)) { + return; + } + thread.interrupt(); + try { + watchService.close(); + } catch (IOException e) { + // COVERAGE: there is no way to simulate this condition in a unit test + logger.debug("Error closing file watch service: {}", LogValues.exceptionSummary(e)); + } + } +} diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java new file mode 100644 index 00000000..77dca424 --- /dev/null +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java @@ -0,0 +1,247 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.google.common.collect.ImmutableList; +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FlagFactory; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FlagFileParser; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FlagFileRep; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.DataKind; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.NoSuchFileException; +import java.nio.file.Path; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.util.AbstractMap; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.launchdarkly.sdk.server.DataModel.FEATURES; +import static com.launchdarkly.sdk.server.DataModel.SEGMENTS; + +/** + * Reads the files of the file-based override source and merges them into one snapshot. + *

+ * This loader is separate from the file data source's loader because the override source has + * different rules: a configured file that does not exist contributes no entries, entries keep the + * versions that the documents specify, a value-only entry becomes a flag that is off and serves the + * value, and every failure to read or parse a file is reported as a file data error. The file data + * source keeps its own behavior. + */ +final class OverrideFileLoader { + /** + * Describes one configured file after a load. + */ + static final class FileSummary { + private final Path path; + private final boolean present; + private final int flags; + private final int segments; + + FileSummary(Path path, boolean present, int flags, int segments) { + this.path = path; + this.present = present; + this.flags = flags; + this.segments = segments; + } + + Path getPath() { + return path; + } + + /** + * False when the file does not exist. + */ + boolean isPresent() { + return present; + } + + /** + * The number of flag entries the merge kept from this file. An entry dropped by the duplicate keys + * handling is not counted. + */ + int getFlags() { + return flags; + } + + /** + * The number of segment entries the merge kept from this file. + */ + int getSegments() { + return segments; + } + } + + /** + * The merged result of one full load. + */ + static final class LoadResult { + private final Iterable>> data; + private final List files; + private final int flagCount; + private final int segmentCount; + private final byte[] contentHash; + + LoadResult( + Iterable>> data, + List files, + int flagCount, + int segmentCount, + byte[] contentHash + ) { + this.data = data; + this.files = Collections.unmodifiableList(new ArrayList<>(files)); + this.flagCount = flagCount; + this.segmentCount = segmentCount; + this.contentHash = contentHash; + } + + Iterable>> getData() { + return data; + } + + /** + * One summary per configured file, in configuration order. + */ + List getFiles() { + return files; + } + + int getFlagCount() { + return flagCount; + } + + int getSegmentCount() { + return segmentCount; + } + + /** + * A digest of the raw content of every file that was read. Two loads with an equal digest produced + * the same data. The digest may be null when the runtime cannot compute one. + */ + byte[] getContentHash() { + return contentHash; + } + } + + private final List paths; + private final FileData.DuplicateKeysHandling duplicateKeysHandling; + + /** + * Creates a loader. + * + * @param paths the files to read, in precedence order + * @param duplicateKeysHandling what to do when the same key appears in more than one file + */ + OverrideFileLoader(List paths, FileData.DuplicateKeysHandling duplicateKeysHandling) { + this.paths = new ArrayList<>(paths); + this.duplicateKeysHandling = duplicateKeysHandling; + } + + /** + * Reads every configured file in order and merges the entries. + * + * @return the merged result with a summary of each file + * @throws FileDataException if a file that exists cannot be read or parsed, or the files cannot be merged + */ + LoadResult load() throws FileDataException { + MessageDigest digest = newDigest(); + Map> data = new HashMap<>(); + List files = new ArrayList<>(paths.size()); + for (Path path : paths) { + byte[] raw; + try { + raw = Files.readAllBytes(path); + } catch (NoSuchFileException e) { + // Absence is a valid state, not a failure. + files.add(new FileSummary(path, false, 0, 0)); + continue; + } catch (IOException e) { + throw new FileDataException("file " + path + ": unable to read file", e); + } + // One read feeds both the digest and the parse, so the skip-unchanged digest can never disagree + // with the content that was applied. + if (digest != null) { + digest.update(raw); + digest.update((byte) 0); + } + int flagsAdded = 0; + int segmentsAdded = 0; + try { + FlagFileParser parser = FlagFileParser.selectForContent(raw); + FlagFileRep fileContents = parser.parse(new ByteArrayInputStream(raw)); + if (fileContents.flags != null) { + for (Map.Entry e : fileContents.flags.entrySet()) { + if (add(data, FEATURES, e.getKey(), FlagFactory.flagFromJson(e.getValue()))) { + flagsAdded++; + } + } + } + if (fileContents.flagValues != null) { + for (Map.Entry e : fileContents.flagValues.entrySet()) { + if (add(data, FEATURES, e.getKey(), FlagFactory.offFlagWithValue(e.getKey(), e.getValue()))) { + flagsAdded++; + } + } + } + if (fileContents.segments != null) { + for (Map.Entry e : fileContents.segments.entrySet()) { + if (add(data, SEGMENTS, e.getKey(), FlagFactory.segmentFromJson(e.getValue()))) { + segmentsAdded++; + } + } + } + } catch (FileDataException e) { + throw new FileDataException("file " + path + ": " + e.getMessage(), e.getCause()); + } catch (IOException e) { + throw new FileDataException("file " + path + ": cannot read document", e); + } catch (RuntimeException e) { + // A document can be valid JSON or YAML and still hold a flag or segment that the data model + // cannot accept. For the override source that is a parse failure of the file, so the last good + // overrides stay in effect. + throw new FileDataException("file " + path + ": cannot parse flag or segment data", e); + } + files.add(new FileSummary(path, true, flagsAdded, segmentsAdded)); + } + + ImmutableList.Builder>> all = ImmutableList.builder(); + for (Map.Entry> e : data.entrySet()) { + all.add(new AbstractMap.SimpleEntry<>(e.getKey(), new KeyedItems<>(ImmutableList.copyOf(e.getValue().entrySet())))); + } + Map flags = data.get(FEATURES); + Map segments = data.get(SEGMENTS); + return new LoadResult(all.build(), files, flags == null ? 0 : flags.size(), + segments == null ? 0 : segments.size(), digest == null ? null : digest.digest()); + } + + // Adds an entry unless the key was already added. Reports whether it added the entry. + private boolean add(Map> data, DataKind kind, String key, ItemDescriptor item) + throws FileDataException { + Map items = data.computeIfAbsent(kind, k -> new HashMap<>()); + if (items.containsKey(key)) { + if (duplicateKeysHandling == FileData.DuplicateKeysHandling.IGNORE) { + return false; + } + throw new FileDataException("in " + kind.getName() + ", key \"" + key + "\" was already defined", null); + } + items.put(key, item); + return true; + } + + private static MessageDigest newDigest() { + try { + return MessageDigest.getInstance("SHA-256"); + } catch (NoSuchAlgorithmException e) { + // COVERAGE: every Java runtime provides SHA-256 + return null; + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataLoadingBehaviorTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataLoadingBehaviorTest.java new file mode 100644 index 00000000..87b1ffd4 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataLoadingBehaviorTest.java @@ -0,0 +1,80 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.sdk.server.datasources.Synchronizer; +import com.launchdarkly.sdk.server.integrations.FileDataSourceBase.DataBuilder; +import com.launchdarkly.sdk.server.integrations.FileDataSourceBase.DataLoader; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; +import com.launchdarkly.sdk.server.subsystems.SerializationException; +import com.launchdarkly.testhelpers.TempDir; +import com.launchdarkly.testhelpers.TempFile; + +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.fail; + +/** + * Pins the error behavior of the file data source's loader. A document that parses but holds a flag + * that the data model rejects is not a file data error: the deserialization exception escapes as it + * always has. The exception description requires a cause. The override source has its own loader + * with different rules. + */ +@SuppressWarnings("javadoc") +public class FileDataLoadingBehaviorTest { + private static final String MODEL_REJECTED_DOCUMENT = "{\"flags\":{\"f1\":{\"key\":\"f1\",\"rules\":5}}}"; + + @Test + public void modelRejectedDocumentEscapesTheLoaderAsARuntimeException() throws Exception { + try (TempDir dir = TempDir.create()) { + try (TempFile file = dir.tempFile(".json")) { + file.setContents(MODEL_REJECTED_DOCUMENT); + DataLoader loader = new DataLoader(FileData.dataSource().filePaths(file.getPath()).sources); + try { + loader.load(new DataBuilder(FileData.DuplicateKeysHandling.FAIL)); + fail("expected exception"); + } catch (FileDataException e) { + fail("the loader must not convert a model rejection into a file data error"); + } catch (RuntimeException e) { + assertThat(e, instanceOf(SerializationException.class)); + } + } + } + } + + @Test + public void modelRejectedDocumentEscapesTheSynchronizerAsARuntimeException() throws Exception { + try (TempDir dir = TempDir.create()) { + try (TempFile file = dir.tempFile(".json")) { + file.setContents(MODEL_REJECTED_DOCUMENT); + try (Synchronizer synchronizer = FileData.synchronizer().filePaths(file.getPath()) + .build(TestDataSourceBuildInputs.create(LDLogger.none()))) { + try { + synchronizer.next(); + fail("expected exception"); + } catch (RuntimeException e) { + assertThat(e, instanceOf(SerializationException.class)); + } + } + } + } + } + + @Test + public void exceptionDescriptionRequiresACause() { + FileDataException withCause = new FileDataException("message", new RuntimeException("boom"), null); + assertThat(withCause.getDescription(), equalTo("message [java.lang.RuntimeException: boom]")); + + // A duplicate key failure carries no cause. The file data source builds its own description for + // that case rather than calling this method. + FileDataException withoutCause = new FileDataException("message", null, null); + try { + withoutCause.getDescription(); + fail("expected exception"); + } catch (NullPointerException e) { + // current behavior + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java new file mode 100644 index 00000000..b243d968 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java @@ -0,0 +1,182 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.testhelpers.TempDir; + +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.FileTime; +import java.time.Duration; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.lessThan; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("javadoc") +public class FileDataPollerTest { + private static final Duration INTERVAL = Duration.ofMillis(20); + private static final long WAIT_MILLIS = 5000; + private static final long QUIET_MILLIS = 250; + + private static void awaitCount(AtomicInteger counter, int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (counter.get() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for " + expected + " change signals, got " + counter.get()); + } + Thread.sleep(5); + } + } + + private static Path createFile(TempDir dir, String name, String contents, long modifiedMillis) throws Exception { + Path path = dir.getPath().resolve(name); + Files.write(path, contents.getBytes()); + Files.setLastModifiedTime(path, FileTime.fromMillis(modifiedMillis)); + return path; + } + + private static void rewrite(Path path, String contents, long modifiedMillis) throws Exception { + Files.write(path, contents.getBytes()); + Files.setLastModifiedTime(path, FileTime.fromMillis(modifiedMillis)); + } + + @Test + public void detectsModifiedTimeChange() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + Thread.sleep(QUIET_MILLIS); + assertEquals(0, changes.get()); + + rewrite(file, "{}", 2000_000); // same size, new modified time + awaitCount(changes, 1); + } + } + } + + @Test + public void detectsSizeChangeWithSameModifiedTime() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + rewrite(file, "{\"flagValues\":{}}", 1000_000); + awaitCount(changes, 1); + } + } + } + + @Test + public void firesOncePerChange() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + rewrite(file, "{}", 2000_000); + awaitCount(changes, 1); + Thread.sleep(QUIET_MILLIS); + assertEquals(1, changes.get()); + } + } + } + + @Test + public void detectsFileAppearing() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("later.json"); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + Thread.sleep(QUIET_MILLIS); + assertEquals(0, changes.get()); + + Files.write(file, "{}".getBytes()); + awaitCount(changes, 1); + } + } + } + + @Test + public void detectsFileDisappearing() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + Files.delete(file); + awaitCount(changes, 1); + } + } + } + + @Test + public void watchesEveryConfiguredFile() throws Exception { + try (TempDir dir = TempDir.create()) { + Path first = createFile(dir, "a.json", "{}", 1000_000); + Path second = createFile(dir, "b.json", "{}", 1000_000); + List paths = Arrays.asList(first, second); + AtomicInteger changes = new AtomicInteger(); + try (FileDataPoller poller = new FileDataPoller(paths, INTERVAL, changes::incrementAndGet)) { + rewrite(second, "{}", 2000_000); + awaitCount(changes, 1); + rewrite(first, "{}", 2000_000); + awaitCount(changes, 2); + } + } + } + + @Test + public void stopsOnClose() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet); + poller.close(); + poller.close(); // idempotent + + rewrite(file, "{}", 2000_000); + Thread.sleep(QUIET_MILLIS); + assertEquals(0, changes.get()); + } + } + + @Test + public void closeReturnsWhileCallbackBlocks() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, () -> { + entered.countDown(); + try { + release.await(WAIT_MILLIS, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); + rewrite(file, "{}", 2000_000); + assertTrue(entered.await(WAIT_MILLIS, TimeUnit.MILLISECONDS)); + + long start = System.currentTimeMillis(); + poller.close(); + assertThat(System.currentTimeMillis() - start, lessThan(1000L)); + release.countDown(); + } + } + + @Test + public void unreadableAttributesCountAsAbsent() throws Exception { + try (TempDir dir = TempDir.create()) { + Path missing = dir.getPath().resolve("nope").resolve("missing.json"); + List states = FileDataPoller.observeAll(Collections.singletonList(missing)); + assertEquals(FileDataPoller.FileState.ABSENT, states.get(0)); + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java new file mode 100644 index 00000000..4c2c1704 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java @@ -0,0 +1,512 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; +import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; + +import org.junit.After; +import org.junit.Test; + +import java.time.Duration; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingQueue; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Supplier; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +@SuppressWarnings("javadoc") +public class FileDataReloaderTest { + private static final Duration SHORT = Duration.ofMillis(50); + private static final long WAIT_MILLIS = 5000; + + private final LogCapture logCapture = Logs.capture(); + private final LDLogger logger = LDLogger.withAdapter(logCapture, ""); + private final List reloaders = new ArrayList<>(); + + @After + public void closeReloaders() { + for (FileDataReloader r : reloaders) { + r.close(); + } + } + + private static LoadResult resultWithHash(int... values) { + byte[] hash = new byte[values.length]; + for (int i = 0; i < values.length; i++) { + hash[i] = (byte) values[i]; + } + return new LoadResult(Collections.emptyList(), Collections.emptyList(), 0, 0, hash); + } + + private static LoadResult resultWithoutHash() { + return new LoadResult(Collections.emptyList(), Collections.emptyList(), 0, 0, null); + } + + private static FileDataException failure(String message) { + return new FileDataException(message, null, null); + } + + /** + * A scripted loader: each call takes the next step from the queue. A step either returns a result + * or throws. When the queue is empty the last step repeats. + */ + private static final class ScriptedLoader implements FileDataReloader.Loader { + final AtomicInteger calls = new AtomicInteger(); + final LinkedBlockingQueue> steps = new LinkedBlockingQueue<>(); + volatile Supplier last; + volatile CountDownLatch entered; + volatile CountDownLatch release; + + ScriptedLoader then(LoadResult result) { + steps.add(() -> result); + return this; + } + + ScriptedLoader thenFail(String message) { + steps.add(() -> { + throw new UncheckedFileDataException(failure(message)); + }); + return this; + } + + @Override + public LoadResult load() throws FileDataException { + // Both latches are read before the entered signal, so a test that clears them after the + // signal cannot change what this call does. + CountDownLatch enteredLatch = entered; + CountDownLatch releaseLatch = release; + calls.incrementAndGet(); + if (enteredLatch != null) { + enteredLatch.countDown(); + } + if (releaseLatch != null) { + try { + releaseLatch.await(WAIT_MILLIS, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + Supplier step = steps.poll(); + if (step == null) { + step = last; + } else { + last = step; + } + try { + return step.get(); + } catch (UncheckedFileDataException e) { + throw e.wrapped; + } + } + } + + @SuppressWarnings("serial") + private static final class UncheckedFileDataException extends RuntimeException { + final FileDataException wrapped; + + UncheckedFileDataException(FileDataException wrapped) { + this.wrapped = wrapped; + } + } + + private static final class RecordingHandler implements FileDataReloader.Handler { + final List applied = Collections.synchronizedList(new ArrayList<>()); + final List errors = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void apply(LoadResult result) { + applied.add(result); + } + + @Override + public void onError(FileDataException e) { + errors.add(e); + } + } + + private FileDataReloader reloader(ScriptedLoader loader, RecordingHandler handler, + Duration debounce, Duration retry, boolean skipUnchanged) { + FileDataReloader r = new FileDataReloader(loader, handler, logger, debounce, retry, skipUnchanged); + reloaders.add(r); + return r; + } + + private static void awaitAtLeast(AtomicInteger counter, int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (counter.get() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for count " + expected + ", got " + counter.get()); + } + Thread.sleep(5); + } + } + + private static void awaitSize(List list, int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (list.size() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for size " + expected + ", got " + list.size()); + } + Thread.sleep(5); + } + } + + @Test + public void initialLoadAppliesResultSynchronously() { + LoadResult result = resultWithHash(1); + ScriptedLoader loader = new ScriptedLoader().then(result); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + + r.reloadNow(); + + assertEquals(1, loader.calls.get()); + assertEquals(1, handler.applied.size()); + assertEquals(result, handler.applied.get(0)); + assertTrue(handler.errors.isEmpty()); + } + + @Test + public void failedLoadReportsErrorAndAppliesNothing() { + ScriptedLoader loader = new ScriptedLoader().thenFail("bad file"); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + + r.reloadNow(); + + assertTrue(handler.applied.isEmpty()); + assertEquals(1, handler.errors.size()); + assertThat(FileDataReloader.describe(handler.errors.get(0)), equalTo("bad file")); + assertThat(logCapture.getMessageStrings(), hasItem("ERROR:Unable to load flags: bad file")); + } + + @Test + public void reloaderThatIsNeverTriggeredStartsNoThread() { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, SHORT, SHORT, true); + + r.reloadNow(); + + assertFalse(r.hasWorker()); + } + + @Test + public void triggerReloadsAfterDebounceDelay() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, SHORT, Duration.ZERO, true); + r.reloadNow(); + + r.trigger(); + + awaitSize(handler.applied, 2); + assertTrue(r.hasWorker()); + assertThat(logCapture.getMessageStrings(), hasItem("INFO:Reloading flag data after detecting a change")); + } + + @Test + public void debounceCoalescesBurstOfTriggersIntoOneReload() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ofMillis(150), Duration.ZERO, false); + r.reloadNow(); + + for (int i = 0; i < 10; i++) { + r.trigger(); + } + + awaitSize(handler.applied, 2); + Thread.sleep(400); + assertEquals(2, loader.calls.get()); + assertEquals(2, handler.applied.size()); + } + + @Test + public void debounceWindowIsExtendedByEachTrigger() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ofMillis(200), Duration.ZERO, false); + r.reloadNow(); + + // Triggers arrive every 50ms for 400ms. The window is 200ms, so no reload happens while they + // keep arriving. + for (int i = 0; i < 8; i++) { + r.trigger(); + Thread.sleep(50); + } + assertEquals(1, loader.calls.get()); + + awaitSize(handler.applied, 2); + Thread.sleep(300); + assertEquals(2, loader.calls.get()); + } + + @Test + public void zeroDebounceReloadsOnEveryTrigger() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + r.reloadNow(); + + r.trigger(); + awaitAtLeast(loader.calls, 2); + r.trigger(); + awaitAtLeast(loader.calls, 3); + + awaitSize(handler.applied, 3); + } + + @Test + public void failedInitialLoadRetriesWithoutFurtherTriggers() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenFail("first").thenFail("first").then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + + r.reloadNow(); + assertTrue(r.hasWorker()); + + awaitSize(handler.applied, 1); + assertThat(loader.calls.get(), greaterThanOrEqualTo(3)); + assertThat(logCapture.getMessageStrings(), hasItem("DEBUG:Retrying flag data load after earlier failure")); + } + + @Test + public void identicalRepeatedFailureIsReportedOnceUntilItChanges() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenFail("same").thenFail("same").thenFail("same") + .thenFail("different").then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + + r.reloadNow(); + + awaitSize(handler.applied, 1); + assertEquals(2, handler.errors.size()); + assertThat(FileDataReloader.describe(handler.errors.get(0)), equalTo("same")); + assertThat(FileDataReloader.describe(handler.errors.get(1)), equalTo("different")); + List messages = logCapture.getMessageStrings(); + assertEquals(1, messages.stream().filter(m -> m.equals("ERROR:Unable to load flags: same")).count()); + assertThat(messages, hasItem("DEBUG:Unable to load flags: same")); + assertThat(messages, hasItem("ERROR:Unable to load flags: different")); + } + + @Test + public void successReArmsFailureReporting() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenFail("same").then(resultWithHash(1)).thenFail("same") + .then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + + r.reloadNow(); // fails, retry applies result 1 + awaitSize(handler.applied, 1); + r.trigger(); // fails again with the same message, retry applies result 2 + awaitSize(handler.applied, 2); + + assertEquals(2, handler.errors.size()); + } + + @Test + public void retryStopsAfterSuccess() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenFail("first").then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + + r.reloadNow(); + awaitSize(handler.applied, 1); + int callsAfterSuccess = loader.calls.get(); + + Thread.sleep(300); + assertEquals(callsAfterSuccess, loader.calls.get()); + } + + @Test + public void reloadWithUnchangedContentIsSkipped() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(1)).then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + + r.reloadNow(); + r.trigger(); + awaitAtLeast(loader.calls, 2); + r.trigger(); + awaitAtLeast(loader.calls, 3); + + awaitSize(handler.applied, 2); + assertEquals(2, handler.applied.size()); + } + + @Test + public void recoveryAppliesEvenWhenContentIsUnchanged() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).thenFail("broken").then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + + r.reloadNow(); + r.trigger(); + awaitSize(handler.errors, 1); + r.trigger(); + + awaitSize(handler.applied, 2); + } + + @Test + public void everyReloadIsAppliedWhenSkipUnchangedIsOff() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + + r.reloadNow(); + r.trigger(); + + awaitSize(handler.applied, 2); + } + + @Test + public void nullHashIsNeverTreatedAsUnchanged() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithoutHash()); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + + r.reloadNow(); + r.trigger(); + + awaitSize(handler.applied, 2); + } + + @Test + public void closeStopsFurtherReloads() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + r.reloadNow(); + + r.close(); + r.trigger(); + r.reloadNow(); + Thread.sleep(100); + + assertEquals(1, loader.calls.get()); + assertEquals(1, handler.applied.size()); + } + + @Test + public void closeCancelsPendingRetry() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenFail("broken"); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ofMillis(100), true); + r.reloadNow(); + + r.close(); + Thread.sleep(300); + + assertEquals(1, loader.calls.get()); + } + + @Test + public void closeIsIdempotent() { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + FileDataReloader r = reloader(loader, new RecordingHandler(), SHORT, SHORT, true); + r.trigger(); + r.close(); + r.close(); + } + + @Test + public void closeDoesNotWaitForInFlightReloadAndDropsItsResult() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)); + loader.entered = new CountDownLatch(1); + loader.release = new CountDownLatch(1); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + + r.trigger(); + assertTrue(loader.entered.await(WAIT_MILLIS, TimeUnit.MILLISECONDS)); + + long start = System.currentTimeMillis(); + r.close(); + assertThat(System.currentTimeMillis() - start, lessThan(1000L)); + + loader.release.countDown(); + Thread.sleep(200); + assertTrue(handler.applied.isEmpty()); + } + + @Test + public void reloadsAreSerializedBetweenCallerAndWorker() throws Exception { + // The loader blocks on the worker thread. A synchronous reload from the caller must wait for + // it rather than run concurrently. + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + CountDownLatch entered = new CountDownLatch(1); + CountDownLatch release = new CountDownLatch(1); + loader.entered = entered; + loader.release = release; + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + + r.trigger(); + assertTrue(entered.await(WAIT_MILLIS, TimeUnit.MILLISECONDS)); + // Later calls must not block. + loader.entered = null; + loader.release = null; + + AtomicReference syncReloadFinished = new AtomicReference<>(false); + Thread caller = new Thread(() -> { + r.reloadNow(); + syncReloadFinished.set(true); + }); + caller.start(); + Thread.sleep(150); + assertFalse(syncReloadFinished.get()); + assertEquals(1, loader.calls.get()); + + release.countDown(); + caller.join(WAIT_MILLIS); + assertTrue(syncReloadFinished.get()); + assertEquals(2, loader.calls.get()); + assertEquals(2, handler.applied.size()); + } + + @Test + public void unexpectedRuntimeExceptionFromHandlerIsLoggedAndWorkerSurvives() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + AtomicInteger applies = new AtomicInteger(); + FileDataReloader.Handler handler = new FileDataReloader.Handler() { + @Override + public void apply(LoadResult result) { + if (applies.incrementAndGet() == 1) { + throw new IllegalStateException("consumer failed"); + } + } + + @Override + public void onError(FileDataException e) { + } + }; + FileDataReloader r = new FileDataReloader(loader, handler, logger, Duration.ZERO, Duration.ZERO, false); + reloaders.add(r); + + r.trigger(); + awaitAtLeast(applies, 1); + r.trigger(); + awaitAtLeast(applies, 2); + + assertThat(logCapture.getMessageStrings(), + hasItem(startsWith("ERROR:Unexpected error while reloading flag data:"))); + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java new file mode 100644 index 00000000..af58b078 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java @@ -0,0 +1,166 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.testhelpers.TempDir; + +import org.junit.Test; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Collections; +import java.util.concurrent.atomic.AtomicInteger; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +@SuppressWarnings("javadoc") +public class FileDataWatcherTest { + private static final LDLogger testLogger = LDLogger.none(); + private static final long WAIT_MILLIS = 10000; + private static final long QUIET_MILLIS = 300; + + private static void awaitAtLeast(AtomicInteger counter, int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (counter.get() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for " + expected + " change signals, got " + counter.get()); + } + Thread.sleep(10); + } + } + + @Test + public void startSignalsOnceSoThatAChangeBeforeWatchingIsNotMissed() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + assertEquals(1, changes.get()); + } + } + } + + @Test + public void modifiedFileSignalsChange() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + Files.write(file, "{\"flagValues\":{}}".getBytes()); + awaitAtLeast(changes, 2); + } + } + } + + @Test + public void fileThatDoesNotExistYetSignalsChangeWhenItAppears() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("later.json"); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + Files.write(file, "{}".getBytes()); + awaitAtLeast(changes, 2); + } + } + } + + @Test + public void deletedFileSignalsChange() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + Files.delete(file); + awaitAtLeast(changes, 2); + } + } + } + + @Test + public void unrelatedFileInSameDirectoryDoesNotSignal() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + Files.write(dir.getPath().resolve("other.json"), "{}".getBytes()); + Thread.sleep(QUIET_MILLIS); + assertEquals(1, changes.get()); + } + } + } + + @Test + public void relativePathIsWatchedByItsAbsoluteLocation() throws Exception { + // The watcher must compare the directory-relative event name against the same absolute form + // it registered, whatever form the caller used. + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("sub").resolve("..").resolve("a.json"); + Files.createDirectories(dir.getPath().resolve("sub")); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + Files.write(file, "{\"flagValues\":{}}".getBytes()); + awaitAtLeast(changes, 2); + } + } + } + + @Test + public void missingDirectoryIsAnErrorAtCreation() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("nope").resolve("a.json"); + try { + FileDataWatcher.create(Collections.singletonList(file), testLogger).close(); + fail("expected IOException"); + } catch (IOException e) { + // expected + } + } + } + + @Test + public void closeStopsSignalsAndIsIdempotent() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), testLogger); + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + watcher.close(); + watcher.close(); + + Files.write(file, "{\"flagValues\":{}}".getBytes()); + Thread.sleep(QUIET_MILLIS); + assertEquals(1, changes.get()); + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileSynchronizerReloadBehaviorTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileSynchronizerReloadBehaviorTest.java new file mode 100644 index 00000000..ab0e592c --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileSynchronizerReloadBehaviorTest.java @@ -0,0 +1,204 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogLevel; +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; +import com.launchdarkly.sdk.fdv2.SourceResultType; +import com.launchdarkly.sdk.fdv2.SourceSignal; +import com.launchdarkly.sdk.server.datasources.FDv2SourceResult; +import com.launchdarkly.sdk.server.datasources.Synchronizer; +import com.launchdarkly.sdk.server.interfaces.DataSourceStatusProvider.ErrorKind; +import com.launchdarkly.testhelpers.TempDir; +import com.launchdarkly.testhelpers.TempFile; + +import org.junit.Test; + +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import static com.launchdarkly.sdk.server.integrations.FileDataSourceTestData.getResourceContents; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.fail; + +/** + * Pins the reload behavior of the file data source's synchronizer: it reloads on every file system + * event, reports every failed load, does not retry on its own, and logs a failure as the plain + * description at error level. The override source has different rules and its own tests. + */ +@SuppressWarnings("javadoc") +public class FileSynchronizerReloadBehaviorTest { + private static final long CHANGE_TIMEOUT_SECONDS = 15; + private static final String MALFORMED = "{\"flags\""; + + private final LogCapture logCapture = Logs.capture(); + private final LDLogger logger = LDLogger.withAdapter(logCapture, ""); + + private Synchronizer autoUpdatingSynchronizer(TempFile file) { + return FileData.synchronizer() + .filePaths(file.getPath()) + .autoUpdate(true) + .build(TestDataSourceBuildInputs.create(logger)); + } + + private static FDv2SourceResult await(CompletableFuture future) throws Exception { + return future.get(CHANGE_TIMEOUT_SECONDS, TimeUnit.SECONDS); + } + + // Consumes results until none arrives for a short quiet period, then returns the one future that is + // still outstanding. A single edit can produce more than one file system event, and the synchronizer + // reloads on each of them. Only one next() future is ever left outstanding, because the queue + // delivers each result to the oldest waiting future, and a forgotten one would swallow a result. + private static CompletableFuture settle(Synchronizer synchronizer) throws Exception { + CompletableFuture pending = synchronizer.next(); + while (true) { + try { + pending.get(500, TimeUnit.MILLISECONDS); + pending = synchronizer.next(); + } catch (TimeoutException e) { + return pending; + } + } + } + + private void awaitErrorMessages(int atLeast) throws Exception { + long deadline = System.currentTimeMillis() + CHANGE_TIMEOUT_SECONDS * 1000; + while (errorMessages().size() < atLeast) { + if (System.currentTimeMillis() > deadline) { + fail("expected at least " + atLeast + " error logs but saw " + errorMessages()); + } + Thread.sleep(50); + } + } + + private List errorMessages() { + List out = new java.util.ArrayList<>(); + for (LogCapture.Message m : logCapture.getMessages()) { + if (m.getLevel() == LDLogLevel.ERROR) { + out.add(m.getText()); + } + } + return out; + } + + @Test + public void rewriteWithIdenticalContentEmitsAnotherChangeSet() throws Exception { + try (TempDir dir = TempDir.create()) { + try (TempFile file = dir.tempFile(".json")) { + String contents = getResourceContents("flag-only.json"); + file.setContents(contents); + try (Synchronizer synchronizer = autoUpdatingSynchronizer(file)) { + assertThat(await(synchronizer.next()).getResultType(), equalTo(SourceResultType.CHANGE_SET)); + + CompletableFuture next = synchronizer.next(); + Thread.sleep(200); // let the watcher register before the change + file.setContents(contents); // same bytes, new modification time + + // The content did not change, and the synchronizer still delivers a new change set. + assertThat(await(next).getResultType(), equalTo(SourceResultType.CHANGE_SET)); + } + } + } + } + + @Test + public void everyFailedReloadIsReportedAndLoggedAsThePlainDescription() throws Exception { + try (TempDir dir = TempDir.create()) { + try (TempFile file = dir.tempFile(".json")) { + file.setContents(getResourceContents("flag-only.json")); + try (Synchronizer synchronizer = autoUpdatingSynchronizer(file)) { + assertThat(await(synchronizer.next()).getResultType(), equalTo(SourceResultType.CHANGE_SET)); + + CompletableFuture next = synchronizer.next(); + Thread.sleep(200); + file.setContents(MALFORMED); + FDv2SourceResult first = await(next); + assertThat(first.getResultType(), equalTo(SourceResultType.STATUS)); + assertThat(first.getStatus().getState(), equalTo(SourceSignal.INTERRUPTED)); + assertThat(first.getStatus().getErrorInfo().getKind(), equalTo(ErrorKind.INVALID_DATA)); + CompletableFuture pending = settle(synchronizer); + int errorsAfterFirstFailure = errorMessages().size(); + + // The same malformed content again is the same failure. It is reported again, not deduplicated. + file.setContents(MALFORMED); + FDv2SourceResult second = await(pending); + assertThat(second.getResultType(), equalTo(SourceResultType.STATUS)); + assertThat(second.getStatus().getState(), equalTo(SourceSignal.INTERRUPTED)); + assertEquals(first.getStatus().getErrorInfo().getMessage(), second.getStatus().getErrorInfo().getMessage()); + awaitErrorMessages(errorsAfterFirstFailure + 1); + + // Each failure is logged at error level as the plain description: the parser message and + // the cause in brackets. There is no prefix, and the description does not name the file. + List errors = errorMessages(); + assertThat(errors.size(), greaterThanOrEqualTo(2)); + for (String message : errors) { + assertThat(message, startsWith("cannot parse JSON [com.google.gson.JsonSyntaxException")); + assertThat(message, endsWith("]")); + assertThat(message, not(containsString(file.getPath().toString()))); + } + assertEquals(errors.get(0), first.getStatus().getErrorInfo().getMessage()); + } + } + } + } + + @Test + public void failedReloadIsNotRetriedWithoutAFileChange() throws Exception { + try (TempDir dir = TempDir.create()) { + try (TempFile file = dir.tempFile(".json")) { + file.setContents(getResourceContents("flag-only.json")); + try (Synchronizer synchronizer = autoUpdatingSynchronizer(file)) { + assertThat(await(synchronizer.next()).getResultType(), equalTo(SourceResultType.CHANGE_SET)); + + CompletableFuture next = synchronizer.next(); + Thread.sleep(200); + file.setContents(MALFORMED); + assertThat(await(next).getStatus().getState(), equalTo(SourceSignal.INTERRUPTED)); + CompletableFuture later = settle(synchronizer); + int errorsAfterFailure = errorMessages().size(); + + // With the file untouched, nothing reloads: no result, no further error log, no retry log. + try { + FDv2SourceResult unexpected = later.get(2, TimeUnit.SECONDS); + fail("unexpected reload result: " + unexpected.getResultType()); + } catch (TimeoutException e) { + // expected + } + assertEquals(errorsAfterFailure, errorMessages().size()); + for (LogCapture.Message m : logCapture.getMessages()) { + assertFalse(m.getText(), m.getText().contains("Retrying")); + } + assertFalse(later.isDone()); + } + } + } + } + + @Test + public void missingFileAtStartupIsAnErrorLoggedWithThePath() throws Exception { + try (TempDir dir = TempDir.create()) { + java.nio.file.Path missing = dir.getPath().resolve("missing.json"); + try (Synchronizer synchronizer = FileData.synchronizer().filePaths(missing) + .build(TestDataSourceBuildInputs.create(logger))) { + FDv2SourceResult result = await(synchronizer.next()); + assertThat(result.getResultType(), equalTo(SourceResultType.STATUS)); + assertThat(result.getStatus().getState(), equalTo(SourceSignal.INTERRUPTED)); + // The description is the cause in brackets. The path appears only inside the cause text. + List errors = errorMessages(); + assertEquals(1, errors.size()); + assertEquals("[java.nio.file.NoSuchFileException: " + missing + "]", errors.get(0)); + assertEquals(errors.get(0), result.getStatus().getErrorInfo().getMessage()); + } + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java new file mode 100644 index 00000000..d244316c --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java @@ -0,0 +1,290 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; +import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.FileSummary; +import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.DataKind; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; +import com.launchdarkly.sdk.server.subsystems.SerializationException; +import com.launchdarkly.testhelpers.TempDir; + +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.launchdarkly.sdk.server.DataModel.FEATURES; +import static com.launchdarkly.sdk.server.DataModel.SEGMENTS; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + +@SuppressWarnings("javadoc") +public class OverrideFileLoaderTest { + private static Map> toMap(LoadResult result) { + Map> out = new HashMap<>(); + for (Map.Entry> kind : result.getData()) { + Map items = new HashMap<>(); + for (Map.Entry item : kind.getValue().getItems()) { + items.put(item.getKey(), item.getValue()); + } + out.put(kind.getKey(), items); + } + return out; + } + + private static LDValue json(DataKind kind, ItemDescriptor item) { + return LDValue.parse(kind.serialize(item)); + } + + private static Path write(TempDir dir, String name, String contents) throws Exception { + Path p = dir.getPath().resolve(name); + Files.write(p, contents.getBytes()); + return p; + } + + private static OverrideFileLoader loader(Path... paths) { + return new OverrideFileLoader(Arrays.asList(paths), FileData.DuplicateKeysHandling.FAIL); + } + + @Test + public void loadsFlagsFlagValuesAndSegmentsKeepingDocumentVersions() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", + "{\"flags\":{\"f1\":{\"key\":\"f1\",\"version\":7,\"on\":true,\"variations\":[true],\"fallthrough\":{\"variation\":0}}}," + + "\"flagValues\":{\"f2\":\"x\"}," + + "\"segments\":{\"s1\":{\"key\":\"s1\",\"version\":9}}}"); + LoadResult result = loader(file).load(); + + Map> data = toMap(result); + assertEquals(7, data.get(FEATURES).get("f1").getVersion()); + assertEquals(LDValue.of(7), json(FEATURES, data.get(FEATURES).get("f1")).get("version")); + assertEquals(9, data.get(SEGMENTS).get("s1").getVersion()); + assertEquals(LDValue.of(9), json(SEGMENTS, data.get(SEGMENTS).get("s1")).get("version")); + assertEquals(2, result.getFlagCount()); + assertEquals(1, result.getSegmentCount()); + assertEquals(1, result.getFiles().size()); + assertTrue(result.getFiles().get(0).isPresent()); + assertEquals(2, result.getFiles().get(0).getFlags()); + assertEquals(1, result.getFiles().get(0).getSegments()); + assertEquals(file, result.getFiles().get(0).getPath()); + } + } + + @Test + public void valueOnlyEntryBecomesOffFlagServingTheValue() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", "{\"flagValues\":{\"f2\":\"x\"}}"); + LoadResult result = loader(file).load(); + ItemDescriptor f2 = toMap(result).get(FEATURES).get("f2"); + assertEquals(0, f2.getVersion()); + LDValue flag = json(FEATURES, f2); + assertEquals(LDValue.of("f2"), flag.get("key")); + assertEquals(LDValue.of(0), flag.get("version")); + assertEquals(LDValue.of(false), flag.get("on")); + assertEquals(LDValue.of(0), flag.get("offVariation")); + assertEquals(LDValue.buildArray().add("x").build(), flag.get("variations")); + } + } + + @Test + public void yamlIsAutoDetected() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", "flagValues:\n yaml-flag: \"override-value\"\n"); + LoadResult result = loader(file).load(); + assertEquals(LDValue.buildArray().add("override-value").build(), + json(FEATURES, toMap(result).get(FEATURES).get("yaml-flag")).get("variations")); + } + } + + @Test + public void missingFileContributesNothing() throws Exception { + try (TempDir dir = TempDir.create()) { + Path present = write(dir, "present.json", "{\"flagValues\":{\"flag\":\"value\"}}"); + Path missing = dir.getPath().resolve("missing.json"); + Path segments = write(dir, "segments.json", "{\"segments\":{\"s1\":{\"key\":\"s1\"}}}"); + LoadResult result = loader(present, missing, segments).load(); + + Map> data = toMap(result); + assertEquals(1, data.get(FEATURES).size()); + assertEquals(1, data.get(SEGMENTS).size()); + List files = result.getFiles(); + assertEquals(3, files.size()); + assertTrue(files.get(0).isPresent()); + assertEquals(1, files.get(0).getFlags()); + assertFalse(files.get(1).isPresent()); + assertEquals(missing, files.get(1).getPath()); + assertEquals(0, files.get(1).getFlags()); + assertTrue(files.get(2).isPresent()); + assertEquals(1, files.get(2).getSegments()); + } + } + + @Test + public void allFilesMissingProducesEmptyResult() throws Exception { + try (TempDir dir = TempDir.create()) { + LoadResult result = loader(dir.getPath().resolve("a.json"), dir.getPath().resolve("b.json")).load(); + assertEquals(0, result.getFlagCount()); + assertEquals(0, result.getSegmentCount()); + assertFalse(result.getData().iterator().hasNext()); + assertEquals(2, result.getFiles().size()); + } + } + + @Test + public void unreadableFileFailsTheLoad() throws Exception { + try (TempDir dir = TempDir.create()) { + // A directory exists but cannot be read as a file. + Path directory = dir.getPath().resolve("dir.json"); + Files.createDirectory(directory); + try { + loader(directory).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(directory.toString())); + assertThat(e.getMessage(), containsString("unable to read file")); + assertThat(e.getCause(), instanceOf(java.io.IOException.class)); + } + } + } + + @Test + public void malformedDocumentFailsTheLoadWithFileAttribution() throws Exception { + try (TempDir dir = TempDir.create()) { + Path good = write(dir, "good.json", "{\"flagValues\":{\"a\":1}}"); + Path bad = write(dir, "bad.json", "{\"flagValues\""); + try { + loader(good, bad).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(bad.toString())); + assertThat(e.getMessage(), containsString("cannot parse JSON")); + assertThat(e.getCause(), instanceOf(com.google.gson.JsonSyntaxException.class)); + } + } + } + + @Test + public void modelRejectedDocumentIsAFileDataError() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "bad-model.json", "{\"flags\":{\"f1\":{\"key\":\"f1\",\"rules\":5}}}"); + try { + loader(file).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(file.toString())); + assertThat(e.getMessage(), containsString("cannot parse flag or segment data")); + assertThat(e.getCause(), instanceOf(SerializationException.class)); + } + } + } + + @Test + public void duplicateKeysFailByDefaultAcrossFilesAndWithinAFile() throws Exception { + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.json", "{\"flagValues\":{\"flag\":\"a\"}}"); + Path b = write(dir, "b.json", "{\"flagValues\":{\"flag\":\"b\"}}"); + try { + loader(a, b).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString("in features, key \"flag\" was already defined")); + assertThat(e.getMessage(), containsString(b.toString())); + assertNull(e.getCause()); + } + + Path both = write(dir, "both.json", "{\"flags\":{\"x\":{\"key\":\"x\"}},\"flagValues\":{\"x\":true}}"); + try { + loader(both).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString("key \"x\" was already defined")); + } + + Path seg1 = write(dir, "seg1.json", "{\"segments\":{\"s\":{\"key\":\"s\"}}}"); + Path seg2 = write(dir, "seg2.json", "{\"segments\":{\"s\":{\"key\":\"s\"}}}"); + try { + loader(seg1, seg2).load(); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString("in segments, key \"s\" was already defined")); + } + } + } + + @Test + public void ignoreHandlingKeepsFirstFileAndCountsOnlyKeptEntries() throws Exception { + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.json", "{\"flagValues\":{\"shared\":\"first\"}}"); + Path second = write(dir, "second.json", "{\"flagValues\":{\"shared\":\"second\",\"only-second\":\"x\"}}"); + OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(first, second), FileData.DuplicateKeysHandling.IGNORE); + LoadResult result = loader.load(); + + Map flags = toMap(result).get(FEATURES); + assertEquals(LDValue.buildArray().add("first").build(), json(FEATURES, flags.get("shared")).get("variations")); + assertEquals(2, flags.size()); + assertEquals(1, result.getFiles().get(0).getFlags()); + assertEquals(1, result.getFiles().get(1).getFlags()); // the dropped duplicate is not counted + assertEquals(2, result.getFlagCount()); + } + } + + @Test + public void contentHashReflectsRawContentOfPresentFiles() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", "{\"flagValues\":{\"a\":1}}"); + Path missing = dir.getPath().resolve("missing.json"); + OverrideFileLoader loader = loader(file, missing); + byte[] first = loader.load().getContentHash(); + byte[] second = loader.load().getContentHash(); + assertArrayEquals(first, second); + + Files.write(file, "{\"flagValues\":{\"a\":2}}".getBytes()); + byte[] third = loader.load().getContentHash(); + assertThat(Arrays.equals(first, third), is(false)); + + // A file that appears changes the digest. + Files.write(missing, "{}".getBytes()); + byte[] fourth = loader.load().getContentHash(); + assertThat(Arrays.equals(third, fourth), is(false)); + } + } + + @Test + public void emptyDocumentContributesNothing() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "empty.json", "{}"); + LoadResult result = loader(file).load(); + assertEquals(0, result.getFlagCount()); + assertTrue(result.getFiles().get(0).isPresent()); + assertThat(result.getFiles().get(0).getFlags(), equalTo(0)); + } + } + + @Test + public void loaderReadsFilesInConfiguredOrder() throws Exception { + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.json", "{\"flagValues\":{\"k\":\"a\"}}"); + Path b = write(dir, "b.json", "{\"flagValues\":{\"k\":\"b\"}}"); + OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(b, a), FileData.DuplicateKeysHandling.IGNORE); + assertEquals(LDValue.buildArray().add("b").build(), + json(FEATURES, toMap(loader.load()).get(FEATURES).get("k")).get("variations")); + assertEquals(Collections.singletonList(b).get(0), loader.load().getFiles().get(0).getPath()); + } + } +} From b9ceace01326456735c71013df1ac448c883db05 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:16:17 +0000 Subject: [PATCH 2/9] fix: Expand value-only overrides to a flag served by fallthrough --- .../integrations/FileDataSourceParsing.java | 21 +++---------------- .../integrations/OverrideFileLoader.java | 2 +- .../integrations/OverrideFileLoaderTest.java | 7 ++++--- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java index bae5309d..56fb4f22 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java @@ -202,26 +202,11 @@ static ItemDescriptor flagFromJson(LDValue jsonTree) { return FEATURES.deserialize(jsonTree.toJsonString()); } - /** - * Constructs a flag that is off and serves the given value as its single variation for every - * context. The document supplies no version, so the flag has version zero. This is the shape - * that the override source uses for value-only entries. The file data source keeps - * {@link #flagWithValue(String, LDValue, int)}. - */ - static ItemDescriptor offFlagWithValue(String key, LDValue jsonValue) { - LDValue o = LDValue.buildObject() - .put("key", key) - .put("version", 0) - .put("on", false) - .put("offVariation", 0) - .put("variations", LDValue.buildArray().add(jsonValue).build()) - .build(); - return FEATURES.deserialize(o.toJsonString()); - } - /** * Constructs a flag that always returns the same value. This is done by giving it a single - * variation and setting the fallthrough variation to that. + * variation and setting the fallthrough variation to that. The file data source passes its + * load version. The override source passes version zero, because its documents supply no + * version for a value-only entry. */ static ItemDescriptor flagWithValue(String key, LDValue jsonValue, int version) { LDValue o = LDValue.buildObject() diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java index 77dca424..987056d0 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java @@ -187,7 +187,7 @@ LoadResult load() throws FileDataException { } if (fileContents.flagValues != null) { for (Map.Entry e : fileContents.flagValues.entrySet()) { - if (add(data, FEATURES, e.getKey(), FlagFactory.offFlagWithValue(e.getKey(), e.getValue()))) { + if (add(data, FEATURES, e.getKey(), FlagFactory.flagWithValue(e.getKey(), e.getValue(), 0))) { flagsAdded++; } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java index d244316c..2c00651c 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java @@ -87,7 +87,7 @@ public void loadsFlagsFlagValuesAndSegmentsKeepingDocumentVersions() throws Exce } @Test - public void valueOnlyEntryBecomesOffFlagServingTheValue() throws Exception { + public void valueOnlyEntryBecomesFallthroughFlagServingTheValue() throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "overrides.json", "{\"flagValues\":{\"f2\":\"x\"}}"); LoadResult result = loader(file).load(); @@ -96,8 +96,9 @@ public void valueOnlyEntryBecomesOffFlagServingTheValue() throws Exception { LDValue flag = json(FEATURES, f2); assertEquals(LDValue.of("f2"), flag.get("key")); assertEquals(LDValue.of(0), flag.get("version")); - assertEquals(LDValue.of(false), flag.get("on")); - assertEquals(LDValue.of(0), flag.get("offVariation")); + assertEquals(LDValue.of(true), flag.get("on")); + assertEquals(LDValue.ofNull(), flag.get("offVariation")); + assertEquals(LDValue.of(0), flag.get("fallthrough").get("variation")); assertEquals(LDValue.buildArray().add("x").build(), flag.get("variations")); } } From 71971c6f3bd269b8ecbdc4688f477d948f5d2128 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:25:07 +0000 Subject: [PATCH 3/9] fix: Register a watched directory again when it is missing or deleted A directory that does not exist at start, or whose watch key becomes invalid because it was deleted, is remembered as missing and registered again on a fixed schedule from the worker thread. A recovered directory signals one change so that files written while it was not watched are picked up. The worker ends on close. --- .../server/integrations/FileDataWatcher.java | 169 +++++++++++++++--- .../integrations/FileDataWatcherTest.java | 79 +++++++- 2 files changed, 213 insertions(+), 35 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java index 172f1a0f..70ceb6e8 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java @@ -7,13 +7,17 @@ import java.io.IOException; import java.nio.file.ClosedWatchServiceException; import java.nio.file.FileSystems; +import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.nio.file.WatchEvent; import java.nio.file.WatchKey; import java.nio.file.WatchService; import java.nio.file.Watchable; +import java.time.Duration; import java.util.HashSet; +import java.util.Iterator; import java.util.Set; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import static java.nio.file.StandardWatchEventKinds.ENTRY_CREATE; @@ -26,9 +30,10 @@ * is created, modified, or deleted. *

* The Java file system API reports changes at the directory level, so the watcher registers the - * parent directory of every file. A file that does not exist yet is reported when it appears, as - * long as its directory exists when the watcher is created. A directory that does not exist is an - * error from {@link #create(Iterable, LDLogger)}. + * parent directory of every file. A file that does not exist yet is reported when it appears. A + * directory that does not exist, or that is deleted while it is watched, is not an error. The + * watcher tries to register it again on a fixed schedule until it exists, and then signals one + * change so that a file written to it in the meantime is picked up. *

* The callback is invoked for every relevant notification, including the burst of notifications * that a single edit can produce, and once more right after the watcher starts so that a change @@ -36,23 +41,49 @@ * {@link FileDataReloader}, which coalesces the burst and skips no-op reloads. */ final class FileDataWatcher implements Closeable, Runnable { + /** + * The time between attempts to register a directory that cannot be watched because it does not + * exist. + */ + static final Duration DEFAULT_DIRECTORY_RETRY_DELAY = Duration.ofSeconds(1); + private final WatchService watchService; private final Set watchedFilePaths; + private final long directoryRetryDelayMillis; private final LDLogger logger; private final Thread thread; private final AtomicBoolean stopped = new AtomicBoolean(false); private volatile Runnable onChange; + // The directories that are not registered because they do not exist, and the time of the next + // attempt to register them. Both are written during creation and then only by the worker thread. + private final Set missingDirectories = new HashSet<>(); + private long nextRetryAtMillis; + /** * Creates a watcher for the given files. Nothing is watched until {@link #start(Runnable)}. * * @param filePaths the files to watch, absolute or relative to the working directory * @param logger the logger * @return the watcher - * @throws IOException if the watch service cannot be created or a parent directory cannot be - * registered, for example because it does not exist + * @throws IOException if the watch service cannot be created */ static FileDataWatcher create(Iterable filePaths, LDLogger logger) throws IOException { + return create(filePaths, DEFAULT_DIRECTORY_RETRY_DELAY, logger); + } + + /** + * Creates a watcher with a specific delay between attempts to register a directory that does not + * exist. Visible for tests. + * + * @param filePaths the files to watch, absolute or relative to the working directory + * @param directoryRetryDelay the time between registration attempts for a missing directory + * @param logger the logger + * @return the watcher + * @throws IOException if the watch service cannot be created + */ + static FileDataWatcher create(Iterable filePaths, Duration directoryRetryDelay, LDLogger logger) + throws IOException { Set directoryPaths = new HashSet<>(); Set absoluteFilePaths = new HashSet<>(); for (Path p : filePaths) { @@ -61,20 +92,27 @@ static FileDataWatcher create(Iterable filePaths, LDLogger logger) throws directoryPaths.add(absolutePath.getParent()); } WatchService ws = FileSystems.getDefault().newWatchService(); + FileDataWatcher watcher = new FileDataWatcher(ws, absoluteFilePaths, directoryRetryDelay, logger); try { for (Path d : directoryPaths) { - d.register(ws, ENTRY_CREATE, ENTRY_MODIFY, ENTRY_DELETE); + watcher.registerOrRememberAsMissing(d); } - } catch (IOException | RuntimeException e) { + } catch (RuntimeException e) { ws.close(); throw e; } - return new FileDataWatcher(ws, absoluteFilePaths, logger); + return watcher; } - private FileDataWatcher(WatchService watchService, Set watchedFilePaths, LDLogger logger) { + private FileDataWatcher( + WatchService watchService, + Set watchedFilePaths, + Duration directoryRetryDelay, + LDLogger logger + ) { this.watchService = watchService; this.watchedFilePaths = watchedFilePaths; + this.directoryRetryDelayMillis = Math.max(directoryRetryDelay.toMillis(), 1); this.logger = logger; this.thread = new Thread(this, "LaunchDarkly-FileDataWatcher"); this.thread.setDaemon(true); @@ -97,33 +135,100 @@ public void run() { while (!stopped.get()) { WatchKey key; try { - key = watchService.take(); // blocks until a change is available or the thread is interrupted + if (missingDirectories.isEmpty()) { + key = watchService.take(); // blocks until a change is available or the thread is interrupted + } else { + // Wake up for the next registration attempt even when no change arrives. + long waitMillis = nextRetryAtMillis - System.currentTimeMillis(); + key = waitMillis > 0 ? watchService.poll(waitMillis, TimeUnit.MILLISECONDS) : null; + } } catch (InterruptedException e) { continue; // if stopped, the loop condition ends the thread } catch (ClosedWatchServiceException e) { return; } - boolean watchedFileWasChanged = false; - for (WatchEvent event : key.pollEvents()) { - if (event.kind() == OVERFLOW) { - // Notifications were dropped, so a watched file may have changed. + if (key != null) { + processKey(key); + } + if (!missingDirectories.isEmpty() && System.currentTimeMillis() >= nextRetryAtMillis) { + retryMissingDirectories(); + } + } + } + + private void processKey(WatchKey key) { + boolean watchedFileWasChanged = false; + for (WatchEvent event : key.pollEvents()) { + if (event.kind() == OVERFLOW) { + // Notifications were dropped, so a watched file may have changed. + watchedFileWasChanged = true; + break; + } + Watchable w = key.watchable(); + Object context = event.context(); + if (w instanceof Path && context instanceof Path) { + Path absolutePath = ((Path) w).resolve((Path) context); + if (watchedFilePaths.contains(absolutePath)) { watchedFileWasChanged = true; break; } - Watchable w = key.watchable(); - Object context = event.context(); - if (w instanceof Path && context instanceof Path) { - Path absolutePath = ((Path) w).resolve((Path) context); - if (watchedFilePaths.contains(absolutePath)) { - watchedFileWasChanged = true; - break; - } - } } - key.reset(); // without this, the watch on this key stops working - if (watchedFileWasChanged && !stopped.get()) { - signal(); + } + // Without the reset, the watch on this key stops working. The reset fails when the key is no + // longer valid, which means that the directory was deleted. The watch on it is gone, so the + // directory is registered again once it exists. + if (!key.reset() && !stopped.get() && key.watchable() instanceof Path) { + Path directory = (Path) key.watchable(); + logger.warn("Directory {} no longer exists. It is watched again once it appears.", directory); + rememberAsMissing(directory); + } + if (watchedFileWasChanged && !stopped.get()) { + signal(); + } + } + + // Registers a directory with the watch service. A directory that cannot be registered is + // remembered as missing, so that the worker thread tries again on the retry schedule. + private void registerOrRememberAsMissing(Path directory) { + try { + directory.register(watchService, ENTRY_CREATE, ENTRY_MODIFY, ENTRY_DELETE); + } catch (NoSuchFileException e) { + logger.warn("Directory {} does not exist. It is watched once it appears.", directory); + rememberAsMissing(directory); + } catch (IOException e) { + logger.warn("Unable to watch directory {}: {}. The attempt is repeated until it succeeds.", directory, + LogValues.exceptionSummary(e)); + rememberAsMissing(directory); + } + } + + private void rememberAsMissing(Path directory) { + if (missingDirectories.isEmpty()) { + nextRetryAtMillis = System.currentTimeMillis() + directoryRetryDelayMillis; + } + missingDirectories.add(directory); + } + + // Tries to register every missing directory. A directory that is registered leaves the missing + // set, and one change is signalled so that files written to it while it was not watched are + // picked up. A directory that still cannot be registered stays in the set without a new log + // entry, because its absence was logged when it was first found missing. + private void retryMissingDirectories() { + boolean registered = false; + for (Iterator it = missingDirectories.iterator(); it.hasNext();) { + Path directory = it.next(); + try { + directory.register(watchService, ENTRY_CREATE, ENTRY_MODIFY, ENTRY_DELETE); + } catch (IOException | ClosedWatchServiceException e) { + continue; } + logger.info("Directory {} exists. Watching it for changes.", directory); + it.remove(); + registered = true; + } + nextRetryAtMillis = System.currentTimeMillis() + directoryRetryDelayMillis; + if (registered && !stopped.get()) { + signal(); } } @@ -136,6 +241,18 @@ private void signal() { } } + /** + * Waits for the worker thread to end after {@link #close()}. Visible for tests. + * + * @param timeoutMillis how long to wait + * @return true if the worker thread has ended + * @throws InterruptedException if the wait is interrupted + */ + boolean awaitStop(long timeoutMillis) throws InterruptedException { + thread.join(timeoutMillis); + return !thread.isAlive(); + } + /** * Stops watching and releases the watch service. It does not wait for the worker thread. */ diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java index af58b078..246ccf0d 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataWatcherTest.java @@ -1,24 +1,28 @@ package com.launchdarkly.sdk.server.integrations; +import com.launchdarkly.logging.LDLogLevel; import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; import com.launchdarkly.testhelpers.TempDir; import org.junit.Test; -import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.time.Duration; import java.util.Collections; import java.util.concurrent.atomic.AtomicInteger; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; +import static org.junit.Assert.assertTrue; @SuppressWarnings("javadoc") public class FileDataWatcherTest { private static final LDLogger testLogger = LDLogger.none(); private static final long WAIT_MILLIS = 10000; private static final long QUIET_MILLIS = 300; + private static final Duration DIRECTORY_RETRY = Duration.ofMillis(50); private static void awaitAtLeast(AtomicInteger counter, int expected) throws InterruptedException { long deadline = System.currentTimeMillis() + WAIT_MILLIS; @@ -134,18 +138,75 @@ public void relativePathIsWatchedByItsAbsoluteLocation() throws Exception { } @Test - public void missingDirectoryIsAnErrorAtCreation() throws Exception { + public void deletedAndRecreatedDirectoryIsWatchedAgain() throws Exception { + LogCapture logCapture = Logs.capture(); + LDLogger logger = LDLogger.withAdapter(logCapture, ""); try (TempDir dir = TempDir.create()) { - Path file = dir.getPath().resolve("nope").resolve("a.json"); - try { - FileDataWatcher.create(Collections.singletonList(file), testLogger).close(); - fail("expected IOException"); - } catch (IOException e) { - // expected + Path sub = dir.getPath().resolve("sub"); + Files.createDirectory(sub); + Path file = sub.resolve("a.json"); + Files.write(file, "{}".getBytes()); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), DIRECTORY_RETRY, logger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + // Delete the file and then its directory, and let the deletion notifications settle. + Files.delete(file); + Files.delete(sub); + awaitAtLeast(changes, 2); + Thread.sleep(QUIET_MILLIS); + int afterDeletion = changes.get(); + + // A file written into the recreated directory must be reported. + Files.createDirectory(sub); + Files.write(file, "{\"flagValues\":{}}".getBytes()); + awaitAtLeast(changes, afterDeletion + 1); + + // The lost directory is reported once, not on every registration attempt. + long warnings = logCapture.getMessages().stream().filter(m -> m.getLevel() == LDLogLevel.WARN).count(); + assertEquals(1, warnings); } } } + @Test + public void directoryMissingAtStartIsWatchedOnceItExists() throws Exception { + try (TempDir dir = TempDir.create()) { + Path sub = dir.getPath().resolve("later"); + Path file = sub.resolve("a.json"); + AtomicInteger changes = new AtomicInteger(); + try (FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), DIRECTORY_RETRY, testLogger)) { + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + Thread.sleep(QUIET_MILLIS); + + // Create the directory and write the file into it. + Files.createDirectory(sub); + Files.write(file, "{}".getBytes()); + // The file is reported even though its directory did not exist when watching began. + awaitAtLeast(changes, 2); + } + } + } + + @Test + public void closeEndsTheWorkerWhileADirectoryIsMissing() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = dir.getPath().resolve("later").resolve("a.json"); + AtomicInteger changes = new AtomicInteger(); + FileDataWatcher watcher = FileDataWatcher.create(Collections.singletonList(file), DIRECTORY_RETRY, testLogger); + watcher.start(changes::incrementAndGet); + awaitAtLeast(changes, 1); + + // Close while the worker is waiting for its next registration attempt. + watcher.close(); + // The worker must end instead of continuing to retry. + assertTrue(watcher.awaitStop(WAIT_MILLIS)); + } + } + @Test public void closeStopsSignalsAndIsIdempotent() throws Exception { try (TempDir dir = TempDir.create()) { From fd26195933ea46796d720ef508c9962835baad7e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:25:07 +0000 Subject: [PATCH 4/9] fix: Keep polling after the change callback throws A RuntimeException from the callback is logged at error level instead of ending the repeating task. --- .../server/integrations/FileDataPoller.java | 16 ++++++- .../integrations/FileDataPollerTest.java | 46 +++++++++++++++---- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java index 2cc5e488..954e22e6 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java @@ -1,5 +1,8 @@ package com.launchdarkly.sdk.server.integrations; +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogValues; + import java.io.Closeable; import java.io.IOException; import java.nio.file.Files; @@ -31,6 +34,7 @@ final class FileDataPoller implements Closeable { private final List paths; private final Runnable onChange; + private final LDLogger logger; private final ScheduledThreadPoolExecutor executor; private final AtomicBoolean closed = new AtomicBoolean(false); private List last; @@ -42,10 +46,12 @@ final class FileDataPoller implements Closeable { * @param paths the files to examine * @param interval the time between examinations * @param onChange called when any file changed since the previous examination + * @param logger receives log output about a callback that fails */ - FileDataPoller(List paths, Duration interval, Runnable onChange) { + FileDataPoller(List paths, Duration interval, Runnable onChange, LDLogger logger) { this.paths = new ArrayList<>(paths); this.onChange = onChange; + this.logger = logger; this.last = observeAll(this.paths); ThreadFactory threadFactory = runnable -> { Thread t = new Thread(runnable, "LaunchDarkly-FileDataPoller"); @@ -80,7 +86,13 @@ private void examine() { boolean changed = !current.equals(last); last = current; if (changed && !closed.get()) { - onChange.run(); + try { + onChange.run(); + } catch (RuntimeException e) { + // The executor would otherwise cancel the repeating task and the poller would go quiet. + logger.error("Unexpected error while handling a file change: {}", LogValues.exceptionSummary(e)); + logger.debug(LogValues.exceptionTrace(e)); + } } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java index b243d968..cd6bc3d6 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java @@ -1,5 +1,9 @@ package com.launchdarkly.sdk.server.integrations; +import com.launchdarkly.logging.LDLogLevel; +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; import com.launchdarkly.testhelpers.TempDir; import org.junit.Test; @@ -22,6 +26,7 @@ @SuppressWarnings("javadoc") public class FileDataPollerTest { + private static final LDLogger testLogger = LDLogger.none(); private static final Duration INTERVAL = Duration.ofMillis(20); private static final long WAIT_MILLIS = 5000; private static final long QUIET_MILLIS = 250; @@ -53,7 +58,7 @@ public void detectsModifiedTimeChange() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { Thread.sleep(QUIET_MILLIS); assertEquals(0, changes.get()); @@ -68,7 +73,7 @@ public void detectsSizeChangeWithSameModifiedTime() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { rewrite(file, "{\"flagValues\":{}}", 1000_000); awaitCount(changes, 1); } @@ -80,7 +85,7 @@ public void firesOncePerChange() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { rewrite(file, "{}", 2000_000); awaitCount(changes, 1); Thread.sleep(QUIET_MILLIS); @@ -94,7 +99,7 @@ public void detectsFileAppearing() throws Exception { try (TempDir dir = TempDir.create()) { Path file = dir.getPath().resolve("later.json"); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { Thread.sleep(QUIET_MILLIS); assertEquals(0, changes.get()); @@ -109,7 +114,7 @@ public void detectsFileDisappearing() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { Files.delete(file); awaitCount(changes, 1); } @@ -123,7 +128,7 @@ public void watchesEveryConfiguredFile() throws Exception { Path second = createFile(dir, "b.json", "{}", 1000_000); List paths = Arrays.asList(first, second); AtomicInteger changes = new AtomicInteger(); - try (FileDataPoller poller = new FileDataPoller(paths, INTERVAL, changes::incrementAndGet)) { + try (FileDataPoller poller = new FileDataPoller(paths, INTERVAL, changes::incrementAndGet, testLogger)) { rewrite(second, "{}", 2000_000); awaitCount(changes, 1); rewrite(first, "{}", 2000_000); @@ -137,7 +142,7 @@ public void stopsOnClose() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); - FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet); + FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger); poller.close(); poller.close(); // idempotent @@ -160,7 +165,7 @@ public void closeReturnsWhileCallbackBlocks() throws Exception { } catch (InterruptedException e) { Thread.currentThread().interrupt(); } - }); + }, testLogger); rewrite(file, "{}", 2000_000); assertTrue(entered.await(WAIT_MILLIS, TimeUnit.MILLISECONDS)); @@ -171,6 +176,31 @@ public void closeReturnsWhileCallbackBlocks() throws Exception { } } + @Test + public void pollingContinuesAfterTheCallbackThrows() throws Exception { + LogCapture logCapture = Logs.capture(); + LDLogger logger = LDLogger.withAdapter(logCapture, ""); + try (TempDir dir = TempDir.create()) { + Path file = createFile(dir, "a.json", "{}", 1000_000); + AtomicInteger changes = new AtomicInteger(); + Runnable onChange = () -> { + if (changes.incrementAndGet() == 1) { + throw new IllegalStateException("consumer failed"); + } + }; + try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, onChange, logger)) { + // The first change makes the callback throw. + rewrite(file, "{}", 2000_000); + awaitCount(changes, 1); + + // The next change must still be detected and delivered, and the failure logged as an error. + rewrite(file, "{}", 3000_000); + awaitCount(changes, 2); + assertTrue(logCapture.getMessages().stream().anyMatch(m -> m.getLevel() == LDLogLevel.ERROR)); + } + } + } + @Test public void unreadableAttributesCountAsAbsent() throws Exception { try (TempDir dir = TempDir.create()) { From f9613962b84e719fc98a96c90c365beba3e4ee01 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:25:07 +0000 Subject: [PATCH 5/9] fix: Report a failed apply as a load failure and retry it The last good hash and the failure state are recorded only after the handler accepts the result. An exception from apply goes through the same failure path as a read or parse failure, so it is reported once and retried after the retry delay. --- .../server/integrations/FileDataReloader.java | 22 +++++-- .../integrations/FileDataReloaderTest.java | 58 ++++++++++++++----- 2 files changed, 60 insertions(+), 20 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java index 259ee185..97a73c07 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java @@ -54,16 +54,19 @@ interface Loader { */ interface Handler { /** - * Called with each successfully merged result. + * Called with each successfully merged result. An exception thrown here fails the reload: it is + * reported through {@link #onError(FileDataException)}, nothing from the load is remembered as + * the last good result, and the reload is retried. * * @param result the merged data */ void apply(LoadResult result); /** - * Called when a reload fails, once per distinct failure. With automatic retries, repeats of an - * identical failure do not call this again. A success re-arms it. The reloader logs failures - * itself, so implementations only need to update their own state. + * Called when a reload fails, including when {@link #apply(LoadResult)} throws, once per + * distinct failure. With automatic retries, repeats of an identical failure do not call this + * again. A success re-arms it. The reloader logs failures itself, so implementations only need + * to update their own state. * * @param e the failure */ @@ -291,13 +294,20 @@ private boolean reload() { // last success. The consumer heard about the failure and may have moved to an interrupted // state. Only apply tells it that things are good again. boolean recovering = lastErrorMessage != null; - lastErrorMessage = null; byte[] hash = result.getContentHash(); if (skipUnchanged && !recovering && hash != null && Arrays.equals(hash, lastGoodHash)) { return true; } + // Nothing is remembered until the consumer has accepted the result. A result that the + // consumer rejects must not become the baseline that skip-unchanged compares against, and + // must not count as a recovery. + try { + handler.apply(result); + } catch (RuntimeException e) { + return fail(new FileDataException("unable to apply flag data", e)); + } + lastErrorMessage = null; lastGoodHash = hash; - handler.apply(result); return true; } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java index 4c2c1704..017906bc 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java @@ -24,6 +24,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; @@ -85,6 +86,13 @@ ScriptedLoader thenFail(String message) { return this; } + ScriptedLoader thenThrow(RuntimeException e) { + steps.add(() -> { + throw e; + }); + return this; + } + @Override public LoadResult load() throws FileDataException { // Both latches are read before the entered signal, so a test that clears them after the @@ -125,7 +133,7 @@ private static final class UncheckedFileDataException extends RuntimeException { } } - private static final class RecordingHandler implements FileDataReloader.Handler { + private static class RecordingHandler implements FileDataReloader.Handler { final List applied = Collections.synchronizedList(new ArrayList<>()); final List errors = Collections.synchronizedList(new ArrayList<>()); @@ -483,29 +491,51 @@ public void reloadsAreSerializedBetweenCallerAndWorker() throws Exception { } @Test - public void unexpectedRuntimeExceptionFromHandlerIsLoggedAndWorkerSurvives() throws Exception { - ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).then(resultWithHash(2)); + public void applyThatThrowsIsReportedAsAFailureAndRetried() throws Exception { + LoadResult result = resultWithHash(1); + ScriptedLoader loader = new ScriptedLoader().then(result); AtomicInteger applies = new AtomicInteger(); - FileDataReloader.Handler handler = new FileDataReloader.Handler() { + RecordingHandler handler = new RecordingHandler() { @Override - public void apply(LoadResult result) { + public void apply(LoadResult r) { if (applies.incrementAndGet() == 1) { throw new IllegalStateException("consumer failed"); } - } - - @Override - public void onError(FileDataException e) { + super.apply(r); } }; - FileDataReloader r = new FileDataReloader(loader, handler, logger, Duration.ZERO, Duration.ZERO, false); - reloaders.add(r); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + // The initial load's apply throws. The reload returns normally and reports the failure. + r.reloadNow(); + assertTrue(handler.applied.isEmpty()); + assertEquals(1, handler.errors.size()); + assertThat(handler.errors.get(0).getCause(), instanceOf(IllegalStateException.class)); + assertThat(logCapture.getMessageStrings(), hasItem(startsWith("ERROR:Unable to load flags:"))); + + // The retry applies the same content without a further trigger. + awaitSize(handler.applied, 1); + assertEquals(result, handler.applied.get(0)); + + // Only now is the content remembered: a further reload of it is skipped. r.trigger(); - awaitAtLeast(applies, 1); - r.trigger(); - awaitAtLeast(applies, 2); + awaitAtLeast(loader.calls, 3); + assertEquals(1, handler.applied.size()); + } + @Test + public void unexpectedRuntimeExceptionFromLoaderIsLoggedAndWorkerSurvives() throws Exception { + ScriptedLoader loader = new ScriptedLoader().thenThrow(new IllegalStateException("loader failed")) + .then(resultWithHash(1)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, false); + + // The first reload's loader throws something other than a file data failure. + r.trigger(); + awaitAtLeast(loader.calls, 1); + // The worker must still run the next reload, and the exception must have been logged. + r.trigger(); + awaitSize(handler.applied, 1); assertThat(logCapture.getMessageStrings(), hasItem(startsWith("ERROR:Unexpected error while reloading flag data:"))); } From 548e2cfeb33f95510e75c6fe9fc9205eba204f86 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:25:07 +0000 Subject: [PATCH 6/9] docs: Describe value-only overrides as flags served by fallthrough --- .../sdk/server/integrations/OverrideFileLoader.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java index 987056d0..ece5b8b7 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java @@ -32,9 +32,9 @@ *

* This loader is separate from the file data source's loader because the override source has * different rules: a configured file that does not exist contributes no entries, entries keep the - * versions that the documents specify, a value-only entry becomes a flag that is off and serves the - * value, and every failure to read or parse a file is reported as a file data error. The file data - * source keeps its own behavior. + * versions that the documents specify, a value-only entry becomes a flag that is on and serves the + * value by fallthrough, and every failure to read or parse a file is reported as a file data error. + * The file data source keeps its own behavior. */ final class OverrideFileLoader { /** From 8edc7dbe7b0d438919255cf355726b9700446413 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:51:05 +0000 Subject: [PATCH 7/9] fix: Schedule directory registration retries on the monotonic clock The time of the next registration attempt and the wait before it come from System.nanoTime, so a step of the system clock neither delays nor advances the retry. --- .../server/integrations/FileDataWatcher.java | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java index 70ceb6e8..dbbd897c 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataWatcher.java @@ -49,16 +49,18 @@ final class FileDataWatcher implements Closeable, Runnable { private final WatchService watchService; private final Set watchedFilePaths; - private final long directoryRetryDelayMillis; + private final long directoryRetryDelayNanos; private final LDLogger logger; private final Thread thread; private final AtomicBoolean stopped = new AtomicBoolean(false); private volatile Runnable onChange; // The directories that are not registered because they do not exist, and the time of the next - // attempt to register them. Both are written during creation and then only by the worker thread. + // attempt to register them. The time comes from the monotonic clock, so a change to the system + // clock does not move the schedule. Both are written during creation and then only by the + // worker thread. private final Set missingDirectories = new HashSet<>(); - private long nextRetryAtMillis; + private long nextRetryAtNanos; /** * Creates a watcher for the given files. Nothing is watched until {@link #start(Runnable)}. @@ -112,7 +114,7 @@ private FileDataWatcher( ) { this.watchService = watchService; this.watchedFilePaths = watchedFilePaths; - this.directoryRetryDelayMillis = Math.max(directoryRetryDelay.toMillis(), 1); + this.directoryRetryDelayNanos = TimeUnit.MILLISECONDS.toNanos(Math.max(directoryRetryDelay.toMillis(), 1)); this.logger = logger; this.thread = new Thread(this, "LaunchDarkly-FileDataWatcher"); this.thread.setDaemon(true); @@ -139,8 +141,8 @@ public void run() { key = watchService.take(); // blocks until a change is available or the thread is interrupted } else { // Wake up for the next registration attempt even when no change arrives. - long waitMillis = nextRetryAtMillis - System.currentTimeMillis(); - key = waitMillis > 0 ? watchService.poll(waitMillis, TimeUnit.MILLISECONDS) : null; + long waitNanos = nextRetryAtNanos - System.nanoTime(); + key = waitNanos > 0 ? watchService.poll(waitNanos, TimeUnit.NANOSECONDS) : null; } } catch (InterruptedException e) { continue; // if stopped, the loop condition ends the thread @@ -150,7 +152,7 @@ public void run() { if (key != null) { processKey(key); } - if (!missingDirectories.isEmpty() && System.currentTimeMillis() >= nextRetryAtMillis) { + if (!missingDirectories.isEmpty() && System.nanoTime() - nextRetryAtNanos >= 0) { retryMissingDirectories(); } } @@ -204,7 +206,7 @@ private void registerOrRememberAsMissing(Path directory) { private void rememberAsMissing(Path directory) { if (missingDirectories.isEmpty()) { - nextRetryAtMillis = System.currentTimeMillis() + directoryRetryDelayMillis; + nextRetryAtNanos = System.nanoTime() + directoryRetryDelayNanos; } missingDirectories.add(directory); } @@ -226,7 +228,7 @@ private void retryMissingDirectories() { it.remove(); registered = true; } - nextRetryAtMillis = System.currentTimeMillis() + directoryRetryDelayMillis; + nextRetryAtNanos = System.nanoTime() + directoryRetryDelayNanos; if (registered && !stopped.get()) { signal(); } From 0b4c68c19e6beb18608fa81d9ffb7c4958476a8f Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 13 Oct 2026 21:02:18 +0000 Subject: [PATCH 8/9] test: Assert that the file data poller goes quiet once a file is stable A plain rewrite is not atomic: the write and the modification-time update are separate steps, and a poll that lands between them legitimately reports an extra change. The poller promises generous detection, not one callback per logical change; the reloader coalesces and skips the excess. The test that expected exactly one callback could therefore fail on a busy machine. It now waits for the write to be fully observed and then requires that the count stays put, which still fails for a poller that does not update its baseline. --- .../sdk/server/integrations/FileDataPollerTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java index cd6bc3d6..cf5c82d6 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataPollerTest.java @@ -30,6 +30,8 @@ public class FileDataPollerTest { private static final Duration INTERVAL = Duration.ofMillis(20); private static final long WAIT_MILLIS = 5000; private static final long QUIET_MILLIS = 250; + // Long enough for a rewrite that the poller observed in several steps to be fully reported. + private static final long SETTLE_MILLIS = 100; private static void awaitCount(AtomicInteger counter, int expected) throws InterruptedException { long deadline = System.currentTimeMillis() + WAIT_MILLIS; @@ -80,16 +82,21 @@ public void detectsSizeChangeWithSameModifiedTime() throws Exception { } } + // The poller updates its baseline after a change, so it goes quiet once the file is stable. It + // does not promise one callback per logical change: a rewrite is not atomic, and a poll that + // lands between its steps legitimately reports an extra change, which the reloader absorbs. @Test - public void firesOncePerChange() throws Exception { + public void stopsFiringWhenFileIsStable() throws Exception { try (TempDir dir = TempDir.create()) { Path file = createFile(dir, "a.json", "{}", 1000_000); AtomicInteger changes = new AtomicInteger(); try (FileDataPoller poller = new FileDataPoller(Collections.singletonList(file), INTERVAL, changes::incrementAndGet, testLogger)) { rewrite(file, "{}", 2000_000); awaitCount(changes, 1); + Thread.sleep(SETTLE_MILLIS); + int settled = changes.get(); Thread.sleep(QUIET_MILLIS); - assertEquals(1, changes.get()); + assertEquals(settled, changes.get()); } } } From 3b81e47c99dbafffa2630349ff01e9e18f8b147c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 3 Nov 2026 23:47:25 +0000 Subject: [PATCH 9/9] fix: Hold back an override file that is emptied during an in-place rewrite Applications and editors rewrite a file in place: they truncate it and then write the new content. A reload in between found the override file empty, chose YAML for it because it did not start with '{', and applied a document with no entries, so every override was briefly removed. The override loader now chooses the parser by the file's extension (.yaml and .yml are YAML, any other file is JSON), detects the text encoding from a byte order mark and reads UTF-8 otherwise, and rejects an empty JSON file and a JSON document that is not an object. A file whose content is in effect and that a load finds empty, white space, or YAML with no document is held back until it has stayed so, without being written, for the retry delay; the hold's retry is armed for the time left, and each file's hold is bounded at five retry delays from its first hold, starting over at a load that holds nothing back. A load reads every file, also after a failure, fails with the first failure, and merges the entries only after every file is read. The skip-unchanged digest covers each file's path and presence. The file data source keeps choosing the parser by content. --- .../server/integrations/FileDataPoller.java | 9 +- .../server/integrations/FileDataReloader.java | 160 ++- .../integrations/FileDataSourceParsing.java | 91 +- .../integrations/OverrideFileLoader.java | 442 +++++- .../FileDataReloaderEmptiedFileTest.java | 680 +++++++++ .../integrations/FileDataReloaderTest.java | 199 ++- .../server/integrations/InPlaceWrites.java | 49 + .../integrations/OverrideFileLoaderTest.java | 1256 ++++++++++++++++- 8 files changed, 2751 insertions(+), 135 deletions(-) create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderEmptiedFileTest.java create mode 100644 lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/InPlaceWrites.java diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java index 954e22e6..65100cb8 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataPoller.java @@ -104,7 +104,14 @@ static List observeAll(List paths) { return states; } - private static FileState observe(Path path) { + /** + * Observes one file. The attributes are read through any symbolic link, so they describe the file + * that the link points to. + * + * @param path the file + * @return its state, or {@link FileState#ABSENT} if its attributes cannot be read + */ + static FileState observe(Path path) { try { BasicFileAttributes attributes = Files.readAttributes(path, BasicFileAttributes.class); return new FileState(true, attributes.lastModifiedTime().toMillis(), attributes.size()); diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java index 97a73c07..e844d1ca 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataReloader.java @@ -6,8 +6,12 @@ import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; import java.io.Closeable; +import java.nio.file.Path; import java.time.Duration; +import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.List; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.ThreadFactory; @@ -23,6 +27,10 @@ * and returns the merged result, or throws {@link FileDataException} when a file cannot be read or * parsed or when the files cannot be merged. *

+ * A file whose content is in effect and is found empty may be in the middle of an in-place rewrite. + * The loader can hold such a file back with {@link EmptiedFileException}: the reloader keeps the + * current data, reports no failure, and reads the files again when the exception says. + *

* The file data source does not use this class. It keeps its own reload behavior. */ final class FileDataReloader implements Closeable { @@ -43,7 +51,55 @@ final class FileDataReloader implements Closeable { * Performs one full load of all configured files. */ interface Loader { - LoadResult load() throws FileDataException; + /** + * Reads every configured file and merges the entries. + * + * @param retryDelay the reloader's retry delay, or zero if it does not retry. A loader may hold + * an empty file back for a time based on it, because a retry reads the files again. With zero, + * nothing would read them again, so it must not hold any back. + * @return the merged result + * @throws FileDataException if a file cannot be read or parsed, or the files cannot be merged + * @throws EmptiedFileException if a file was found empty and is held back + */ + LoadResult load(Duration retryDelay) throws FileDataException, EmptiedFileException; + + /** + * Called with the result of a load that is now the data in effect: it was applied, or it was + * skipped because it is unchanged. It is called with the reload lock held. + * + * @param result the result + */ + void accepted(LoadResult result); + } + + /** + * Thrown by a {@link Loader} that holds back files it found empty. A file may be in the middle of an + * in-place rewrite: a writer that truncates the file and then writes the new content. + */ + @SuppressWarnings("serial") + static final class EmptiedFileException extends Exception { + private final List paths; + private final long retryAfterNanos; + + EmptiedFileException(List paths, long retryAfterNanos) { + super("files " + paths + " are empty"); + this.paths = Collections.unmodifiableList(new ArrayList<>(paths)); + this.retryAfterNanos = retryAfterNanos; + } + + /** + * The files held back, in configuration order. + */ + List getPaths() { + return paths; + } + + /** + * How long to wait before loading again, in nanoseconds. + */ + long getRetryAfterNanos() { + return retryAfterNanos; + } } /** @@ -66,7 +122,8 @@ interface Handler { * Called when a reload fails, including when {@link #apply(LoadResult)} throws, once per * distinct failure. With automatic retries, repeats of an identical failure do not call this * again. A success re-arms it. The reloader logs failures itself, so implementations only need - * to update their own state. + * to update their own state. A load that defers an empty file and reads it again later is not a + * failure and does not call this. * * @param e the failure */ @@ -78,18 +135,26 @@ interface Handler { private final LDLogger logger; private final long debounceDelayMillis; private final long retryDelayMillis; + // What reload() returns when no retry is needed. + private static final long NO_RETRY = -1; private final boolean skipUnchanged; // Serializes the load work between reloadNow and the worker thread. private final Object reloadLock = new Object(); private byte[] lastGoodHash = null; private String lastErrorMessage = null; + // True from a load that found an emptied file until the next load starts. Written inside the + // reload lock. It only chooses the log message of the retry that such a load arms. + private volatile boolean readAgainPending = false; // Guards the worker and the two timers. private final Object timerLock = new Object(); private ScheduledThreadPoolExecutor worker = null; private ScheduledFuture debounceFuture = null; private ScheduledFuture retryFuture = null; + // The delay of the latest request to arm a retry, in nanoseconds, or -1 before the first. Visible for + // tests through getLastRequestedRetryNanos. + private volatile long lastRequestedRetryNanos = -1; private final AtomicBoolean closed = new AtomicBoolean(false); @@ -97,16 +162,17 @@ interface Handler { * Creates a reloader. The caller performs the initial load with {@link #reloadNow()}, routes * change signals to {@link #trigger()}, and calls {@link #close()} when finished. *

- * The worker thread starts on the first {@link #trigger()} call or the first failed load that - * arms a retry, not here. Construction alone does not start a thread. + * The worker thread starts on the first {@link #trigger()} call or the first load that arms a + * retry, not here. Construction alone does not start a thread. * * @param loader performs each full load * @param handler receives results and failures * @param logger receives log output about reloads and failures * @param debounceDelay how long to wait after a trigger for further triggers to settle before * reloading. Zero or negative reloads on every trigger without waiting. - * @param retryDelay how long to wait after a failed reload before retrying without a trigger. - * Zero or negative disables the automatic retry. + * @param retryDelay how long to wait after a failed reload before retrying without a trigger. A + * load that holds back an empty file says itself when to load again. Zero or negative disables + * the automatic retry, and the loader then parses an empty file as it is. * @param skipUnchanged if true, a successful load whose raw file contents are identical to the * last applied contents does not call the apply callback */ @@ -131,8 +197,9 @@ interface Handler { * initial load. A failure here arms the same automatic retry as a failed triggered reload. */ void reloadNow() { - if (!reload() && retryDelayMillis > 0) { - armRetry(); + long retryAfterNanos = reload(); + if (retryAfterNanos != NO_RETRY) { + armRetry(retryAfterNanos); } } @@ -181,6 +248,16 @@ public void close() { } } + /** + * Returns the delay of the latest request to arm a retry, whether or not it armed one. Visible for + * tests. + * + * @return the delay in nanoseconds, or -1 if no retry was requested + */ + long getLastRequestedRetryNanos() { + return lastRequestedRetryNanos; + } + /** * Reports whether the worker thread has been started. Visible for tests. * @@ -228,7 +305,8 @@ private ScheduledThreadPoolExecutor ensureWorker() { return worker; } - private void armRetry() { + private void armRetry(long delayNanos) { + lastRequestedRetryNanos = delayNanos; synchronized (timerLock) { if (closed.get()) { return; @@ -237,7 +315,7 @@ private void armRetry() { if (retryFuture != null && !retryFuture.isDone()) { return; } - retryFuture = ensureWorker().schedule(() -> runReload(true), retryDelayMillis, TimeUnit.MILLISECONDS); + retryFuture = ensureWorker().schedule(() -> runReload(true), delayNanos, TimeUnit.NANOSECONDS); } } @@ -245,7 +323,8 @@ private void armRetry() { private void runReload(boolean isRetry) { try { if (isRetry) { - logger.debug("Retrying flag data load after earlier failure"); + logger.debug(readAgainPending ? "Reading flag data again after a file was found empty" + : "Retrying flag data load after earlier failure"); } else { logger.info("Reloading flag data after detecting a change"); } @@ -257,8 +336,9 @@ private void runReload(boolean isRetry) { retryFuture = null; } } - if (!reload() && retryDelayMillis > 0) { - armRetry(); + long retryAfterNanos = reload(); + if (retryAfterNanos != NO_RETRY) { + armRetry(retryAfterNanos); } } catch (RuntimeException e) { // The executor would otherwise swallow the exception and the worker would go quiet. @@ -267,27 +347,31 @@ private void runReload(boolean isRetry) { } } - // Performs one full load of all configured files. It reports whether the load succeeded, which - // decides whether a retry gets armed. A skipped no-op application counts as success. The whole - // set is re-read on every reload because entries are combined across files in order, so a - // change to one file can alter which file wins for a key. - private boolean reload() { + // Performs one full load of all configured files. It returns when to load again: NO_RETRY after a + // success, including a skipped no-op application, the retry delay after a failure, or the time that + // a load that holds back an empty file asks for. The whole set is re-read on every reload because + // entries are combined across files in order, so a change to one file can alter which file wins + // for a key. + private long reload() { synchronized (reloadLock) { // A trigger already queued when close was called can still reach here. if (closed.get()) { - return true; + return NO_RETRY; } + readAgainPending = false; LoadResult result; try { - result = loader.load(); + result = loader.load(retryDelayMillis > 0 ? Duration.ofMillis(retryDelayMillis) : Duration.ZERO); + } catch (EmptiedFileException e) { + return readAgainLater(e); } catch (FileDataException e) { return fail(e); } // Close may have happened while the files were being read. Deliver nothing in that case. if (closed.get()) { - return true; + return NO_RETRY; } // A success right after a failure must apply even when the content is unchanged since the @@ -296,7 +380,8 @@ private boolean reload() { boolean recovering = lastErrorMessage != null; byte[] hash = result.getContentHash(); if (skipUnchanged && !recovering && hash != null && Arrays.equals(hash, lastGoodHash)) { - return true; + loader.accepted(result); + return NO_RETRY; } // Nothing is remembered until the consumer has accepted the result. A result that the // consumer rejects must not become the baseline that skip-unchanged compares against, and @@ -308,27 +393,44 @@ private boolean reload() { } lastErrorMessage = null; lastGoodHash = hash; - return true; + loader.accepted(result); + return NO_RETRY; + } + } + + // Called inside the reload lock. The loader held back files that it found empty: a file whose + // content is in effect may be in the middle of an in-place rewrite, a writer that truncates the file + // and then writes the new content. Whatever the files' format, the load keeps the current data and + // arms the retry without reporting a failure, for the time that the loader asks for. The change + // signal for the write starts a load sooner. The loader decides how long a file can be held back, and + // holds nothing back when retries are off, because nothing would read the file again. A closed + // reloader arms no retry. + private long readAgainLater(EmptiedFileException e) { + for (Path path : e.getPaths()) { + logger.debug("File {} is empty, possibly while it is being rewritten; reading it again before applying it", path); } + readAgainPending = true; + return e.getRetryAfterNanos(); } - private boolean fail(FileDataException e) { - // Close may have happened while the files were being read. Deliver nothing and report success - // so that no retry is armed. + // Reports a failure and returns when to retry. + private long fail(FileDataException e) { + // Close may have happened while the files were being read. Deliver nothing and arm no retry. if (closed.get()) { - return true; + return NO_RETRY; } + long retryAfterNanos = retryDelayMillis > 0 ? TimeUnit.MILLISECONDS.toNanos(retryDelayMillis) : NO_RETRY; // With automatic retries, a persistent failure would repeat the same log entry and the same // callback on every attempt. Repeats of an identical failure are logged at debug level and do // not call onError again. A consumer therefore sees one report per distinct failure. String description = describe(e); if (description.equals(lastErrorMessage)) { logger.debug("Unable to load flags: {}", description); - return false; + return retryAfterNanos; } lastErrorMessage = description; logger.error("Unable to load flags: {}", description); handler.onError(e); - return false; + return retryAfterNanos; } } diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java index 56fb4f22..179661f5 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/FileDataSourceParsing.java @@ -2,7 +2,7 @@ import com.google.gson.Gson; import com.google.gson.JsonElement; -import com.google.gson.JsonObject; +import com.google.gson.JsonIOException; import com.google.gson.JsonSyntaxException; import com.launchdarkly.sdk.LDValue; import com.launchdarkly.sdk.ObjectBuilder; @@ -12,6 +12,7 @@ import org.yaml.snakeyaml.Yaml; import org.yaml.snakeyaml.constructor.SafeConstructor; import org.yaml.snakeyaml.error.YAMLException; +import org.yaml.snakeyaml.reader.UnicodeReader; import org.yaml.snakeyaml.representer.Representer; import java.io.ByteArrayInputStream; @@ -19,6 +20,8 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.io.Reader; +import java.nio.file.Path; +import java.util.Locale; import java.util.Map; import static com.launchdarkly.sdk.server.DataModel.FEATURES; @@ -64,6 +67,13 @@ public String getDescription() { * parse the flags or segments at this level; that will be done by {@link FlagFactory}. */ static final class FlagFileRep { + /** + * What a parser returns for content that holds no document, such as YAML with only a comment or a + * document marker. It has no entries. The override source compares a result with it by identity, + * so it must not be modified. + */ + static final FlagFileRep NO_DOCUMENT = new FlagFileRep(); + Map flags; Map flagValues; Map segments; @@ -91,7 +101,43 @@ public static FlagFileParser selectForContent(byte[] data) { Reader r = new InputStreamReader(new ByteArrayInputStream(data)); return detectJson(r) ? JsonParserHolder.getInstance() : YamlParserHolder.getInstance(); } - + + /** + * Chooses the parser by the extension of the file name: a file whose name ends in {@code .yaml} + * or {@code .yml}, in any case, is parsed as YAML, and any other file is parsed as JSON. The + * override source uses this. The file data source keeps choosing by content, with + * {@link #selectForContent(byte[])}. + * + * @param path the file + * @return the parser for the file + */ + static FlagFileParser selectForPath(Path path) { + Path fileName = path.getFileName(); + String name = fileName == null ? "" : fileName.toString().toLowerCase(Locale.ROOT); + return name.endsWith(".yaml") || name.endsWith(".yml") + ? YamlParserHolder.getInstance() : JsonParserHolder.getInstance(); + } + + /** + * Parses the content of a file with the parser that {@link #selectForPath(Path)} chooses. The + * text encoding is detected from a byte order mark, UTF-8, UTF-16 or UTF-32, and is UTF-8 + * otherwise, for both formats; the YAML parser does that itself. The override source uses this. + * The file data source keeps reading JSON in the platform's default encoding. + * + * @param path the file + * @param data its content + * @return the parsed content + * @throws FileDataException if the content cannot be decoded or parsed + * @throws IOException if the content cannot be read + */ + static FlagFileRep parseForPath(Path path, byte[] data) throws FileDataException, IOException { + FlagFileParser parser = selectForPath(path); + if (parser instanceof JsonFlagFileParser) { + return ((JsonFlagFileParser) parser).parse(new UnicodeReader(new ByteArrayInputStream(data))); + } + return parser.parse(new ByteArrayInputStream(data)); + } + private static boolean detectJson(Reader r) { // A valid JSON file for our purposes must be an object, i.e. it must start with '{' while (true) { @@ -119,11 +165,40 @@ static final class JsonFlagFileParser extends FlagFileParser { @Override public FlagFileRep parse(InputStream input) throws FileDataException, IOException { + return parse(new InputStreamReader(input)); + } + + /** + * Parses JSON from text that is already decoded. + * + * @param reader the text + * @return the parsed content + * @throws FileDataException if the text is not a JSON object, or cannot be decoded + * @throws IOException if the text cannot be read + */ + FlagFileRep parse(Reader reader) throws FileDataException, IOException { + JsonElement tree; try { - return parseJson(gson.fromJson(new InputStreamReader(input), JsonElement.class)); + tree = gson.fromJson(reader, JsonElement.class); } catch (JsonSyntaxException e) { throw new FileDataException("cannot parse JSON", e); - } + } catch (JsonIOException e) { + // For example, bytes that are not valid in the detected encoding. + throw new FileDataException("cannot read JSON", e); + } + if (tree == null) { + // Gson finds no value in a file that is empty or white space only, or, since it reads + // leniently, one that holds only comments. That is not a JSON document. A file that is being + // rewritten in place can be empty for a moment, and the failure keeps its last good data + // until it is complete. The file data source never gets here: it parses a file as JSON only + // when the file starts with '{'. + throw new FileDataException("the file has no content, which is not a JSON document", null); + } + if (!tree.isJsonObject()) { + // A document such as null or [] holds no flags. The file data source never gets here either. + throw new FileDataException("the JSON document is not an object", null); + } + return parseJson(tree); } public FlagFileRep parseJson(JsonElement tree) throws FileDataException, IOException { @@ -174,8 +249,12 @@ public FlagFileRep parse(InputStream input) throws FileDataException, IOExceptio } catch (YAMLException e) { throw new FileDataException("unable to parse YAML", e); } - JsonElement jsonRoot = root == null ? new JsonObject() : gson.toJsonTree(root); - return jsonFileParser.parseJson(jsonRoot); + if (root == null) { + // Empty content, or content with only comments or document markers, holds no document. It has + // no entries. + return FlagFileRep.NO_DOCUMENT; + } + return jsonFileParser.parseJson(gson.toJsonTree(root)); } } diff --git a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java index ece5b8b7..f58488c5 100644 --- a/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java +++ b/lib/sdk/server/src/main/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoader.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.integrations.FileDataReloader.EmptiedFileException; import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FlagFactory; import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FlagFileParser; @@ -10,19 +11,25 @@ import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; +import org.yaml.snakeyaml.reader.UnicodeReader; + import java.io.ByteArrayInputStream; import java.io.IOException; +import java.io.Reader; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; +import java.time.Duration; import java.util.AbstractMap; import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.LongSupplier; import static com.launchdarkly.sdk.server.DataModel.FEATURES; import static com.launchdarkly.sdk.server.DataModel.SEGMENTS; @@ -33,22 +40,55 @@ * This loader is separate from the file data source's loader because the override source has * different rules: a configured file that does not exist contributes no entries, entries keep the * versions that the documents specify, a value-only entry becomes a flag that is on and serves the - * value by fallthrough, and every failure to read or parse a file is reported as a file data error. - * The file data source keeps its own behavior. + * value by fallthrough, the format of a file is chosen by its extension, and every failure to read or + * parse a file is reported as a file data error. The file data source keeps its own behavior. + *

+ * Whether a file with no content is valid depends on its format, like the rest of its content. A JSON + * file with no content is not a JSON document, so it fails to parse. A YAML file with no content is a + * document with no entries. + *

+ * A file that is rewritten in place is empty for a moment, and a load can find it so. A file counts + * as empty when it holds only white space, or when its parser reads its content as no document, as the + * YAML parser does for a part of a file that holds only a comment or a document marker. The loader + * remembers, for each file, whether it contributed content to the data in effect, and since when it + * has been found empty without being written. A file whose content is in effect and that is found + * empty is held back, not parsed: {@link #load(Duration)} still reads the other files, so that files + * emptied together are held back from the same load, and then throws {@link EmptiedFileException}. + * The data in effect stays, and the reloader reads the files again when the exception says. A file is + * held back until it has stayed empty, without being written, for one retry delay, and for at most + * five retry delays from the first load that held it back, so that a file that keeps being rewritten + * cannot hold the changes back for longer. The bound is per file, and it starts over at a load that + * holds nothing back. A file that never contributed content, such as a placeholder, and a file whose empty state + * is in effect are parsed right away. The reloader calls {@link #load(Duration)} and + * {@link #accepted(LoadResult)} one call at a time, so this state needs no synchronization. */ -final class OverrideFileLoader { +final class OverrideFileLoader implements FileDataReloader.Loader { + /** + * How many retry delays a file can be held back for, from the first load that held it back. + */ + static final int MAX_HOLD_RETRY_DELAYS = 5; + + /** + * Reads a file. Tests substitute one that takes time. + */ + interface FileReader { + byte[] read(Path path) throws IOException; + } + /** * Describes one configured file after a load. */ static final class FileSummary { private final Path path; private final boolean present; + private final boolean hasContent; private final int flags; private final int segments; - FileSummary(Path path, boolean present, int flags, int segments) { + FileSummary(Path path, boolean present, boolean hasContent, int flags, int segments) { this.path = path; this.present = present; + this.hasContent = hasContent; this.flags = flags; this.segments = segments; } @@ -64,6 +104,14 @@ boolean isPresent() { return present; } + /** + * False when the file does not exist, is empty, holds only white space or a byte order mark, or + * holds content that its parser reads as no document, such as YAML with only a comment. + */ + boolean hasContent() { + return hasContent; + } + /** * The number of flag entries the merge kept from this file. An entry dropped by the duplicate keys * handling is not counted. @@ -132,8 +180,32 @@ byte[] getContentHash() { } } + /** + * A file whose content is in effect and that loads have found empty. + * Times are from the monotonic clock. + */ + private static final class EmptyFile { + // Since when the file has been empty without being written, or null after a load read it with + // content, even content that fails to parse, or found it missing. + Long sinceNanos; + // The state of the file then: its modification time and size, read through any symbolic link. It + // is only compared for equality. + FileDataPoller.FileState state; + // When a load first held the file back, or null if no load has since the last load that got to the + // file and held nothing back. Loads that hold another file back keep it, so that a file that keeps + // being rewritten stays bounded. + Long firstHeldNanos; + } + private final List paths; private final FileData.DuplicateKeysHandling duplicateKeysHandling; + private final LongSupplier nanoClock; + // Whether each configured file contributed content to the data in effect. Updated only by + // accepted(LoadResult). A file that is not listed did not. + private final Map appliedHadContent = new HashMap<>(); + private final FileReader reader; + // The files whose content is in effect and that loads have found empty. + private final Map emptyFiles = new HashMap<>(); /** * Creates a loader. @@ -142,84 +214,334 @@ byte[] getContentHash() { * @param duplicateKeysHandling what to do when the same key appears in more than one file */ OverrideFileLoader(List paths, FileData.DuplicateKeysHandling duplicateKeysHandling) { + this(paths, duplicateKeysHandling, System::nanoTime); + } + + /** + * Creates a loader with a specific monotonic clock. Visible for tests. + */ + OverrideFileLoader(List paths, FileData.DuplicateKeysHandling duplicateKeysHandling, + LongSupplier nanoClock) { + this(paths, duplicateKeysHandling, nanoClock, Files::readAllBytes); + } + + /** + * Creates a loader with a specific monotonic clock and file reader. Visible for tests. + */ + OverrideFileLoader(List paths, FileData.DuplicateKeysHandling duplicateKeysHandling, + LongSupplier nanoClock, FileReader reader) { this.paths = new ArrayList<>(paths); this.duplicateKeysHandling = duplicateKeysHandling; + this.nanoClock = nanoClock; + this.reader = reader; } /** * Reads every configured file in order and merges the entries. * + * @param retryDelay the reloader's retry delay, or zero if it does not retry. A file whose content is + * in effect and that is found empty is held back until it has stayed empty, without being written, + * for this long, and for at most {@link #MAX_HOLD_RETRY_DELAYS} times this from the first load + * that held it back. With zero, every file is parsed as it is. * @return the merged result with a summary of each file - * @throws FileDataException if a file that exists cannot be read or parsed, or the files cannot be merged + * @throws FileDataException if a file that exists cannot be read or parsed, or the files cannot be + * merged, also when another file is held back + * @throws EmptiedFileException if a file is held back and every other file loads */ - LoadResult load() throws FileDataException { + @Override + public LoadResult load(Duration retryDelay) throws FileDataException, EmptiedFileException { MessageDigest digest = newDigest(); - Map> data = new HashMap<>(); - List files = new ArrayList<>(paths.size()); - for (Path path : paths) { - byte[] raw; - try { - raw = Files.readAllBytes(path); - } catch (NoSuchFileException e) { - // Absence is a valid state, not a failure. - files.add(new FileSummary(path, false, 0, 0)); - continue; - } catch (IOException e) { - throw new FileDataException("file " + path + ": unable to read file", e); + List heldBack = new ArrayList<>(); + // The files that this load got to and did not hold back: read with content, parsed empty, found + // missing, or failing to read or parse. A load reads every configured file, also after a failure, + // so every file that is not held back is here. + List reached = new ArrayList<>(); + // The parsed documents of the files that are not held back, in configuration order. + List documents = new ArrayList<>(paths.size()); + // The first failure to read or parse a file. The other files are still read, so that each file + // is classified, and the load fails with this one after them. + FileDataException failure = null; + try { + for (Path path : paths) { + byte[] raw; + try { + raw = reader.read(path); + } catch (NoSuchFileException e) { + // Absence is a valid state, not a failure. + noteNotEmpty(path, reached); + updateDigest(digest, path, null); + documents.add(new ReadFile(path, null, false)); + continue; + } catch (IOException e) { + reached.add(path); + failure = first(failure, new FileDataException("file " + path + ": unable to read file", e)); + continue; + } + // Content that the parser reads as no document, such as YAML with only a comment, has no + // entries, and a read can see that much of a file that is being written. Content with only + // white space is invalid JSON, so it is parsed only when the file is not held back. + FlagFileRep document = null; + boolean hasContent = hasContent(raw); + if (hasContent) { + try { + document = parse(path, raw); + } catch (FileDataException e) { + noteNotEmpty(path, reached); + failure = first(failure, e); + continue; + } + hasContent = document != FlagFileRep.NO_DOCUMENT; + } + if (hasContent) { + noteNotEmpty(path, reached); + } else if (shouldHoldBack(path, retryDelay, nanoClock.getAsLong())) { + // The clock is read after this file's own read, so a slow read of another file does not + // shorten its delay. The other files are still read, so that each emptied file starts its + // delay in this load, and a failure of another file still fails the load. + heldBack.add(path); + continue; + } else { + reached.add(path); + } + // One read feeds both the digest and the parse, so the skip-unchanged digest can never disagree + // with the content that was applied. + updateDigest(digest, path, raw); + if (document == null) { + try { + document = parse(path, raw); + } catch (FileDataException e) { + failure = first(failure, e); + continue; + } + } + documents.add(new ReadFile(path, document, hasContent)); + } + + // A failure fails the load before a hold, and a hold comes before the merge. + if (failure != null) { + throw failure; + } + if (!heldBack.isEmpty()) { + throw new EmptiedFileException(heldBack, holdBack(heldBack, retryDelay, nanoClock.getAsLong())); + } + + // The entries are merged only after every file is read, so that a duplicate key or an entry + // that the data model rejects cannot keep a later file from being read. + Map> data = new HashMap<>(); + List files = new ArrayList<>(documents.size()); + for (ReadFile file : documents) { + if (file.document == null) { + files.add(new FileSummary(file.path, false, false, 0, 0)); + continue; + } + int[] added = addEntries(data, file.path, file.document); + files.add(new FileSummary(file.path, true, file.hasContent, added[0], added[1])); } - // One read feeds both the digest and the parse, so the skip-unchanged digest can never disagree - // with the content that was applied. - if (digest != null) { - digest.update(raw); - digest.update((byte) 0); + ImmutableList.Builder>> all = ImmutableList.builder(); + for (Map.Entry> e : data.entrySet()) { + all.add(new AbstractMap.SimpleEntry<>(e.getKey(), new KeyedItems<>(ImmutableList.copyOf(e.getValue().entrySet())))); } - int flagsAdded = 0; - int segmentsAdded = 0; - try { - FlagFileParser parser = FlagFileParser.selectForContent(raw); - FlagFileRep fileContents = parser.parse(new ByteArrayInputStream(raw)); - if (fileContents.flags != null) { - for (Map.Entry e : fileContents.flags.entrySet()) { - if (add(data, FEATURES, e.getKey(), FlagFactory.flagFromJson(e.getValue()))) { - flagsAdded++; - } + Map flags = data.get(FEATURES); + Map segments = data.get(SEGMENTS); + return new LoadResult(all.build(), files, flags == null ? 0 : flags.size(), + segments == null ? 0 : segments.size(), digest == null ? null : digest.digest()); + } finally { + if (heldBack.isEmpty()) { + // A load that holds nothing back, whether it is applied, skipped as unchanged, or fails, + // starts the bound over for every file that it got to, including one that failed: such a + // file is no longer held, so its next rewrite is a new one. + for (Path path : reached) { + EmptyFile empty = emptyFiles.get(path); + if (empty != null) { + empty.firstHeldNanos = null; } } - if (fileContents.flagValues != null) { - for (Map.Entry e : fileContents.flagValues.entrySet()) { - if (add(data, FEATURES, e.getKey(), FlagFactory.flagWithValue(e.getKey(), e.getValue(), 0))) { - flagsAdded++; - } + } + } + } + + /** + * A file that a load read and did not hold back. + */ + private static final class ReadFile { + final Path path; + // The parsed content, or null if the file does not exist. + final FlagFileRep document; + final boolean hasContent; + + ReadFile(Path path, FlagFileRep document, boolean hasContent) { + this.path = path; + this.document = document; + this.hasContent = hasContent; + } + } + + private static FileDataException first(FileDataException earlier, FileDataException e) { + return earlier != null ? earlier : e; + } + + // Adds a file to the skip-unchanged digest: its path, whether it exists, and its content. The format + // depends on the path, so the same bytes at another path can be different data. + private static void updateDigest(MessageDigest digest, Path path, byte[] raw) { + if (digest == null) { + return; + } + digest.update(path.toString().getBytes(StandardCharsets.UTF_8)); + digest.update((byte) 0); + digest.update((byte) (raw == null ? 0 : 1)); + if (raw != null) { + digest.update(raw); + digest.update((byte) 0); + } + } + + // For a file that a load read with content, including content that fails to parse, or found + // missing: if it was empty before, its empty state has ended, so its next emptying starts a new delay + // even where a coarse modification time makes it look the same. + private void noteNotEmpty(Path path, List reached) { + reached.add(path); + EmptyFile empty = emptyFiles.get(path); + if (empty != null) { + empty.sinceNanos = null; + } + } + + private static FlagFileRep parse(Path path, byte[] raw) throws FileDataException { + try { + return FlagFileParser.parseForPath(path, raw); + } catch (FileDataException e) { + throw new FileDataException("file " + path + ": " + e.getMessage(), e.getCause()); + } catch (IOException e) { + throw new FileDataException("file " + path + ": cannot read document", e); + } catch (RuntimeException e) { + throw new FileDataException("file " + path + ": cannot parse flag or segment data", e); + } + } + + // Adds the entries of one file and returns how many flags and segments the merge kept from it. + private int[] addEntries(Map> data, Path path, FlagFileRep document) + throws FileDataException { + int flagsAdded = 0; + int segmentsAdded = 0; + try { + if (document.flags != null) { + for (Map.Entry e : document.flags.entrySet()) { + if (add(data, FEATURES, e.getKey(), FlagFactory.flagFromJson(e.getValue()))) { + flagsAdded++; + } + } + } + if (document.flagValues != null) { + for (Map.Entry e : document.flagValues.entrySet()) { + if (add(data, FEATURES, e.getKey(), FlagFactory.flagWithValue(e.getKey(), e.getValue(), 0))) { + flagsAdded++; } } - if (fileContents.segments != null) { - for (Map.Entry e : fileContents.segments.entrySet()) { - if (add(data, SEGMENTS, e.getKey(), FlagFactory.segmentFromJson(e.getValue()))) { - segmentsAdded++; - } + } + if (document.segments != null) { + for (Map.Entry e : document.segments.entrySet()) { + if (add(data, SEGMENTS, e.getKey(), FlagFactory.segmentFromJson(e.getValue()))) { + segmentsAdded++; } } - } catch (FileDataException e) { - throw new FileDataException("file " + path + ": " + e.getMessage(), e.getCause()); - } catch (IOException e) { - throw new FileDataException("file " + path + ": cannot read document", e); - } catch (RuntimeException e) { - // A document can be valid JSON or YAML and still hold a flag or segment that the data model - // cannot accept. For the override source that is a parse failure of the file, so the last good - // overrides stay in effect. - throw new FileDataException("file " + path + ": cannot parse flag or segment data", e); } - files.add(new FileSummary(path, true, flagsAdded, segmentsAdded)); + } catch (FileDataException e) { + throw new FileDataException("file " + path + ": " + e.getMessage(), e.getCause()); + } catch (RuntimeException e) { + // A document can be valid JSON or YAML and still hold a flag or segment that the data model + // cannot accept. For the override source that is a parse failure of the file, so the last good + // overrides stay in effect. + throw new FileDataException("file " + path + ": cannot parse flag or segment data", e); + } + return new int[] { flagsAdded, segmentsAdded }; + } + + /** + * Records that the result of a load is now the data in effect: it was applied, or it was skipped + * because it is unchanged. Whether each file contributed content to the data in effect decides + * whether finding it empty later defers it. + * + * @param result the result of a load by this loader + */ + @Override + public void accepted(LoadResult result) { + for (FileSummary file : result.getFiles()) { + appliedHadContent.put(file.getPath(), file.hasContent()); + } + } + + // Decides whether a file that a load found empty is held back. A file whose content is in effect + // and that is empty now may be in the middle of an in-place rewrite: a writer that truncates the file + // and then writes the new content. Such a file is held back until it has stayed empty, without being + // written, for the retry delay. A change of its state means that it was written since a load last + // found it empty, for example rewritten and truncated again between two loads, so the delay starts + // over. A file that keeps being written while loads find it empty would be held back without end, so + // each file is held back for at most MAX_HOLD_RETRY_DELAYS retry delays from the first load that held + // it back. The bound is the file's own, so one file reaching it does not cut short another file's + // rewrite. It starts over only at a load that holds nothing back, for every file that load got to, + // including one that fails to read or parse, whatever the load's outcome. Loads that hold another file back keep it, which bounds files + // rewritten in turn or emptied in staggered turns, and a file that was read during a long failure + // of another file has a fresh bound when that failure ends. A file whose content is not in effect, + // such as a placeholder that has always been empty, or a file whose empty state was applied, is + // parsed right away. A zero retry delay, which means that the reloader does not retry, holds nothing + // back: nothing would read the file again. Elapsed times are differences of monotonic clock + // readings, which do not overflow. + private boolean shouldHoldBack(Path path, Duration retryDelay, long now) { + if (!Boolean.TRUE.equals(appliedHadContent.get(path))) { + return false; } + FileDataPoller.FileState state = FileDataPoller.observe(path); + EmptyFile empty = emptyFiles.computeIfAbsent(path, p -> new EmptyFile()); + if (empty.sinceNanos == null || !state.equals(empty.state)) { + empty.sinceNanos = now; + empty.state = state; + } + long delay = retryDelay.toNanos(); + if (now - empty.sinceNanos >= delay) { + // The file has stayed empty for the whole delay, so it is parsed as it is. Its bound starts over + // only at a load that holds nothing back, so that files that stay empty in staggered turns, each + // keeping the loads holding while the other settles, cannot hold the changes back without end. + return false; + } + if (empty.firstHeldNanos != null && now - empty.firstHeldNanos >= MAX_HOLD_RETRY_DELAYS * delay) { + return false; + } + if (empty.firstHeldNanos == null) { + empty.firstHeldNanos = now; + } + return true; + } - ImmutableList.Builder>> all = ImmutableList.builder(); - for (Map.Entry> e : data.entrySet()) { - all.add(new AbstractMap.SimpleEntry<>(e.getKey(), new KeyedItems<>(ImmutableList.copyOf(e.getValue().entrySet())))); + // Returns how long the reloader waits before it loads again: until the first of the held files has + // stayed empty for the retry delay or reaches its bound, plus a tenth of the retry delay, so that a + // timer that fires a little early still finds the time over. A change signal for a write starts a + // load sooner. + private long holdBack(List heldBack, Duration retryDelay, long now) { + long delay = retryDelay.toNanos(); + long remaining = Long.MAX_VALUE; + for (Path path : heldBack) { + EmptyFile empty = emptyFiles.get(path); + long delayLeft = delay - (now - empty.sinceNanos); + long boundLeft = MAX_HOLD_RETRY_DELAYS * delay - (now - empty.firstHeldNanos); + remaining = Math.min(remaining, Math.min(delayLeft, boundLeft)); + } + return Math.max(remaining, 0) + delay / 10; + } + + // A file with only white space, after a byte order mark if it has one, has no content. The text is + // decoded as the parsers decode it. Content that cannot be decoded is left to the parser to report. + private static boolean hasContent(byte[] raw) { + try (Reader text = new UnicodeReader(new ByteArrayInputStream(raw))) { + int c; + while ((c = text.read()) >= 0) { + if (!Character.isWhitespace(c)) { + return true; + } + } + return false; + } catch (IOException e) { + return true; } - Map flags = data.get(FEATURES); - Map segments = data.get(SEGMENTS); - return new LoadResult(all.build(), files, flags == null ? 0 : flags.size(), - segments == null ? 0 : segments.size(), digest == null ? null : digest.digest()); } // Adds an entry unless the key was already added. Reports whether it added the entry. diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderEmptiedFileTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderEmptiedFileTest.java new file mode 100644 index 00000000..561d8b40 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderEmptiedFileTest.java @@ -0,0 +1,680 @@ +package com.launchdarkly.sdk.server.integrations; + +import com.launchdarkly.logging.LDLogger; +import com.launchdarkly.logging.LogCapture; +import com.launchdarkly.logging.Logs; +import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.integrations.FileDataReloader.EmptiedFileException; +import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; +import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.DataKind; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.ItemDescriptor; +import com.launchdarkly.sdk.server.subsystems.DataStoreTypes.KeyedItems; +import com.launchdarkly.testhelpers.TempDir; + +import org.junit.After; +import org.junit.Test; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.LongSupplier; + +import static com.launchdarkly.sdk.server.DataModel.FEATURES; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.not; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +/** + * The reloader with the override files loader, on real files that are rewritten in place: a file + * whose content is in effect and is found empty is deferred until it has stayed empty for the retry + * delay. + */ +@SuppressWarnings("javadoc") +public class FileDataReloaderEmptiedFileTest { + // Long enough that a test finishes what it does while a file is empty before the retry reads the + // file again, also on a loaded machine. + private static final Duration LONG_RETRY = Duration.ofMillis(1500); + private static final long WAIT_MILLIS = 10000; + private static final String REREAD_LOG = "DEBUG:Reading flag data again after a file was found empty"; + + private final LogCapture logCapture = Logs.capture(); + private final LDLogger logger = LDLogger.withAdapter(logCapture, ""); + private final List reloaders = new ArrayList<>(); + + @After + public void closeReloaders() { + for (FileDataReloader r : reloaders) { + r.close(); + } + } + + /** + * Records the value of the flag "flag" in each applied snapshot, or "none" when a snapshot does + * not have it, and every reported failure. + */ + private static final class Recorder implements FileDataReloader.Handler { + final List applied = Collections.synchronizedList(new ArrayList<>()); + final List errors = Collections.synchronizedList(new ArrayList<>()); + + @Override + public void apply(LoadResult result) { + String value = "none"; + for (Map.Entry> kind : result.getData()) { + if (kind.getKey() != FEATURES) { + continue; + } + for (Map.Entry item : kind.getValue().getItems()) { + if (item.getKey().equals("flag")) { + LDValue v = LDValue.parse(FEATURES.serialize(item.getValue())).get("variations").get(0); + value = v.isString() ? v.stringValue() : v.toJsonString(); + } + } + } + applied.add(value); + } + + @Override + public void onError(FileDataException e) { + errors.add(e); + } + } + + /** + * The override files loader, counting the loads that have finished. + */ + private static final class CountingLoader implements FileDataReloader.Loader { + final OverrideFileLoader loader; + final AtomicInteger finished = new AtomicInteger(); + + CountingLoader(Path... paths) { + loader = new OverrideFileLoader(Arrays.asList(paths), FileData.DuplicateKeysHandling.FAIL); + } + + CountingLoader(LongSupplier clock, Path... paths) { + loader = new OverrideFileLoader(Arrays.asList(paths), FileData.DuplicateKeysHandling.FAIL, clock); + } + + @Override + public LoadResult load(Duration retryDelay) throws FileDataException, EmptiedFileException { + try { + return loader.load(retryDelay); + } finally { + finished.incrementAndGet(); + } + } + + @Override + public void accepted(LoadResult result) { + loader.accepted(result); + } + + void awaitFinished(int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (finished.get() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for load " + expected + ", got " + finished.get()); + } + Thread.sleep(5); + } + } + } + + private FileDataReloader reloader(CountingLoader loader, Recorder recorder, Duration retryDelay) { + FileDataReloader r = new FileDataReloader(loader, recorder, logger, Duration.ZERO, retryDelay, true); + reloaders.add(r); + return r; + } + + private static void awaitSize(List list, int expected) throws InterruptedException { + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (list.size() < expected) { + if (System.currentTimeMillis() > deadline) { + throw new AssertionError("timed out waiting for size " + expected + ", got " + list); + } + Thread.sleep(5); + } + } + + private static Path write(TempDir dir, String name, String contents) throws Exception { + Path p = dir.getPath().resolve(name); + Files.write(p, contents.getBytes()); + return p; + } + + private static String yaml(String value) { + return "flagValues:\n flag: \"" + value + "\"\n"; + } + + private static String json(String value) { + return "{\"flagValues\":{\"flag\":\"" + value + "\"}}"; + } + + private static String readAgainLog(Path file) { + return "DEBUG:File " + file + " is empty, possibly while it is being rewritten; reading it again before applying it"; + } + + // For the tests that move a clock themselves: a retry delay so long that no retry fires during the + // test, so every load is one the test starts with reloadNow. + private static final Duration HOUR = Duration.ofHours(1); + + private static final long TENTH = HOUR.toNanos() / 10; + + // Writes the file again, in place, with a later modification time than the previous write. File + // system timestamps can be coarse, so it waits until the time differs. + private static void writeAgain(Path file, String contents) throws Exception { + long before = Files.getLastModifiedTime(file).toMillis(); + long deadline = System.currentTimeMillis() + 5000; + do { + Thread.sleep(20); + InPlaceWrites.rewrite(file, "x", () -> { }); + InPlaceWrites.rewrite(file, contents, () -> { }); + } while (Files.getLastModifiedTime(file).toMillis() == before && System.currentTimeMillis() < deadline); + } + + @Test + public void holdAsksForTheRetryWhenTheDelayIsOver() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + FileDataReloader r = reloader(new CountingLoader(clock::get, file), new Recorder(), HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.reloadNow(); + assertEquals(HOUR.toNanos() + TENTH, r.getLastRequestedRetryNanos()); + + // A later load asks for the rest of the delay, not a whole one. + clock.addAndGet(Duration.ofMinutes(30).toNanos()); + r.reloadNow(); + assertEquals(Duration.ofMinutes(30).toNanos() + TENTH, r.getLastRequestedRetryNanos()); + } + } + + @Test + public void holdAsksForTheRetryWhenTheBoundIsOver() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(new CountingLoader(clock::get, file), recorder, HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.reloadNow(); + for (int i = 0; i < 4; i++) { + clock.addAndGet(Duration.ofMinutes(60).toNanos()); + writeAgain(file, i % 2 == 0 ? " " : " "); + r.reloadNow(); + } + clock.addAndGet(Duration.ofMinutes(48).toNanos()); + writeAgain(file, " "); + r.reloadNow(); + + // The file's delay would be over in an hour, but its bound is over in 12 minutes. + assertEquals(Collections.singletonList("a"), recorder.applied); + assertEquals(Duration.ofMinutes(12).toNanos() + TENTH, r.getLastRequestedRetryNanos()); + } + } + + @Test + public void holdOfSeveralFilesAsksForTheRetryWhenTheFirstIsDue() throws Exception { + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", yaml("a")); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"b\"\n"); + AtomicLong clock = new AtomicLong(); + FileDataReloader r = reloader(new CountingLoader(clock::get, a, b), new Recorder(), HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(a); + r.reloadNow(); + clock.addAndGet(Duration.ofMinutes(20).toNanos()); + InPlaceWrites.truncate(b); + r.reloadNow(); + assertEquals(Duration.ofMinutes(40).toNanos() + TENTH, r.getLastRequestedRetryNanos()); + } + } + + @Test + public void holdStartedByAChangeSignalAsksForTheRetryWhenTheDelayIsOver() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + CountingLoader loader = new CountingLoader(clock::get, file); + FileDataReloader r = reloader(loader, new Recorder(), HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.trigger(); + loader.awaitFinished(2); + long deadline = System.currentTimeMillis() + WAIT_MILLIS; + while (r.getLastRequestedRetryNanos() != HOUR.toNanos() + TENTH && System.currentTimeMillis() < deadline) { + Thread.sleep(5); + } + assertEquals(HOUR.toNanos() + TENTH, r.getLastRequestedRetryNanos()); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(file))); + } + } + + @Test + public void failedLoadAsksForTheRetryDelay() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(new CountingLoader(clock::get, file), recorder, HOUR); + r.reloadNow(); + + InPlaceWrites.rewrite(file, "flagValues: [", () -> { }); + r.reloadNow(); + assertEquals(1, recorder.errors.size()); + assertEquals(HOUR.toNanos(), r.getLastRequestedRetryNanos()); + } + } + + @Test + public void holdEndsExactlyAfterTheRetryDelay() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + CountingLoader loader = new CountingLoader(clock::get, file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.reloadNow(); + clock.addAndGet(HOUR.toNanos() * 95 / 100); + r.reloadNow(); + assertEquals(Collections.singletonList("a"), recorder.applied); + clock.addAndGet(HOUR.toNanos() * 5 / 100); + r.reloadNow(); + assertEquals(Arrays.asList("a", "none"), recorder.applied); + assertTrue(recorder.errors.isEmpty()); + } + } + + @Test + public void fileRewrittenWithTheSameContentIsHeldBackAgainLater() throws Exception { + // A tool rewrites the file in place now and then with the same content. The load after a rewrite + // is skipped as unchanged, and that must start the file's bound over as an application does, or + // a rewrite five retry delays later would be applied while the file is empty. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + AtomicLong clock = new AtomicLong(); + CountingLoader loader = new CountingLoader(clock::get, file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.reloadNow(); + InPlaceWrites.rewrite(file, yaml("a"), () -> { }); + r.reloadNow(); + assertEquals(3, loader.finished.get()); + assertEquals(Collections.singletonList("a"), recorder.applied); + + clock.addAndGet(HOUR.toNanos() * 6); + InPlaceWrites.truncate(file); + r.reloadNow(); + assertEquals(Collections.singletonList("a"), recorder.applied); + assertTrue(recorder.errors.isEmpty()); + } + } + + @Test + public void sameBytesMovedToAFileOfAnotherFormatAreApplied() throws Exception { + // The format depends on the path, so the load after the move is not skipped as unchanged: YAML + // reads on as true, and JSON reads it as a string. + try (TempDir dir = TempDir.create()) { + Path yaml = write(dir, "overrides.yaml", "{\"flagValues\": {\"flag\": on}}"); + Path json = dir.getPath().resolve("overrides.json"); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(new CountingLoader(yaml, json), recorder, HOUR); + r.reloadNow(); + Files.move(yaml, json); + r.reloadNow(); + assertEquals(Arrays.asList("true", "on"), recorder.applied); + } + } + + @Test + public void changesToOtherFilesWaitWhileAFileIsHeld() throws Exception { + // A reload applies all the files or none. + try (TempDir dir = TempDir.create()) { + Path emptied = write(dir, "emptied.yaml", "flagValues:\n other: \"x\"\n"); + Path changed = write(dir, "changed.yaml", yaml("b1")); + AtomicLong clock = new AtomicLong(); + CountingLoader loader = new CountingLoader(clock::get, emptied, changed); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, HOUR); + r.reloadNow(); + + InPlaceWrites.truncate(emptied); + InPlaceWrites.rewrite(changed, yaml("b2"), () -> { }); + r.reloadNow(); + assertEquals(Collections.singletonList("b1"), recorder.applied); + clock.addAndGet(HOUR.toNanos()); + r.reloadNow(); + assertEquals(Arrays.asList("b1", "b2"), recorder.applied); + } + } + + @Test + public void emptiedYamlFileIsReadAgainAndTheRewrittenContentIsApplied() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.rewrite(file, yaml("b"), () -> { + // The change signal of the truncation starts a load that finds the file empty. + r.trigger(); + loader.awaitFinished(2); + }); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "b"), recorder.applied); + assertTrue(recorder.errors.isEmpty()); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(file))); + } + } + + @Test + public void yamlFileThatStaysEmptyIsAppliedByTheRetry() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.trigger(); + loader.awaitFinished(2); + assertEquals(Collections.singletonList("a"), recorder.applied); + + // No further signal arrives. The retry reads the file again and applies it. + awaitSize(recorder.applied, 2); + assertEquals(Arrays.asList("a", "none"), recorder.applied); + assertEquals(3, loader.finished.get()); + assertTrue(recorder.errors.isEmpty()); + assertThat(logCapture.getMessageStrings(), hasItem(REREAD_LOG)); + } + } + + @Test + public void emptiedJsonFileIsReportedOnlyByTheLoadAfterTheOneThatFindsItEmpty() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", json("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.trigger(); + loader.awaitFinished(2); + assertTrue(recorder.errors.isEmpty()); + + awaitSize(recorder.errors, 1); + assertThat(FileDataReloader.describe(recorder.errors.get(0)), + containsString("the file has no content, which is not a JSON document")); + assertEquals(Collections.singletonList("a"), recorder.applied); + } + } + + @Test + public void furtherChangeSignalWithinTheGraceDoesNotApplyTheEmptyFile() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.trigger(); + loader.awaitFinished(2); + r.trigger(); + loader.awaitFinished(3); + assertEquals(Collections.singletonList("a"), recorder.applied); + + // The retry armed by the second load finds that the file has stayed empty for the retry delay. + awaitSize(recorder.applied, 2); + assertEquals(Arrays.asList("a", "none"), recorder.applied); + assertEquals(4, loader.finished.get()); + assertThat(logCapture.getMessageStrings(), hasItem(REREAD_LOG)); + } + } + + @Test + public void secondRewriteWithinOneDebounceWindowIsDeferredToo() throws Exception { + // F1: the write of b and the next truncation happen before any load reads b. The next load + // finds the file empty again; the file is in a new rewrite, not one that stayed empty. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.rewrite(file, yaml("b"), () -> { + r.trigger(); + loader.awaitFinished(2); + }); + Thread.sleep(50); + InPlaceWrites.rewrite(file, yaml("c"), () -> { + r.trigger(); + loader.awaitFinished(3); + }); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "c"), recorder.applied); + } + } + + @Test + public void changeToAnotherFileWhileAFileIsEmptyDoesNotApplyTheEmptyFile() throws Exception { + // F2: a change to b starts a load while a is still in the middle of its rewrite. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", yaml("a")); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"b1\"\n"); + CountingLoader loader = new CountingLoader(a, b); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.rewrite(a, yaml("a2"), () -> { + r.trigger(); + loader.awaitFinished(2); + InPlaceWrites.rewrite(b, "flagValues:\n other: \"b2\"\n", () -> { }); + r.trigger(); + loader.awaitFinished(3); + }); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "a2"), recorder.applied); + } + } + + @Test + public void retryThatFindsANewTruncationIsDeferredToo() throws Exception { + // F3: the file is rewritten, and rewritten again before any load reads the first new content. + // The retry armed by the first deferral finds the second truncation. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.rewrite(file, yaml("b"), () -> { + r.trigger(); + loader.awaitFinished(2); + }); + Thread.sleep(50); + InPlaceWrites.rewrite(file, yaml("c"), () -> loader.awaitFinished(3)); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "c"), recorder.applied); + assertThat(logCapture.getMessageStrings(), hasItem(REREAD_LOG)); + } + } + + @Test + public void filesEmptiedTogetherAreAppliedByTheFirstRetry() throws Exception { + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", yaml("a")); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"b\"\n"); + CountingLoader loader = new CountingLoader(a, b); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.truncate(a); + InPlaceWrites.truncate(b); + r.trigger(); + loader.awaitFinished(2); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "none"), recorder.applied); + assertEquals(3, loader.finished.get()); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(a))); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(b))); + } + } + + @Test + public void fileMovedToAnotherConfiguredPathIsDeferredWhenItIsRewritten() throws Exception { + // Moving a file from one configured path to another leaves the same flags in effect. The load + // after the move records that the content now comes from the new path, or an in-place rewrite + // of that path would be applied while it is empty. The skip-unchanged digest covers the paths, + // so that load is applied rather than skipped. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", yaml("a")); + Path b = dir.getPath().resolve("b.yaml"); + CountingLoader loader = new CountingLoader(a, b); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + Files.move(a, b); + r.trigger(); + awaitSize(recorder.applied, 2); + assertEquals(Arrays.asList("a", "a"), recorder.applied); + + InPlaceWrites.rewrite(b, yaml("b"), () -> { + r.trigger(); + loader.awaitFinished(3); + }); + r.trigger(); + awaitSize(recorder.applied, 3); + + assertEquals(Arrays.asList("a", "a", "b"), recorder.applied); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(b))); + } + } + + @Test + public void emptyPlaceholderFileDoesNotDelayAChangeToAnotherFile() throws Exception { + try (TempDir dir = TempDir.create()) { + Path placeholder = write(dir, "placeholder.yaml", ""); + Path data = write(dir, "data.json", json("a")); + CountingLoader loader = new CountingLoader(placeholder, data); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + InPlaceWrites.rewrite(data, json("b"), () -> { }); + r.trigger(); + awaitSize(recorder.applied, 2); + + // The load of the signal applied the change, without waiting for a retry. + assertEquals(Arrays.asList("a", "b"), recorder.applied); + assertEquals(2, loader.finished.get()); + assertThat(logCapture.getMessageStrings(), not(hasItem(readAgainLog(placeholder)))); + } + } + + @Test + public void fileThatIsDeletedAndCreatedEmptyIsAppliedRightAway() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + Files.delete(file); + r.trigger(); + loader.awaitFinished(2); + Files.write(file, new byte[0]); + r.trigger(); + awaitSize(recorder.applied, 3); + + assertEquals(Arrays.asList("a", "none", "none"), recorder.applied); + assertEquals(3, loader.finished.get()); + assertThat(logCapture.getMessageStrings(), not(hasItem(readAgainLog(file)))); + } + } + + @Test + public void retryLeftOverFromAFailureThatFindsTheFileEmptiedReadsItAgain() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, LONG_RETRY); + r.reloadNow(); + + // A load fails and arms the retry. + InPlaceWrites.rewrite(file, "flagValues: [", () -> { }); + r.trigger(); + awaitSize(recorder.errors, 1); + + // The file is rewritten again, and only the retry reads it while it is empty. + InPlaceWrites.rewrite(file, yaml("b"), () -> loader.awaitFinished(3)); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "b"), recorder.applied); + assertThat(logCapture.getMessageStrings(), hasItem(readAgainLog(file))); + } + } + + @Test + public void withoutRetriesAnEmptiedFileIsAppliedRightAway() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", yaml("a")); + CountingLoader loader = new CountingLoader(file); + Recorder recorder = new Recorder(); + FileDataReloader r = reloader(loader, recorder, Duration.ZERO); + r.reloadNow(); + + InPlaceWrites.truncate(file); + r.trigger(); + awaitSize(recorder.applied, 2); + + assertEquals(Arrays.asList("a", "none"), recorder.applied); + assertEquals(2, loader.finished.get()); + assertThat(logCapture.getMessageStrings(), not(hasItem(readAgainLog(file)))); + } + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java index 017906bc..8ded2803 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/FileDataReloaderTest.java @@ -3,14 +3,18 @@ import com.launchdarkly.logging.LDLogger; import com.launchdarkly.logging.LogCapture; import com.launchdarkly.logging.Logs; +import com.launchdarkly.sdk.server.integrations.FileDataReloader.EmptiedFileException; import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; import org.junit.After; import org.junit.Test; +import java.nio.file.Path; +import java.nio.file.Paths; import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.CountDownLatch; @@ -18,7 +22,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Supplier; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -26,6 +29,7 @@ import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.lessThan; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -63,14 +67,24 @@ private static FileDataException failure(String message) { return new FileDataException(message, null, null); } + /** + * One step of a scripted loader: it returns a result or throws. + */ + private interface Step { + LoadResult run() throws FileDataException, EmptiedFileException; + } + /** * A scripted loader: each call takes the next step from the queue. A step either returns a result - * or throws. When the queue is empty the last step repeats. + * or throws. When the queue is empty the last step repeats. It records the argument of each call + * and every accepted result. */ private static final class ScriptedLoader implements FileDataReloader.Loader { final AtomicInteger calls = new AtomicInteger(); - final LinkedBlockingQueue> steps = new LinkedBlockingQueue<>(); - volatile Supplier last; + final List retryDelayArguments = Collections.synchronizedList(new ArrayList<>()); + final List accepted = Collections.synchronizedList(new ArrayList<>()); + final LinkedBlockingQueue steps = new LinkedBlockingQueue<>(); + volatile Step last; volatile CountDownLatch entered; volatile CountDownLatch release; @@ -81,7 +95,7 @@ ScriptedLoader then(LoadResult result) { ScriptedLoader thenFail(String message) { steps.add(() -> { - throw new UncheckedFileDataException(failure(message)); + throw failure(message); }); return this; } @@ -93,12 +107,24 @@ ScriptedLoader thenThrow(RuntimeException e) { return this; } + ScriptedLoader thenEmptied(Path path) { + return thenEmptied(path, SHORT); + } + + ScriptedLoader thenEmptied(Path path, Duration retryAfter) { + steps.add(() -> { + throw new EmptiedFileException(Collections.singletonList(path), retryAfter.toNanos()); + }); + return this; + } + @Override - public LoadResult load() throws FileDataException { + public LoadResult load(Duration retryDelay) throws FileDataException, EmptiedFileException { // Both latches are read before the entered signal, so a test that clears them after the // signal cannot change what this call does. CountDownLatch enteredLatch = entered; CountDownLatch releaseLatch = release; + retryDelayArguments.add(retryDelay); calls.incrementAndGet(); if (enteredLatch != null) { enteredLatch.countDown(); @@ -110,26 +136,18 @@ public LoadResult load() throws FileDataException { Thread.currentThread().interrupt(); } } - Supplier step = steps.poll(); + Step step = steps.poll(); if (step == null) { step = last; } else { last = step; } - try { - return step.get(); - } catch (UncheckedFileDataException e) { - throw e.wrapped; - } + return step.run(); } - } - - @SuppressWarnings("serial") - private static final class UncheckedFileDataException extends RuntimeException { - final FileDataException wrapped; - UncheckedFileDataException(FileDataException wrapped) { - this.wrapped = wrapped; + @Override + public void accepted(LoadResult result) { + accepted.add(result); } } @@ -539,4 +557,147 @@ public void unexpectedRuntimeExceptionFromLoaderIsLoggedAndWorkerSurvives() thro assertThat(logCapture.getMessageStrings(), hasItem(startsWith("ERROR:Unexpected error while reloading flag data:"))); } + + private static final Path EMPTIED = Paths.get("/overrides/emptied.yaml"); + private static final String READ_AGAIN_MESSAGE = "DEBUG:File " + EMPTIED + + " is empty, possibly while it is being rewritten; reading it again before applying it"; + + @Test + public void emptiedFileIsReadAgainAfterTheRetryDelayWithoutReportingAFailure() throws Exception { + LoadResult first = resultWithHash(1); + LoadResult rewritten = resultWithHash(2); + ScriptedLoader loader = new ScriptedLoader().then(first).thenEmptied(EMPTIED).then(rewritten); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + r.reloadNow(); + + // The load that finds the file emptied applies nothing. Only the retry, with no further + // trigger, loads again. + r.trigger(); + awaitSize(handler.applied, 2); + + assertEquals(first, handler.applied.get(0)); + assertEquals(rewritten, handler.applied.get(1)); + assertEquals(3, loader.calls.get()); + assertTrue(handler.errors.isEmpty()); + List messages = logCapture.getMessageStrings(); + assertThat(messages, hasItem(READ_AGAIN_MESSAGE)); + assertThat(messages, hasItem("DEBUG:Reading flag data again after a file was found empty")); + assertThat(messages, not(hasItem("DEBUG:Retrying flag data load after earlier failure"))); + for (String message : messages) { + assertFalse(message, message.startsWith("ERROR:")); + } + } + + @Test + public void holdIsRetriedAfterTheTimeTheLoaderAsksFor() throws Exception { + // The loader asks for a retry sooner than the retry delay, which would not come in this test. + LoadResult rewritten = resultWithHash(2); + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).thenEmptied(EMPTIED, Duration.ofMillis(30)) + .then(rewritten); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ofHours(1), true); + r.reloadNow(); + + r.trigger(); + awaitSize(handler.applied, 2); + assertEquals(rewritten, handler.applied.get(1)); + assertEquals(3, loader.calls.get()); + assertEquals(Duration.ofMillis(30).toNanos(), r.getLastRequestedRetryNanos()); + } + + @Test + public void failureAfterAHoldIsRetriedAfterTheRetryDelay() throws Exception { + // A hold asks for a retry soon. The failure that follows arms the normal retry delay, which does + // not come in this test. + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).thenEmptied(EMPTIED, Duration.ofMillis(30)) + .thenFail("broken").then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ofHours(1), true); + r.reloadNow(); + + r.trigger(); + awaitSize(handler.errors, 1); + Thread.sleep(300); + assertEquals(3, loader.calls.get()); + assertEquals(1, handler.applied.size()); + assertEquals(Duration.ofHours(1).toNanos(), r.getLastRequestedRetryNanos()); + } + + @Test + public void loaderIsGivenTheRetryDelayOrZeroWithoutRetries() { + ScriptedLoader withRetries = new ScriptedLoader().then(resultWithHash(1)); + reloader(withRetries, new RecordingHandler(), Duration.ZERO, SHORT, true).reloadNow(); + assertEquals(Collections.singletonList(SHORT), withRetries.retryDelayArguments); + + // Without retries nothing would read a deferred file again, so the loader must not defer one. + ScriptedLoader withoutRetries = new ScriptedLoader().then(resultWithHash(1)); + reloader(withoutRetries, new RecordingHandler(), Duration.ZERO, Duration.ofMillis(-1), true).reloadNow(); + assertEquals(Collections.singletonList(Duration.ZERO), withoutRetries.retryDelayArguments); + } + + @Test + public void loaderIsToldOfEveryResultThatBecomesTheDataInEffect() throws Exception { + // An applied result and a result skipped as unchanged are the data in effect. A failure, a + // deferral and a result that the consumer rejects are not. + LoadResult first = resultWithHash(1); + LoadResult unchanged = resultWithHash(1); + LoadResult rejected = resultWithHash(2); + LoadResult last = resultWithHash(3); + ScriptedLoader loader = new ScriptedLoader().then(first).then(unchanged).thenFail("broken") + .thenEmptied(EMPTIED).then(rejected).then(last); + RecordingHandler handler = new RecordingHandler() { + @Override + public void apply(LoadResult r) { + if (r == rejected) { + throw new IllegalStateException("consumer failed"); + } + super.apply(r); + } + }; + FileDataReloader r = reloader(loader, handler, Duration.ZERO, Duration.ZERO, true); + r.reloadNow(); + for (int i = 2; i <= 6; i++) { + r.trigger(); + awaitAtLeast(loader.calls, i); + } + awaitSize(loader.accepted, 3); + + assertEquals(Arrays.asList(first, unchanged, last), loader.accepted); + assertEquals(Arrays.asList(first, last), handler.applied); + } + + @Test + public void failureAfterAReadAgainIsRetriedAsAFailure() throws Exception { + ScriptedLoader loader = new ScriptedLoader().then(resultWithHash(1)).thenEmptied(EMPTIED).thenFail("broken") + .then(resultWithHash(2)); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + r.reloadNow(); + + r.trigger(); + awaitSize(handler.applied, 2); + assertEquals(4, loader.calls.get()); + assertEquals(1, handler.errors.size()); + List messages = logCapture.getMessageStrings(); + assertThat(messages, hasItem("DEBUG:Reading flag data again after a file was found empty")); + assertThat(messages, hasItem("DEBUG:Retrying flag data load after earlier failure")); + } + + @Test + public void readingAnEmptiedFileAgainDoesNotEndARecovery() throws Exception { + // After a failure, the next success applies even when its content is unchanged. A read again + // in between is not that success. + LoadResult result = resultWithHash(1); + ScriptedLoader loader = new ScriptedLoader().then(result).thenFail("broken").thenEmptied(EMPTIED).then(result); + RecordingHandler handler = new RecordingHandler(); + FileDataReloader r = reloader(loader, handler, Duration.ZERO, SHORT, true); + r.reloadNow(); + + r.trigger(); + awaitSize(handler.applied, 2); + assertEquals(4, loader.calls.get()); + assertEquals(1, handler.errors.size()); + assertThat(logCapture.getMessageStrings(), hasItem(READ_AGAIN_MESSAGE)); + } } diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/InPlaceWrites.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/InPlaceWrites.java new file mode 100644 index 00000000..69273d79 --- /dev/null +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/InPlaceWrites.java @@ -0,0 +1,49 @@ +package com.launchdarkly.sdk.server.integrations; + +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.time.Duration; + +/** + * Rewrites files in place the way many applications and editors save them: the file is truncated + * when it is opened for writing, and the new content is written through the same open stream. A + * reader in between finds the file empty. The tests use these writes, not an atomic replacement, so + * that the code under test must handle a file that is being rewritten. + */ +@SuppressWarnings("javadoc") +final class InPlaceWrites { + /** + * Work done while the file is empty, between the truncation and the write. + */ + interface WhileEmpty { + void run() throws Exception; + } + + private InPlaceWrites() {} + + /** + * Truncates the file, pauses, and then writes the new content. + */ + static void rewrite(Path path, String content, Duration pause) throws Exception { + rewrite(path, content, () -> Thread.sleep(pause.toMillis())); + } + + /** + * Truncates the file, runs the given work while the file is empty, and then writes the new content. + */ + static void rewrite(Path path, String content, WhileEmpty whileEmpty) throws Exception { + try (OutputStream out = Files.newOutputStream(path)) { + whileEmpty.run(); + out.write(content.getBytes(StandardCharsets.UTF_8)); + } + } + + /** + * Truncates the file and leaves it empty. + */ + static void truncate(Path path) throws Exception { + rewrite(path, "", () -> { }); + } +} diff --git a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java index 2c00651c..cdd8665f 100644 --- a/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java +++ b/lib/sdk/server/src/test/java/com/launchdarkly/sdk/server/integrations/OverrideFileLoaderTest.java @@ -1,6 +1,7 @@ package com.launchdarkly.sdk.server.integrations; import com.launchdarkly.sdk.LDValue; +import com.launchdarkly.sdk.server.integrations.FileDataReloader.EmptiedFileException; import com.launchdarkly.sdk.server.integrations.FileDataSourceParsing.FileDataException; import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.FileSummary; import com.launchdarkly.sdk.server.integrations.OverrideFileLoader.LoadResult; @@ -12,13 +13,17 @@ import org.junit.Test; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; +import java.time.Duration; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicLong; import static com.launchdarkly.sdk.server.DataModel.FEATURES; import static com.launchdarkly.sdk.server.DataModel.SEGMENTS; @@ -27,6 +32,7 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -69,7 +75,7 @@ public void loadsFlagsFlagValuesAndSegmentsKeepingDocumentVersions() throws Exce "{\"flags\":{\"f1\":{\"key\":\"f1\",\"version\":7,\"on\":true,\"variations\":[true],\"fallthrough\":{\"variation\":0}}}," + "\"flagValues\":{\"f2\":\"x\"}," + "\"segments\":{\"s1\":{\"key\":\"s1\",\"version\":9}}}"); - LoadResult result = loader(file).load(); + LoadResult result = loader(file).load(Duration.ZERO); Map> data = toMap(result); assertEquals(7, data.get(FEATURES).get("f1").getVersion()); @@ -90,7 +96,7 @@ public void loadsFlagsFlagValuesAndSegmentsKeepingDocumentVersions() throws Exce public void valueOnlyEntryBecomesFallthroughFlagServingTheValue() throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "overrides.json", "{\"flagValues\":{\"f2\":\"x\"}}"); - LoadResult result = loader(file).load(); + LoadResult result = loader(file).load(Duration.ZERO); ItemDescriptor f2 = toMap(result).get(FEATURES).get("f2"); assertEquals(0, f2.getVersion()); LDValue flag = json(FEATURES, f2); @@ -104,10 +110,10 @@ public void valueOnlyEntryBecomesFallthroughFlagServingTheValue() throws Excepti } @Test - public void yamlIsAutoDetected() throws Exception { + public void yamlFileIsParsedAsYaml() throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "overrides.yaml", "flagValues:\n yaml-flag: \"override-value\"\n"); - LoadResult result = loader(file).load(); + LoadResult result = loader(file).load(Duration.ZERO); assertEquals(LDValue.buildArray().add("override-value").build(), json(FEATURES, toMap(result).get(FEATURES).get("yaml-flag")).get("variations")); } @@ -119,7 +125,7 @@ public void missingFileContributesNothing() throws Exception { Path present = write(dir, "present.json", "{\"flagValues\":{\"flag\":\"value\"}}"); Path missing = dir.getPath().resolve("missing.json"); Path segments = write(dir, "segments.json", "{\"segments\":{\"s1\":{\"key\":\"s1\"}}}"); - LoadResult result = loader(present, missing, segments).load(); + LoadResult result = loader(present, missing, segments).load(Duration.ZERO); Map> data = toMap(result); assertEquals(1, data.get(FEATURES).size()); @@ -139,7 +145,7 @@ public void missingFileContributesNothing() throws Exception { @Test public void allFilesMissingProducesEmptyResult() throws Exception { try (TempDir dir = TempDir.create()) { - LoadResult result = loader(dir.getPath().resolve("a.json"), dir.getPath().resolve("b.json")).load(); + LoadResult result = loader(dir.getPath().resolve("a.json"), dir.getPath().resolve("b.json")).load(Duration.ZERO); assertEquals(0, result.getFlagCount()); assertEquals(0, result.getSegmentCount()); assertFalse(result.getData().iterator().hasNext()); @@ -154,7 +160,7 @@ public void unreadableFileFailsTheLoad() throws Exception { Path directory = dir.getPath().resolve("dir.json"); Files.createDirectory(directory); try { - loader(directory).load(); + loader(directory).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString(directory.toString())); @@ -170,7 +176,7 @@ public void malformedDocumentFailsTheLoadWithFileAttribution() throws Exception Path good = write(dir, "good.json", "{\"flagValues\":{\"a\":1}}"); Path bad = write(dir, "bad.json", "{\"flagValues\""); try { - loader(good, bad).load(); + loader(good, bad).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString(bad.toString())); @@ -185,7 +191,7 @@ public void modelRejectedDocumentIsAFileDataError() throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "bad-model.json", "{\"flags\":{\"f1\":{\"key\":\"f1\",\"rules\":5}}}"); try { - loader(file).load(); + loader(file).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString(file.toString())); @@ -201,7 +207,7 @@ public void duplicateKeysFailByDefaultAcrossFilesAndWithinAFile() throws Excepti Path a = write(dir, "a.json", "{\"flagValues\":{\"flag\":\"a\"}}"); Path b = write(dir, "b.json", "{\"flagValues\":{\"flag\":\"b\"}}"); try { - loader(a, b).load(); + loader(a, b).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString("in features, key \"flag\" was already defined")); @@ -211,7 +217,7 @@ public void duplicateKeysFailByDefaultAcrossFilesAndWithinAFile() throws Excepti Path both = write(dir, "both.json", "{\"flags\":{\"x\":{\"key\":\"x\"}},\"flagValues\":{\"x\":true}}"); try { - loader(both).load(); + loader(both).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString("key \"x\" was already defined")); @@ -220,7 +226,7 @@ public void duplicateKeysFailByDefaultAcrossFilesAndWithinAFile() throws Excepti Path seg1 = write(dir, "seg1.json", "{\"segments\":{\"s\":{\"key\":\"s\"}}}"); Path seg2 = write(dir, "seg2.json", "{\"segments\":{\"s\":{\"key\":\"s\"}}}"); try { - loader(seg1, seg2).load(); + loader(seg1, seg2).load(Duration.ZERO); fail("expected exception"); } catch (FileDataException e) { assertThat(e.getMessage(), containsString("in segments, key \"s\" was already defined")); @@ -234,7 +240,7 @@ public void ignoreHandlingKeepsFirstFileAndCountsOnlyKeptEntries() throws Except Path first = write(dir, "first.json", "{\"flagValues\":{\"shared\":\"first\"}}"); Path second = write(dir, "second.json", "{\"flagValues\":{\"shared\":\"second\",\"only-second\":\"x\"}}"); OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(first, second), FileData.DuplicateKeysHandling.IGNORE); - LoadResult result = loader.load(); + LoadResult result = loader.load(Duration.ZERO); Map flags = toMap(result).get(FEATURES); assertEquals(LDValue.buildArray().add("first").build(), json(FEATURES, flags.get("shared")).get("variations")); @@ -251,17 +257,17 @@ public void contentHashReflectsRawContentOfPresentFiles() throws Exception { Path file = write(dir, "overrides.json", "{\"flagValues\":{\"a\":1}}"); Path missing = dir.getPath().resolve("missing.json"); OverrideFileLoader loader = loader(file, missing); - byte[] first = loader.load().getContentHash(); - byte[] second = loader.load().getContentHash(); + byte[] first = loader.load(Duration.ZERO).getContentHash(); + byte[] second = loader.load(Duration.ZERO).getContentHash(); assertArrayEquals(first, second); Files.write(file, "{\"flagValues\":{\"a\":2}}".getBytes()); - byte[] third = loader.load().getContentHash(); + byte[] third = loader.load(Duration.ZERO).getContentHash(); assertThat(Arrays.equals(first, third), is(false)); // A file that appears changes the digest. Files.write(missing, "{}".getBytes()); - byte[] fourth = loader.load().getContentHash(); + byte[] fourth = loader.load(Duration.ZERO).getContentHash(); assertThat(Arrays.equals(third, fourth), is(false)); } } @@ -270,7 +276,7 @@ public void contentHashReflectsRawContentOfPresentFiles() throws Exception { public void emptyDocumentContributesNothing() throws Exception { try (TempDir dir = TempDir.create()) { Path file = write(dir, "empty.json", "{}"); - LoadResult result = loader(file).load(); + LoadResult result = loader(file).load(Duration.ZERO); assertEquals(0, result.getFlagCount()); assertTrue(result.getFiles().get(0).isPresent()); assertThat(result.getFiles().get(0).getFlags(), equalTo(0)); @@ -284,8 +290,1218 @@ public void loaderReadsFilesInConfiguredOrder() throws Exception { Path b = write(dir, "b.json", "{\"flagValues\":{\"k\":\"b\"}}"); OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(b, a), FileData.DuplicateKeysHandling.IGNORE); assertEquals(LDValue.buildArray().add("b").build(), - json(FEATURES, toMap(loader.load()).get(FEATURES).get("k")).get("variations")); - assertEquals(Collections.singletonList(b).get(0), loader.load().getFiles().get(0).getPath()); + json(FEATURES, toMap(loader.load(Duration.ZERO)).get(FEATURES).get("k")).get("variations")); + assertEquals(Collections.singletonList(b).get(0), loader.load(Duration.ZERO).getFiles().get(0).getPath()); + } + } + + private static final String YAML_DOC = "flagValues:\n flag: \"value\"\n"; + private static final String JSON_DOC = "{\"flagValues\":{\"flag\":\"value\"}}"; + private static final Duration RETRY = Duration.ofSeconds(1); + + /** + * A monotonic clock that the test moves. It starts near the largest value, so that elapsed times + * cross the overflow of the clock's readings. + */ + private static final class TestClock { + final AtomicLong nanos = new AtomicLong(Long.MAX_VALUE - RETRY.toNanos() / 2); + + void advance(double retryDelays) { + nanos.addAndGet((long) (retryDelays * RETRY.toNanos())); + } + } + + private static OverrideFileLoader loader(TestClock clock, Path... paths) { + return new OverrideFileLoader(Arrays.asList(paths), FileData.DuplicateKeysHandling.FAIL, clock.nanos::get); + } + + // Loads and applies the result, as the reloader does with a result it applies. + private static LoadResult loadAndAccept(OverrideFileLoader loader) throws Exception { + LoadResult result = loader.load(RETRY); + loader.accepted(result); + return result; + } + + private static void assertNoContentFailure(OverrideFileLoader loader, Path file) throws Exception { + try { + loader.load(RETRY); + fail("expected exception for " + file); + } catch (FileDataException e) { + assertEquals("file " + file + ": the file has no content, which is not a JSON document", e.getMessage()); + assertNull(e.getCause()); + } + } + + // Asserts that a load holds back the given files and returns how long it asks the reloader to + // wait, in retry delays. + private static double assertHeld(OverrideFileLoader loader, Path... files) throws Exception { + try { + loader.load(RETRY); + fail("expected the empty files " + Arrays.asList(files) + " to be held back"); + return 0; + } catch (EmptiedFileException e) { + assertEquals(Arrays.asList(files), e.getPaths()); + return (double) e.getRetryAfterNanos() / RETRY.toNanos(); + } + } + + // Writes the file again, in place, with a later modification time than the previous write. File + // system timestamps can be coarse, so it waits until the time differs. It writes other content first, + // so that the file is modified even when the new content is what it already holds. + private static void writeAgain(Path file, String contents) throws Exception { + long before = Files.getLastModifiedTime(file).toMillis(); + long deadline = System.currentTimeMillis() + 5000; + do { + Thread.sleep(20); + InPlaceWrites.rewrite(file, "x", () -> { }); + InPlaceWrites.rewrite(file, contents, () -> { }); + } while (Files.getLastModifiedTime(file).toMillis() == before && System.currentTimeMillis() < deadline); + assertThat(Files.getLastModifiedTime(file).toMillis(), not(equalTo(before))); + } + + private static void truncateAgain(Path file) throws Exception { + writeAgain(file, ""); + } + + @Test + public void formatIsChosenByTheFileNameExtension() throws Exception { + try (TempDir dir = TempDir.create()) { + for (String name : new String[] { "a.yaml", "a.yml", "a.YAML", "a.Yml" }) { + assertEquals(name, 1, loader(write(dir, name, YAML_DOC)).load(Duration.ZERO).getFlagCount()); + } + // A YAML file can hold most JSON documents. The YAML parser reads YAML 1.1, which rejects some + // JSON, such as indentation with tabs or the escape \/. + assertEquals(1, loader(write(dir, "json.yaml", JSON_DOC)).load(Duration.ZERO).getFlagCount()); + for (String json : new String[] { "{\n\t\"flagValues\": {\"flag\": \"v\"}\n}", "{\"flagValues\":{\"flag\":\"a\\/b\"}}" }) { + Path file = write(dir, "strict.yaml", json); + try { + loader(file).load(Duration.ZERO); + fail("expected the YAML parser to reject " + json); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(file + ": unable to parse YAML")); + } + } + // Any other file is JSON, whatever its content. + for (String name : new String[] { "b.json", "b.txt", "b", "b.yaml.bak" }) { + Path file = write(dir, name, YAML_DOC); + try { + loader(file).load(Duration.ZERO); + fail("expected exception for " + name); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(file + ": cannot parse JSON")); + } + } + } + } + + @Test + public void emptyOrWhiteSpaceJsonFileFailsToParse() throws Exception { + try (TempDir dir = TempDir.create()) { + // A file with no extension or another extension is JSON, so it gets the same rule. + for (String name : new String[] { "empty.json", "empty", "empty.txt" }) { + for (String contents : new String[] { "", " \n\t\r\n" }) { + Path file = write(dir, name, contents); + assertNoContentFailure(loader(file), file); + } + } + } + } + + @Test + public void yamlFileWithNoDocumentHasNoEntries() throws Exception { + try (TempDir dir = TempDir.create()) { + for (String name : new String[] { "empty.yaml", "empty.yml" }) { + for (String contents : new String[] { "", " \n\n", "# only a comment\n", "---\n" }) { + LoadResult result = loader(write(dir, name, contents)).load(RETRY); + assertEquals(0, result.getFlagCount()); + assertTrue(result.getFiles().get(0).isPresent()); + assertFalse(contents, result.getFiles().get(0).hasContent()); + } + } + } + } + + @Test + public void documentWithNoEntriesHasContent() throws Exception { + try (TempDir dir = TempDir.create()) { + for (String name : new String[] { "empty.yaml", "empty.json" }) { + LoadResult result = loader(write(dir, name, "{}")).load(RETRY); + assertEquals(0, result.getFlagCount()); + assertTrue(name, result.getFiles().get(0).hasContent()); + } + } + } + + @Test + public void emptiedYamlFileIsHeldForExactlyTheRetryDelayAndThenHasNoEntries() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + assertTrue(loadAndAccept(loader).getFiles().get(0).hasContent()); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(0.95); + assertHeld(loader, file); + // The file has stayed empty without being written for the retry delay. + clock.advance(0.05); + assertEquals(0, loader.load(RETRY).getFlagCount()); + // It is not held back again until it changes. + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void emptiedJsonFileIsHeldForTheRetryDelayAndThenFailsToParse() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", JSON_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(1); + assertNoContentFailure(loader, file); + assertNoContentFailure(loader, file); + } + } + + @Test + public void holdAsksForARetryWhenTheFirstDelayEnds() throws Exception { + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.yaml", YAML_DOC); + Path second = write(dir, "second.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, first, second); + loadAndAccept(loader); + + // The retry comes when the delay ends, plus a tenth of the retry delay for a timer that fires a + // little early. + InPlaceWrites.truncate(first); + assertEquals(1.1, assertHeld(loader, first), 1e-9); + clock.advance(0.4); + assertEquals(0.7, assertHeld(loader, first), 1e-9); + // A second file found empty later does not delay the retry for the first. + InPlaceWrites.truncate(second); + clock.advance(0.2); + assertEquals(0.5, assertHeld(loader, first, second), 1e-9); + } + } + + @Test + public void holdAsksForARetryWhenTheBoundEnds() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + for (int i = 0; i < 4; i++) { + clock.advance(0.9); + writeAgain(file, " "); + assertHeld(loader, file); + } + // 4.6 retry delays after the first hold, the file is written again. Its delay would end at 5.6, + // its bound at 5.0. + clock.advance(1.0); + writeAgain(file, " "); + assertEquals(0.5, assertHeld(loader, file), 1e-9); + } + } + + @Test + public void contentThatIsOnlyWhiteSpaceOrAByteOrderMarkCountsAsEmpty() throws Exception { + // A read can see only the byte order mark that an editor writes first. For JSON that is not a + // parse failure to report but an empty file to hold back. + for (String name : new String[] { "overrides.yaml", "overrides.json" }) { + try (TempDir dir = TempDir.create()) { + String doc = name.endsWith(".json") ? JSON_DOC : YAML_DOC; + Path file = write(dir, name, doc); + OverrideFileLoader loader = loader(new TestClock(), file); + for (String contents : new String[] { " \n\t\r\n", "\uFEFF", "\uFEFF\n" }) { + assertEquals(name, 1, loadAndAccept(loader).getFlagCount()); + InPlaceWrites.rewrite(file, contents, () -> { }); + assertHeld(loader, file); + InPlaceWrites.rewrite(file, doc, () -> { }); + } + } + } + } + + @Test + public void fileEmptiedAgainWithTheSameStateAfterContentStartsANewDelay() throws Exception { + // File system timestamps are coarse, so a file can be emptied again with the modification time + // and size that it had when it was last found empty. A load that read content in between shows + // that this is a new emptying, so its delay starts over. Nothing is applied in between: the test + // does not accept the result of the load that reads the content. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + java.nio.file.attribute.FileTime emptiedAt = Files.getLastModifiedTime(file); + assertHeld(loader, file); + clock.advance(0.8); + InPlaceWrites.rewrite(file, YAML_DOC, () -> { }); + assertEquals(1, loader.load(RETRY).getFlagCount()); + + clock.advance(0.4); + InPlaceWrites.truncate(file); + Files.setLastModifiedTime(file, emptiedAt); + assertHeld(loader, file); + // The new delay is still running a little later. + clock.advance(0.2); + assertHeld(loader, file); + } + } + + @Test + public void yamlWithOnlyACommentOrADocumentMarkerIsHeld() throws Exception { + // A read can see only the first line of a YAML file that is being written. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + for (String contents : new String[] { "# overrides for the incident\n", "---\n" }) { + assertEquals(1, loadAndAccept(loader).getFlagCount()); + InPlaceWrites.rewrite(file, contents, () -> { }); + assertHeld(loader, file); + InPlaceWrites.rewrite(file, YAML_DOC, () -> { }); + } + } + } + + @Test + public void documentWithNoEntriesIsAppliedRightAway() throws Exception { + // {} is how to remove a file's overrides without deleting it. + try (TempDir dir = TempDir.create()) { + for (String name : new String[] { "overrides.yaml", "overrides.json" }) { + Path file = write(dir, name, name.endsWith(".json") ? JSON_DOC : YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + loadAndAccept(loader); + + InPlaceWrites.rewrite(file, "{}", () -> { }); + assertEquals(name, 0, loader.load(RETRY).getFlagCount()); + } + } + } + + @Test + public void contentThatWasReadButNotAppliedDoesNotHoldTheFile() throws Exception { + // Only the data in effect counts. A load whose result was not applied, for example because + // another file failed to parse, does not make the file's later emptiness a rewrite. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + loader.load(RETRY); + + InPlaceWrites.truncate(file); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void fileThatHasNoContentFromTheStartIsNeverHeld() throws Exception { + for (String placeholderContents : new String[] { "", "# placeholder\n" }) { + try (TempDir dir = TempDir.create()) { + Path placeholder = write(dir, "placeholder.yaml", placeholderContents); + Path data = write(dir, "data.json", JSON_DOC); + OverrideFileLoader loader = loader(new TestClock(), placeholder, data); + for (int i = 0; i < 3; i++) { + assertEquals(1, loadAndAccept(loader).getFlagCount()); + } + truncateAgain(placeholder); + assertEquals(1, loadAndAccept(loader).getFlagCount()); + } + } + } + + @Test + public void fileWhoseEmptyStateIsInEffectIsNotHeldAgain() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(1); + assertEquals(0, loadAndAccept(loader).getFlagCount()); + + // A new truncation of the file whose empty state is in effect is parsed right away. + writeAgain(file, " "); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void fileThatIsDeletedAndCreatedEmptyIsParsed() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + loadAndAccept(loader); + + Files.delete(file); + assertFalse(loadAndAccept(loader).getFiles().get(0).isPresent()); + Files.write(file, new byte[0]); + LoadResult result = loader.load(RETRY); + assertTrue(result.getFiles().get(0).isPresent()); + assertEquals(0, result.getFlagCount()); + } + } + + @Test + public void newWriteStartsTheDelayAgain() throws Exception { + // A write and a new truncation that no load saw in between: the file is in a new rewrite, not one + // that has stayed empty. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(0.8); + writeAgain(file, YAML_DOC); + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(0.95); + assertHeld(loader, file); + clock.advance(0.05); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void newWriteOfALinkedFileStartsTheDelayAgain() throws Exception { + // The state of a file that is configured through a symbolic link is that of the file it points + // to; the link itself does not change when the file is written. + try (TempDir dir = TempDir.create()) { + Path target = write(dir, "target.yaml", YAML_DOC); + Path link = dir.getPath().resolve("overrides.yaml"); + try { + Files.createSymbolicLink(link, target); + } catch (UnsupportedOperationException | java.io.IOException e) { + org.junit.Assume.assumeNoException("symbolic links are not available", e); + } + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, link); + loadAndAccept(loader); + + InPlaceWrites.truncate(target); + assertHeld(loader, link); + clock.advance(0.8); + truncateAgain(target); + assertHeld(loader, link); + clock.advance(0.95); + assertHeld(loader, link); + clock.advance(0.05); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void fileThatKeepsBeingWrittenIsParsedAfterItsBound() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + // Every load finds a new write, so the delay keeps starting again, up to the bound of five retry + // delays from the first hold. + for (int i = 0; i < 5; i++) { + clock.advance(0.9); + truncateAgain(file); + assertHeld(loader, file); + } + clock.advance(0.5); + truncateAgain(file); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void filesRewrittenInTurnAreParsedAfterTheirBound() throws Exception { + // Each load finds one of the files empty and the other with content, so each file's own delay + // starts again whenever it is found empty. A read with content keeps the file's bound, so it + // still ends. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + for (int i = 0; i < 9; i++) { + clock.advance(0.5); + Path emptied = i % 2 == 0 ? b : a; + Path written = i % 2 == 0 ? a : b; + writeAgain(written, written == a ? YAML_DOC : "flagValues:\n other: \"value\"\n"); + truncateAgain(emptied); + assertHeld(loader, emptied); + } + clock.advance(0.5); + writeAgain(b, "flagValues:\n other: \"value\"\n"); + truncateAgain(a); + assertEquals(1, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void loadThatHoldsNothingBackStartsTheBoundOver() throws Exception { + // A load that reads the file with content and holds nothing back starts the file's bound over, + // although nothing is applied here because the test does not accept the result. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + for (int i = 0; i < 5; i++) { + clock.advance(0.9); + truncateAgain(file); + assertHeld(loader, file); + } + writeAgain(file, YAML_DOC); + assertEquals(1, loader.load(RETRY).getFlagCount()); + truncateAgain(file); + clock.advance(0.5); + assertHeld(loader, file); + clock.advance(0.4); + assertHeld(loader, file); + } + } + + @Test + public void applyingDataStartsTheBoundOver() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + for (int i = 0; i < 5; i++) { + clock.advance(0.9); + truncateAgain(file); + assertHeld(loader, file); + } + writeAgain(file, YAML_DOC); + assertEquals(1, loadAndAccept(loader).getFlagCount()); + truncateAgain(file); + clock.advance(0.5); + assertHeld(loader, file); + clock.advance(0.4); + assertHeld(loader, file); + } + } + + @Test + public void jsonFileThatStaysEmptyIsHeldBackAgainWhenRewrittenLater() throws Exception { + // A JSON file that stays empty is parsed after the delay and fails. That load holds nothing back, + // so the file's bound starts over, and when it is later rewritten in place, the new truncation is + // held back again instead of reported. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.json", JSON_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + clock.advance(1); + assertNoContentFailure(loader, file); + clock.advance(10); + assertNoContentFailure(loader, file); + + writeAgain(file, " "); + assertHeld(loader, file); + InPlaceWrites.rewrite(file, JSON_DOC, () -> { }); + assertEquals(1, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void failingOtherFileDoesNotRestartTheBound() throws Exception { + // a.yaml keeps being written while empty. Another file fails to load now and then, which must not + // give a.yaml a fresh bound. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + String other = "{\"flagValues\":{\"other\":\"value\"}}"; + Path c = write(dir, "c.json", other); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, c); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + for (int i = 1; i < 10; i++) { + clock.advance(0.5); + writeAgain(a, " "); + if (i % 2 == 1) { + InPlaceWrites.rewrite(c, "{bad", () -> { }); + try { + loader.load(RETRY); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(c.toString())); + } + } else { + InPlaceWrites.rewrite(c, other, () -> { }); + assertHeld(loader, a); + } + } + clock.advance(0.5); + writeAgain(a, " "); + InPlaceWrites.rewrite(c, other, () -> { }); + LoadResult result = loader.load(RETRY); + assertEquals(1, result.getFlagCount()); + assertFalse(result.getFiles().get(0).hasContent()); + } + } + + // Loads the files and applies the result, as the reloader does, or reports that the load held a + // file back. + private static boolean loadApplies(OverrideFileLoader loader) throws Exception { + try { + loader.accepted(loader.load(RETRY)); + return true; + } catch (EmptiedFileException e) { + return false; + } + } + + @Test + public void filesEmptiedInStaggeredTurnsAreAppliedAfterTheBound() throws Exception { + // Two files stay empty and are truncated again in turn, so that whenever one of them has stayed + // empty for the whole delay, the other is still within its delay and holds the load back. A file + // that settles so does not get a fresh bound while the other is held back. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + boolean applied = false; + for (int i = 1; i <= 14 && !applied; i++) { + clock.advance(0.9); + if (i == 1) { + InPlaceWrites.truncate(b); + } else { + truncateAgain(i % 2 == 1 ? b : a); + } + applied = loadApplies(loader); + if (!applied) { + // The retry that the hold arms, a little after the other file's delay is over. + clock.advance(0.2); + applied = loadApplies(loader); + clock.advance(-0.2); + } + } + assertTrue("the files were held back without end", applied); + } + } + + @Test + public void boundOfAFileHeldBackDuringAFailureStartsOverOnceItIsReadAgain() throws Exception { + // While b.json fails to load, a.yaml is rewritten once and its gap is held back. Later, b.json is + // fixed while a.yaml is in the middle of another rewrite, which is held back with a bound of its own. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + String other = "{\"flagValues\":{\"other\":\"value\"}}"; + Path b = write(dir, "b.json", other); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.rewrite(b, "{bad", () -> { }); + InPlaceWrites.truncate(a); + assertLoadFailsFor(loader, b); + InPlaceWrites.rewrite(a, "flagValues:\n flag: \"other value\"\n", () -> { }); + assertLoadFailsFor(loader, b); + clock.advance(6); + assertLoadFailsFor(loader, b); + + InPlaceWrites.rewrite(b, other, () -> { }); + InPlaceWrites.truncate(a); + assertHeld(loader, a); + } + } + + @Test + public void fileFoundMissingStartsANewDelayWhenItIsCreatedEmptyAgain() throws Exception { + // While b.json fails, a.yaml is emptied, deleted, and created empty again with the state it had + // when it was first found empty, as within one tick of a coarse clock. Being missing ended its + // empty state, so it is held back with a new delay. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + String other = "{\"flagValues\":{\"other\":\"value\"}}"; + Path b = write(dir, "b.json", other); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.rewrite(b, "{bad", () -> { }); + InPlaceWrites.truncate(a); + java.nio.file.attribute.FileTime emptiedAt = Files.getLastModifiedTime(a); + assertLoadFailsFor(loader, b); + clock.advance(0.5); + Files.delete(a); + assertLoadFailsFor(loader, b); + clock.advance(0.67); + Files.write(a, new byte[0]); + Files.setLastModifiedTime(a, emptiedAt); + assertLoadFailsFor(loader, b); + + InPlaceWrites.rewrite(b, other, () -> { }); + assertHeld(loader, a); + } + } + + @Test + public void fixOfAFileSavedInPlaceWithAnErrorIsHeldBack() throws Exception { + // An operator saves the file in place with a mistake, and saves the fix in place long after. The + // loads in between fail on the file, and the fix's own gap is held back. + for (String name : new String[] { "data.yaml", "data.json" }) { + try (TempDir dir = TempDir.create()) { + boolean yaml = name.endsWith(".yaml"); + Path file = write(dir, name, yaml ? YAML_DOC : JSON_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + InPlaceWrites.rewrite(file, yaml ? "flagValues: [\n" : "{\"flagValues\": ", () -> { }); + assertLoadFailsFor(loader, file); + for (int i = 0; i < 6; i++) { + clock.advance(1); + assertLoadFailsFor(loader, file); + } + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + InPlaceWrites.rewrite(file, yaml ? "flagValues:\n flag: \"fixed\"\n" : "{\"flagValues\":{\"flag\":\"fixed\"}}", + () -> { }); + assertEquals(name, 1, loader.load(RETRY).getFlagCount()); + } + } + } + + @Test + public void fileThatFailedToParseStartsANewDelayWhenItIsEmptiedAgain() throws Exception { + // Content that fails to parse also ends the file's empty state, so an emptying after it starts a + // new delay, even when a coarse clock gives it the state of the earlier emptying. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, file); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + java.nio.file.attribute.FileTime emptiedAt = Files.getLastModifiedTime(file); + assertHeld(loader, file); + clock.advance(0.5); + InPlaceWrites.rewrite(file, "flagValues: [\n", () -> { }); + assertLoadFailsFor(loader, file); + clock.advance(0.7); + InPlaceWrites.truncate(file); + Files.setLastModifiedTime(file, emptiedAt); + assertHeld(loader, file); + } + } + + @Test + public void fileThatFailsToReadStartsItsBoundOver() throws Exception { + // A load that fails because a file cannot be read holds nothing back, so it starts that file's + // bound over too. + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + TestClock clock = new TestClock(); + AtomicLong unreadable = new AtomicLong(); + OverrideFileLoader loader = new OverrideFileLoader(Collections.singletonList(file), + FileData.DuplicateKeysHandling.FAIL, clock.nanos::get, path -> { + if (unreadable.get() != 0) { + throw new java.io.IOException("permission denied"); + } + return Files.readAllBytes(path); + }); + loadAndAccept(loader); + + InPlaceWrites.truncate(file); + assertHeld(loader, file); + unreadable.set(1); + for (int i = 0; i < 6; i++) { + clock.advance(1); + assertLoadFailsFor(loader, file); + } + unreadable.set(0); + writeAgain(file, " "); + assertHeld(loader, file); + } + } + + @Test + public void boundOfAFileAfterAFailingFileStartsOverOnceItIsReadAgain() throws Exception { + // c.json comes first. While it fails, a.yaml, which was held back, gets its content; the failing + // loads still read a.yaml, so its bound starts over, and the in-place rewrite of a.yaml that comes + // with the fix of c.json is held back. + try (TempDir dir = TempDir.create()) { + String other = "{\"flagValues\":{\"other\":\"value\"}}"; + Path c = write(dir, "c.json", other); + Path a = write(dir, "a.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, c, a); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + InPlaceWrites.rewrite(c, "{bad", () -> { }); + InPlaceWrites.rewrite(a, YAML_DOC, () -> { }); + for (int i = 0; i < 6; i++) { + assertLoadFailsFor(loader, c); + clock.advance(1); + } + InPlaceWrites.rewrite(c, other, () -> { }); + InPlaceWrites.truncate(a); + assertHeld(loader, a); + } + } + + @Test + public void failingLoadReportsTheFirstFailure() throws Exception { + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.json", "{bad"); + Path second = write(dir, "second.yaml", "flagValues: [\n"); + try { + loader(first, second).load(RETRY); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(first + ": cannot parse JSON")); + } + } + } + + @Test + public void fileAfterADuplicateKeyIsReadSoItsBoundStartsOver() throws Exception { + // The entries are merged only after every file is read, so a duplicate key between a.yaml and + // b.yaml does not keep the loads from reading c.yaml, whose bound starts over while they fail. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", "flagValues:\n k: \"a\"\n"); + String bDoc = "flagValues:\n j: \"b\"\n"; + Path b = write(dir, "b.yaml", bDoc); + Path c = write(dir, "c.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b, c); + loadAndAccept(loader); + + InPlaceWrites.truncate(c); + assertHeld(loader, c); + InPlaceWrites.rewrite(b, "flagValues:\n j: \"b\"\n k: \"dup\"\n", () -> { }); + InPlaceWrites.rewrite(c, YAML_DOC, () -> { }); + for (int i = 0; i < 8; i++) { + clock.advance(1); + try { + loader.load(RETRY); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString("key \"k\" was already defined")); + } + } + InPlaceWrites.rewrite(b, bDoc, () -> { }); + truncateAgain(c); + assertHeld(loader, c); + } + } + + @Test + public void fixOfAFileSavedInPlaceWithADuplicateKeyIsHeldBack() throws Exception { + // As with a parse error: b.yaml is saved in place with a key that a.yaml defines, then the fix is + // saved in place several retry delays later. The fix's gap is held back. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", "flagValues:\n k: \"a\"\n"); + Path b = write(dir, "b.yaml", "flagValues:\n j: \"b\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.truncate(b); + assertHeld(loader, b); + InPlaceWrites.rewrite(b, "flagValues:\n j: \"b\"\n k: \"dup\"\n", () -> { }); + for (int i = 0; i < 7; i++) { + try { + loader.load(RETRY); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString("key \"k\" was already defined")); + } + clock.advance(1); + } + InPlaceWrites.truncate(b); + assertHeld(loader, b); + } + } + + @Test + public void slowReadOfTheEmptiedFileItselfDoesNotShortenItsDelay() throws Exception { + // The emptied file's own read stalls for most of a retry delay, as on a network file system. Its + // delay counts from after its read. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + TestClock clock = new TestClock(); + AtomicLong slowRead = new AtomicLong(); + OverrideFileLoader loader = new OverrideFileLoader(Collections.singletonList(a), + FileData.DuplicateKeysHandling.FAIL, clock.nanos::get, path -> { + byte[] raw = Files.readAllBytes(path); + if (slowRead.getAndSet(0) != 0) { + clock.advance(0.9); + } + return raw; + }); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + slowRead.set(1); + assertHeld(loader, a); + clock.advance(0.2); + assertHeld(loader, a); + } + } + + @Test + public void jsonDocumentThatIsNotAnObjectFailsToParse() throws Exception { + try (TempDir dir = TempDir.create()) { + for (String doc : new String[] { "null", "[]", "5", "\"x\"" }) { + Path file = write(dir, "overrides.json", doc); + try { + loader(file).load(Duration.ZERO); + fail("expected exception for " + doc); + } catch (FileDataException e) { + assertEquals("file " + file + ": the JSON document is not an object", e.getMessage()); + assertNull(e.getCause()); + } + } + } + } + + @Test + public void textEncodingIsDetectedFromAByteOrderMarkAndIsUtf8Otherwise() throws Exception { + // Applies to both formats. Without a byte order mark the text is UTF-8, whatever the platform's + // default encoding. + String[] names = { "overrides.json", "overrides.yaml" }; + java.nio.charset.Charset[] charsets = { StandardCharsets.UTF_8, StandardCharsets.UTF_16LE, + StandardCharsets.UTF_16BE }; + try (TempDir dir = TempDir.create()) { + for (String name : names) { + String doc = name.endsWith(".json") ? "{\"flagValues\":{\"flag\":\"caf\u00e9\"}}" + : "flagValues:\n flag: \"caf\u00e9\"\n"; + for (java.nio.charset.Charset charset : charsets) { + for (boolean bom : new boolean[] { true, false }) { + if (!bom && charset != StandardCharsets.UTF_8) { + continue; + } + Path file = dir.getPath().resolve(name); + Files.write(file, ((bom ? "\uFEFF" : "") + doc).getBytes(charset)); + LoadResult result = loader(file).load(Duration.ZERO); + String what = name + " " + charset + (bom ? " with a byte order mark" : ""); + assertEquals(what, LDValue.buildArray().add("caf\u00e9").build(), + json(FEATURES, toMap(result).get(FEATURES).get("flag")).get("variations")); + } + } + } + } + } + + @Test + public void utf16FileWithOnlyWhiteSpaceCountsAsEmpty() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + loadAndAccept(loader); + + Files.write(file, "\uFEFF \n".getBytes(StandardCharsets.UTF_16LE)); + assertHeld(loader, file); + } + } + + @Test + public void contentHashCoversPathsAndPresence() throws Exception { + // The format depends on the path, so the same bytes moved from one configured file to another + // can be different data. + try (TempDir dir = TempDir.create()) { + Path yaml = write(dir, "overrides.yaml", "{\"flagValues\": {\"flag\": on}}"); + Path json = dir.getPath().resolve("overrides.json"); + OverrideFileLoader loader = loader(yaml, json); + LoadResult before = loader.load(Duration.ZERO); + Files.move(yaml, json); + LoadResult after = loader.load(Duration.ZERO); + assertThat(Arrays.equals(before.getContentHash(), after.getContentHash()), is(false)); + assertEquals(LDValue.of(true), json(FEATURES, toMap(before).get(FEATURES).get("flag")).get("variations").get(0)); + assertEquals(LDValue.of("on"), json(FEATURES, toMap(after).get(FEATURES).get("flag")).get("variations").get(0)); + } + } + + // Loads the files and applies the result, as the reloader does, or reports that the load held a + // file back or failed. + private static boolean loadAppliesOrFails(OverrideFileLoader loader) throws Exception { + try { + loader.accepted(loader.load(RETRY)); + return true; + } catch (EmptiedFileException | FileDataException e) { + return false; + } + } + + @Test + public void loadThatHoldsAFileBackAndThenFailsDoesNotStartBoundsOver() throws Exception { + // a.yaml and b.yaml are rewritten in turn, so that every load holds one of them back, and c.json + // fails at every other load. Those loads hold a file back, so they start no bound over. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + String bDoc = "flagValues:\n other: \"value\"\n"; + Path b = write(dir, "b.yaml", bDoc); + String cGood = "{\"flagValues\":{\"third\":\"value\"}}"; + Path c = write(dir, "c.json", cGood); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b, c); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + boolean applied = false; + for (int i = 1; i <= 12 && !applied; i++) { + clock.advance(0.5); + boolean odd = i % 2 == 1; + InPlaceWrites.rewrite(odd ? a : b, odd ? YAML_DOC : bDoc, () -> { }); + InPlaceWrites.truncate(odd ? b : a); + InPlaceWrites.rewrite(c, odd ? "{bad" : cGood, () -> { }); + applied = loadAppliesOrFails(loader); + } + assertTrue("the files were held back without end", applied); + } + } + + @Test + public void fileKeptWrittenWhileEmptyIsAppliedAfterItsBoundWhileAnEarlierFileFails() throws Exception { + // c.json comes first and fails now and then, while a.yaml keeps being written while empty. The + // failing loads still read a.yaml and hold it back, so they do not start its bound over. + try (TempDir dir = TempDir.create()) { + String other = "{\"flagValues\":{\"other\":\"value\"}}"; + Path c = write(dir, "c.json", other); + Path a = write(dir, "a.yaml", YAML_DOC); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, c, a); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertHeld(loader, a); + for (int i = 1; i <= 5; i++) { + clock.advance(0.9); + writeAgain(a, i % 2 == 0 ? " " : " "); + if (i % 2 == 1) { + InPlaceWrites.rewrite(c, "{bad", () -> { }); + assertLoadFailsFor(loader, c); + } else { + InPlaceWrites.rewrite(c, other, () -> { }); + assertHeld(loader, a); + } + } + clock.advance(0.9); + writeAgain(a, " "); + InPlaceWrites.rewrite(c, other, () -> { }); + assertEquals(1, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void holdAsksForTheRetryFromTheEndOfTheLoad() throws Exception { + // The read of the file after the held one takes longer than what is left of the held file's + // delay, so the retry is due right away, after the margin. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + AtomicLong slowReadOf = new AtomicLong(); + OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(a, b), FileData.DuplicateKeysHandling.FAIL, + clock.nanos::get, path -> { + byte[] raw = Files.readAllBytes(path); + if (path.equals(b) && slowReadOf.getAndSet(0) != 0) { + clock.advance(0.1); + } + return raw; + }); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + assertEquals(1.1, assertHeld(loader, a), 1e-9); + clock.advance(0.95); + slowReadOf.set(1); + assertEquals(0.1, assertHeld(loader, a), 1e-9); + } + } + + private static void assertLoadFailsFor(OverrideFileLoader loader, Path file) throws Exception { + try { + loader.load(RETRY); + fail("expected the load to fail"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(file.toString())); + } + } + + @Test + public void anotherFilesBoundDoesNotCutShortARewrite() throws Exception { + // b.yaml keeps being written while empty, and reaches its bound while a.yaml is in the middle of an + // ordinary rewrite. a.yaml is still held back. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, a, b); + loadAndAccept(loader); + + InPlaceWrites.truncate(b); + assertHeld(loader, b); + for (int i = 1; i <= 9; i++) { + clock.advance(0.5); + writeAgain(b, " "); + if (i == 9) { + InPlaceWrites.truncate(a); + assertHeld(loader, a, b); + } else { + assertHeld(loader, b); + } + } + // b's bound is over now, but a has been empty for half a retry delay only. + clock.advance(0.5); + writeAgain(b, " "); + assertHeld(loader, a); + + clock.advance(0.25); + InPlaceWrites.rewrite(a, YAML_DOC, () -> { }); + LoadResult result = loader.load(RETRY); + assertEquals(1, result.getFlagCount()); + assertTrue(result.getFiles().get(0).hasContent()); + } + } + + @Test + public void holdAsksForTheRestOfTheDelayAfterASlowReadOfAnotherFile() throws Exception { + // a.yaml is held back, and the read of b.yaml after it takes half a retry delay. The retry comes + // when a.yaml's delay is over, counted from when the load asks for it. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + AtomicLong slowReadOf = new AtomicLong(); + OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(a, b), FileData.DuplicateKeysHandling.FAIL, + clock.nanos::get, path -> { + byte[] raw = Files.readAllBytes(path); + if (path.equals(b) && slowReadOf.getAndSet(0) != 0) { + clock.advance(0.5); + } + return raw; + }); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + slowReadOf.set(1); + assertEquals(0.6, assertHeld(loader, a), 1e-9); + } + } + + @Test + public void slowReadOfAnotherFileDoesNotShortenTheDelay() throws Exception { + // The read of b.yaml, which comes first, takes most of a retry delay. a.yaml's delay counts from + // when a.yaml was read. + try (TempDir dir = TempDir.create()) { + Path a = write(dir, "a.yaml", YAML_DOC); + Path b = write(dir, "b.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + AtomicLong slowReadOf = new AtomicLong(); + OverrideFileLoader loader = new OverrideFileLoader(Arrays.asList(b, a), FileData.DuplicateKeysHandling.FAIL, + clock.nanos::get, path -> { + byte[] raw = Files.readAllBytes(path); + if (path.equals(b) && slowReadOf.getAndSet(0) != 0) { + clock.advance(0.94); + } + return raw; + }); + loadAndAccept(loader); + + InPlaceWrites.truncate(a); + slowReadOf.set(1); + assertHeld(loader, a); + clock.advance(0.1); + assertHeld(loader, a); + } + } + + @Test + public void withoutRetriesAnEmptiedFileIsParsedRightAway() throws Exception { + try (TempDir dir = TempDir.create()) { + Path file = write(dir, "overrides.yaml", YAML_DOC); + OverrideFileLoader loader = loader(new TestClock(), file); + loader.accepted(loader.load(Duration.ZERO)); + + InPlaceWrites.truncate(file); + assertEquals(0, loader.load(Duration.ZERO).getFlagCount()); + } + } + + @Test + public void filesEmptiedTogetherAreHeldFromTheSameLoad() throws Exception { + // A load that holds back one file still reads the others, so each emptied file starts its delay + // in that load and all of them are parsed after one retry delay, not one retry delay per file. + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.yaml", YAML_DOC); + Path second = write(dir, "second.yaml", "flagValues:\n other: \"value\"\n"); + TestClock clock = new TestClock(); + OverrideFileLoader loader = loader(clock, first, second); + assertEquals(2, loadAndAccept(loader).getFlagCount()); + + InPlaceWrites.truncate(first); + InPlaceWrites.truncate(second); + assertHeld(loader, first, second); + clock.advance(1); + assertEquals(0, loader.load(RETRY).getFlagCount()); + } + } + + @Test + public void failureOfAnotherFileFailsALoadThatHoldsAFile() throws Exception { + try (TempDir dir = TempDir.create()) { + Path first = write(dir, "first.yaml", YAML_DOC); + Path second = write(dir, "second.json", JSON_DOC.replace("flag", "other")); + OverrideFileLoader loader = loader(new TestClock(), first, second); + loadAndAccept(loader); + + InPlaceWrites.truncate(first); + InPlaceWrites.rewrite(second, "{\"flagValues\"", () -> { }); + try { + loader.load(RETRY); + fail("expected exception"); + } catch (FileDataException e) { + assertThat(e.getMessage(), containsString(second + ": cannot parse JSON")); + } } } }