Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void start() {
if (t != null) t.cancel(false);
return;
}
var o = RefreshJob.runRefreshWithTimeout(self.action, self.config.timeout);
var o = RefreshJob.runRefreshWithTimeout(self.action, self.config.getTimeoutMilliseconds());
if (o != null) {
boolean changed = !o.equals(self.latestOutput);
self.latestOutput = o;
Expand All @@ -66,7 +66,7 @@ void start() {
}
},
0,
config.interval,
config.getIntervalMilliseconds(),
TimeUnit.MILLISECONDS
);

Expand All @@ -83,7 +83,7 @@ public Optional<T> getOutput() {
if (poll == null) {
log.warn("Polling hasn't started but the output is being used.");
} else if (!firstOutput.isDone()) {
return Optional.ofNullable(firstOutput.get(config.timeout, TimeUnit.MILLISECONDS));
return Optional.ofNullable(firstOutput.get(config.getTimeoutMilliseconds(), TimeUnit.MILLISECONDS));
}
} catch (Exception e) {
log.warn("Attempted to await for poll output but an exception occurred: {}", e.toString());
Expand Down Expand Up @@ -118,9 +118,9 @@ final class OnDemand<T> implements RefreshJob<T> {
@Override
public Optional<T> getOutput() {
if (!stopped) {
if (lastUpdated - System.currentTimeMillis() < config.ttl) {
if (lastUpdated - System.currentTimeMillis() < config.getTtlMilliseconds()) {
log.debug("Running refresh as current output is stale.");
var o = RefreshJob.runRefreshWithTimeout(action, config.timeout);
var o = RefreshJob.runRefreshWithTimeout(action, config.getTimeoutMilliseconds());
Comment on lines 118 to +123

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/RefreshJob.java"

printf '\n== Outline ==\n'
ast-grep outline "$FILE" --view expanded || true

printf '\n== Relevant slice ==\n'
nl -ba "$FILE" | sed -n '1,220p'

printf '\n== Search for ttl/lastUpdated/stopped usage ==\n'
rg -n "lastUpdated|ttlMilliseconds|stopped|getOutput\\(" "$FILE"

Repository: juspay/superposition

Length of output: 843


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/RefreshJob.java"

printf '\n== Relevant slice ==\n'
nl -ba "$FILE" | sed -n '100,160p'

Repository: juspay/superposition

Length of output: 220


🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/RefreshJob.java"

printf '\n== Relevant slice 108-140 ==\n'
sed -n '108,140p' "$FILE"

printf '\n== Relevant slice 80-110 ==\n'
sed -n '80,110p' "$FILE"

printf '\n== Search for lastUpdated assignment and ttl usage ==\n'
rg -n "lastUpdated\\s*=|getTtlMilliseconds\\(|ttlMilliseconds|stopped" "$FILE"

Repository: juspay/superposition

Length of output: 3330


Fix the TTL comparison
lastUpdated - System.currentTimeMillis() < config.getTtlMilliseconds() is backwards. With a non-negative TTL, this stays true after every refresh, so getOutput() always takes the refresh path and the cache never behaves as intended.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/RefreshJob.java`
around lines 118 - 123, Correct the staleness condition in RefreshJob.getOutput:
compare the elapsed time since lastUpdated (System.currentTimeMillis() -
lastUpdated) against config.getTtlMilliseconds(), so refresh runs only when the
cached output has exceeded its TTL. Preserve the existing stopped handling and
refresh flow.

if (o != null) {
boolean changed = !o.equals(output);
output = o;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.juspay.superposition.openfeature;

import dev.openfeature.sdk.Structure;
import dev.openfeature.sdk.Value;
import lombok.Builder;
import lombok.Data;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.TimeUnit;
Expand All @@ -33,7 +34,7 @@
* // Configure experimentation options
* SuperpositionProviderOptions.ExperimentationOptions expOptions =
* SuperpositionProviderOptions.ExperimentationOptions.builder()
* .refreshStrategy(RefreshStrategy.Polling.of(5000, 2000)) // 5s timeout, 2s interval
* .refreshStrategy(new RefreshStrategy.Polling(5000, 2000)) // 5s timeout, 2s interval
* .build();
*
* // Configure provider options
Expand All @@ -43,7 +44,7 @@
* .workspaceId("your-workspace-id")
* .endpoint("https://api.superposition.dev")
* .token("your-api-token")
* .refreshStrategy(RefreshStrategy.Polling.of(10000, 5000)) // 10s timeout, 5s interval
* .refreshStrategy(new RefreshStrategy.Polling(10000, 5000)) // 10s timeout, 5s interval
* .experimentationOptions(expOptions)
* .build();
*
Expand Down Expand Up @@ -95,7 +96,7 @@ public SuperpositionOpenFeatureProvider(@NonNull SuperpositionProviderOptions op
}
this.sdk = builder.build();
this.cache = new ProviderCache();
this.configTimeout = options.refreshStrategy.getTimeout();
this.configTimeout = options.refreshStrategy.getTimeoutMilliseconds();

var getConfigInput = GetConfigInput.builder()
.context(Map.of())
Expand All @@ -110,7 +111,7 @@ public SuperpositionOpenFeatureProvider(@NonNull SuperpositionProviderOptions op

if (options.experimentationOptions != null) {
this.experimentationTimeout =
options.experimentationOptions.refreshStrategy.getTimeout();
options.experimentationOptions.refreshStrategy.getTimeoutMilliseconds();
var listExpInput = ListExperimentInput.builder()
.orgId(options.orgId)
.workspaceId(options.workspaceId)
Expand Down Expand Up @@ -328,16 +329,16 @@ private Map<String, String> evaluateConfigInternal(EvaluationContext ctx) throws
throw new Exception("Experiments cache not initialized within timeout (" + experimentationTimeout + "ms).");
}
}
var ctx_ = defaultCtx.isPresent() ? ctx.merge(defaultCtx.get()) : ctx;
var ctx_ = defaultCtx.isPresent() ? defaultCtx.get().merge(ctx) : ctx;
var queryData = EvaluationArgs.Companion.buildQueryData(ctx_);
String targetingKey = ctx_.getTargetingKey();
return cache.evalConfig(queryData, MergeStrategy.MERGE, null, targetingKey);
}

private List<String> getApplicableVariantsInternal(EvaluationContext ctx) throws Exception {
EvaluationArgs args = getEvaluationArgs(ctx);
var ctx_ = defaultCtx.isPresent() ? ctx.merge(defaultCtx.get()) : ctx;
return args.getApplicableVariants(ctx_, getExperimentationArgs(ctx_));
var ctx_ = defaultCtx.isPresent() ? defaultCtx.get().merge(ctx) : ctx;
return args.getApplicableVariants(ctx_, Objects.requireNonNull(getExperimentationArgs(ctx_)));
Comment on lines 338 to +341
}

private ExperimentationArgs getExperimentationArgs(EvaluationContext ctx) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.juspay.superposition.openfeature;

import io.juspay.superposition.openfeature.options.EvaluationCacheOptions;
import io.juspay.superposition.openfeature.options.RefreshStrategy;
import lombok.Builder;
import lombok.Data;
Expand Down Expand Up @@ -52,8 +51,5 @@ public static class ExperimentationOptions {
/** Refresh strategy for experimentation data. */
@NonNull
RefreshStrategy refreshStrategy;
/** Evaluation cache options for experimentation data (optional). */
@Nullable
EvaluationCacheOptions evaluationCacheOptions;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,252 @@
package io.juspay.superposition.openfeature.data_source;

import io.juspay.superposition.openfeature.FfiUtils;
import io.juspay.superposition.openfeature.error.SuperpositionError;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardWatchEventKinds;
import java.nio.file.WatchEvent;
import java.nio.file.WatchKey;
import java.nio.file.WatchService;
import java.time.Instant;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.CopyOnWriteArrayList;

import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import uniffi.superposition_client.OperationException;
import uniffi.superposition_types.Config;

/**
* File-based implementation of SuperpositionDataSource.
*
* Reads configuration from a local JSON or TOML file, picking the parser from the file
* extension. Always returns fresh data — {@code ifModifiedSince} is accepted but ignored.
* Context and prefix filtering are applied by the core via
* {@link FfiUtils#parseConfigFileWithFilters}, so they prune contexts and overrides exactly
* the way the server would.
*
* Supports file watching via {@link java.nio.file.WatchService}. Each call to {@link #watch()}
* returns its own {@link WatchStream}; a single daemon thread fans every change to the watched
* file out to all of them.
*
* Does NOT support experiments.
*/
public class FileDataSource implements SuperpositionDataSource {

private static final Logger log = LoggerFactory.getLogger(FileDataSource.class);

private static final String JSON = "json";
private static final String TOML = "toml";

private final Path filePath;
private final String fileFormat;

private final Object watcherLock = new Object();
private final CopyOnWriteArrayList<WatchStream> subscribers = new CopyOnWriteArrayList<>();
private WatchService watchService = null;

/**
* Create a new file-based data source.
*
* @param filePath path to the configuration file; the extension must be {@code .json} or {@code .toml}
* @throws SuperpositionError if the extension is missing or unsupported
*/
public FileDataSource(Path filePath) throws SuperpositionError {
this.filePath = filePath;
this.fileFormat = formatOf(filePath);
log.debug("FileDataSource initialized for {} file: {}", fileFormat, filePath);
}

private static String formatOf(Path filePath) throws SuperpositionError {
String name = filePath.getFileName().toString();
int dot = name.lastIndexOf('.');
Comment on lines +66 to +68

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Guard against null file name for root paths.

If the filePath resolves to a root directory (e.g., /), getFileName() will return null, causing a NullPointerException when .toString() is invoked.

💚 Proposed fix
     private static String formatOf(Path filePath) throws SuperpositionError {
-        String name = filePath.getFileName().toString();
+        Path fileName = filePath.getFileName();
+        if (fileName == null) {
+            throw SuperpositionError.dataSourceError("File path must not be a root directory: " + filePath);
+        }
+        String name = fileName.toString();
         int dot = name.lastIndexOf('.');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private static String formatOf(Path filePath) throws SuperpositionError {
String name = filePath.getFileName().toString();
int dot = name.lastIndexOf('.');
private static String formatOf(Path filePath) throws SuperpositionError {
Path fileName = filePath.getFileName();
if (fileName == null) {
throw SuperpositionError.dataSourceError("File path must not be a root directory: " + filePath);
}
String name = fileName.toString();
int dot = name.lastIndexOf('.');
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/data_source/FileDataSource.java`
around lines 66 - 68, Update formatOf to handle a null result from
filePath.getFileName() before calling toString(), including the expected format
behavior for root paths while preserving the existing extension-based logic for
normal file paths.

if (dot < 0 || dot == name.length() - 1) {
throw SuperpositionError.dataSourceError(
"File path must have an extension to determine format: " + filePath);
}
String extension = name.substring(dot + 1).toLowerCase(Locale.ROOT);
return switch (extension) {
case JSON -> JSON;
case TOML -> TOML;
default -> throw SuperpositionError.dataSourceError(
"Unsupported file extension '" + extension + "'. Supported formats are 'json' and 'toml'.");
};
}

@Override
public FetchResponse<ConfigData> fetchFilteredConfig(
Optional<Map<String, String>> context,
Optional<List<String>> prefixFilter,
Optional<Instant> ifModifiedSince)
throws SuperpositionError {
if (ifModifiedSince.isPresent()) {
log.debug("FileDataSource: ignoring ifModifiedSince, always reading fresh from file");
}
Instant now = Instant.now();

String content;
try {
content = Files.readString(filePath);
} catch (IOException e) {
throw SuperpositionError.dataSourceError(
"Failed to read config file " + filePath + ": " + e.getMessage(), e);
}

Config config;
try {
config = FfiUtils.parseConfigFileWithFilters(
content, fileFormat, context.orElse(null), prefixFilter.orElse(null));
} catch (OperationException e) {
throw SuperpositionError.dataSourceError(
"Failed to parse " + fileFormat.toUpperCase(Locale.ROOT) + " config from "
+ filePath + ": " + e.getMessage(), e);
}

return FetchResponse.data(new ConfigData(config, now));
}

@Override
public FetchResponse<ExperimentData> fetchActiveExperiments(
Optional<Instant> ifModifiedSince) throws SuperpositionError {
throw SuperpositionError.dataSourceError("Experiments not supported by FileDataSource");
}

@Override
public FetchResponse<ExperimentData> fetchCandidateActiveExperiments(
Optional<Map<String, String>> context,
Optional<List<String>> prefixFilter,
Optional<Instant> ifModifiedSince) throws SuperpositionError {
throw SuperpositionError.dataSourceError("Experiments not supported by FileDataSource");
}

@Override
public FetchResponse<ExperimentData> fetchMatchingActiveExperiments(
Optional<Map<String, String>> context,
Optional<List<String>> prefixFilter,
Optional<Instant> ifModifiedSince) throws SuperpositionError {
throw SuperpositionError.dataSourceError("Experiments not supported by FileDataSource");
}

@Override
public boolean supportsExperiments() {
return false;
}

/**
* Set up file watching. Each caller gets its own {@link WatchStream}; one daemon thread
* fans OS events for the target file out to every live stream.
*/
@Override
public Optional<WatchStream> watch() throws SuperpositionError {
synchronized (watcherLock) {
WatchStream stream = new WatchStream();
subscribers.add(stream);

if (watchService != null) {
log.debug("FileDataSource: added subscriber to existing watcher for {}", filePath);
return Optional.of(stream);
}

Path dir = filePath.toAbsolutePath().getParent();
if (dir == null) {
subscribers.remove(stream);
throw SuperpositionError.dataSourceError(
"Cannot watch file with no parent directory: " + filePath);
}

WatchService service = null;
try {
service = dir.getFileSystem().newWatchService();
dir.register(
service,
StandardWatchEventKinds.ENTRY_CREATE,
StandardWatchEventKinds.ENTRY_MODIFY,
StandardWatchEventKinds.ENTRY_DELETE);
} catch (Exception e) {
closeQuietly(service);
subscribers.remove(stream);
throw SuperpositionError.dataSourceError(
"Failed to set up file watcher for " + filePath + ": " + e.getMessage(), e);
}
Comment on lines +164 to +176

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Fix resource and state leak on WatchService registration failure.

dir.register can throw multiple types of exceptions (e.g., UnsupportedOperationException, IllegalArgumentException, or IOException). The current block only handles IOException, meaning unchecked exceptions will bypass cleanup and permanently orphan the stream in subscribers.

Furthermore, if an exception is caught, the allocated WatchService is abandoned without being closed, resulting in a resource/file descriptor leak.

🛡️ Proposed fix to ensure cleanup on all failure paths
-            try {
-                watchService = dir.getFileSystem().newWatchService();
-                dir.register(
-                    watchService,
-                    StandardWatchEventKinds.ENTRY_CREATE,
-                    StandardWatchEventKinds.ENTRY_MODIFY,
-                    StandardWatchEventKinds.ENTRY_DELETE);
-            } catch (IOException e) {
-                subscribers.remove(stream);
-                watchService = null;
-                throw SuperpositionError.dataSourceError(
-                    "Failed to set up file watcher for " + filePath + ": " + e.getMessage(), e);
-            }
+            WatchService ws = null;
+            try {
+                ws = dir.getFileSystem().newWatchService();
+                dir.register(
+                    ws,
+                    StandardWatchEventKinds.ENTRY_CREATE,
+                    StandardWatchEventKinds.ENTRY_MODIFY,
+                    StandardWatchEventKinds.ENTRY_DELETE);
+                watchService = ws;
+            } catch (Exception e) {
+                if (ws != null) {
+                    try { ws.close(); } catch (IOException ignored) {}
+                }
+                subscribers.remove(stream);
+                watchService = null;
+                if (e instanceof IOException ioException) {
+                    throw SuperpositionError.dataSourceError(
+                        "Failed to set up file watcher for " + filePath + ": " + e.getMessage(), ioException);
+                }
+                throw (RuntimeException) e;
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
try {
watchService = dir.getFileSystem().newWatchService();
dir.register(
watchService,
StandardWatchEventKinds.ENTRY_CREATE,
StandardWatchEventKinds.ENTRY_MODIFY,
StandardWatchEventKinds.ENTRY_DELETE);
} catch (IOException e) {
subscribers.remove(stream);
watchService = null;
throw SuperpositionError.dataSourceError(
"Failed to set up file watcher for " + filePath + ": " + e.getMessage(), e);
}
WatchService ws = null;
try {
ws = dir.getFileSystem().newWatchService();
dir.register(
ws,
StandardWatchEventKinds.ENTRY_CREATE,
StandardWatchEventKinds.ENTRY_MODIFY,
StandardWatchEventKinds.ENTRY_DELETE);
watchService = ws;
} catch (Exception e) {
if (ws != null) {
try { ws.close(); } catch (IOException ignored) {}
}
subscribers.remove(stream);
watchService = null;
if (e instanceof IOException ioException) {
throw SuperpositionError.dataSourceError(
"Failed to set up file watcher for " + filePath + ": " + e.getMessage(), ioException);
}
throw (RuntimeException) e;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@clients/java/openfeature-provider/src/main/java/io/juspay/superposition/openfeature/data_source/FileDataSource.java`
around lines 163 - 175, Update the WatchService setup block in FileDataSource to
catch all exceptions that can be thrown by newWatchService or dir.register,
including unchecked registration failures, and remove stream from subscribers on
every failure path. Close any allocated watchService before clearing it and
rethrow the failure as the existing SuperpositionError.dataSourceError while
preserving the original cause.


// Only reachable when registration succeeded — the catch above always throws. The
// field must not be published before that: a half-registered watcher would leave
// watchService non-null with no thread running (it starts below), so the next watch()
// would take the "existing watcher" branch and hand back a stream that never fires.
watchService = service;

Thread watchThread = getWatchThread();
watchThread.setDaemon(true);
watchThread.start();

return Optional.of(stream);
}
}

private @NotNull Thread getWatchThread() {
Path watchedFileName = filePath.getFileName();
WatchService ws = watchService;

return new Thread(() -> {
log.debug("FileDataSource: watch thread started for {}", filePath);
while (!Thread.currentThread().isInterrupted()) {
WatchKey key;
try {
key = ws.take();
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
break;
} catch (java.nio.file.ClosedWatchServiceException e) {
log.debug("FileDataSource: watch service closed for {}", filePath);
break;
}

for (WatchEvent<?> event : key.pollEvents()) {
// Editors and config deploys often save via atomic rename, which surfaces
// as CREATE (or OVERFLOW) rather than MODIFY — all of them mean "re-read".
Object ctx = event.context();
if (!(ctx instanceof Path) || watchedFileName.equals(ctx)) {
log.debug("FileDataSource: change detected in {}", filePath);
subscribers.forEach(WatchStream::notifyChange);
}
}

if (!key.reset()) {
log.warn("FileDataSource: watch key no longer valid for {}", filePath);
break;
}
}
log.debug("FileDataSource: watch thread exiting for {}", filePath);
}, "superposition-file-watcher-" + filePath.getFileName());
}

@Override
public void close() {
synchronized (watcherLock) {
// Closing unblocks the watch thread with ClosedWatchServiceException.
closeQuietly(watchService);
watchService = null;
subscribers.forEach(WatchStream::close);
subscribers.clear();
}
log.debug("FileDataSource closed for {}", filePath);
}

/** Releases the watcher's file descriptor. Failing to close it is not worth failing over. */
private static void closeQuietly(WatchService service) {
if (service == null) {
return;
}
try {
service.close();
} catch (IOException e) {
log.warn("FileDataSource: error closing watch service: {}", e.getMessage());
}
}
}
Loading
Loading