Skip to content

Commit 727fc04

Browse files
aepfliclaude
andcommitted
fix(flagd): address code review feedback for WASM resolver
- Load WASM from classpath instead of hardcoded path - Use static SecureRandom for efficiency - Fix JSON exception handling to not terminate stateWatcher thread - Implement shutdown() method properly - Change generic Exception to GeneralError - Fix ImmutableMetadataDeserializer to preserve value types - Move OBJECT_MAPPER config to static block - Make stateWatcher a class field for proper shutdown - Remove dead code (getFlagMetadata, FLAGD_SELECTOR_KEY) - Extract serializers/deserializers to jackson package 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 3ccb8e0 commit 727fc04

7 files changed

Lines changed: 124 additions & 118 deletions

File tree

providers/flagd/WASM_HOST_IMPORTS.md

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ private static HostFunction createGetCurrentTimeUnixSeconds() {
7676

7777
**Implementation:**
7878
```java
79+
// Static SecureRandom instance for efficiency (reused across calls)
80+
private static final SecureRandom SECURE_RANDOM = new SecureRandom();
81+
7982
private static HostFunction createGetRandomValues() {
8083
return new HostFunction(
8184
"__wbindgen_placeholder__",
@@ -93,7 +96,7 @@ private static HostFunction createGetRandomValues() {
9396
// The WASM code expects a 32-byte buffer at bufferPtr
9497
// Fill it with cryptographically secure random bytes
9598
byte[] randomBytes = new byte[32];
96-
new java.security.SecureRandom().nextBytes(randomBytes);
99+
SECURE_RANDOM.nextBytes(randomBytes);
97100

98101
Memory memory = instance.memory();
99102
memory.write(bufferPtr, randomBytes);
-2.66 KB
Binary file not shown.

providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/FlagdWasmRuntime.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
import com.dylibso.chicory.wasm.types.FunctionType;
1313
import com.dylibso.chicory.wasm.types.ValType;
1414
import java.io.IOException;
15-
import java.nio.file.Files;
16-
import java.nio.file.Path;
15+
import java.io.InputStream;
16+
import java.security.SecureRandom;
1717
import java.util.List;
1818
import java.util.function.Function;
1919

@@ -29,11 +29,16 @@ public final class FlagdWasmRuntime {
2929

3030
private static final WasmModule WASM_MODULE;
3131
private static final Function<Instance, Machine> MACHINE_FUNCTION;
32+
private static final SecureRandom SECURE_RANDOM = new SecureRandom();
3233

3334
static {
3435
byte[] wasmBytes;
35-
try {
36-
wasmBytes = Files.readAllBytes(Path.of("flagd_evaluator.wasm"));
36+
try (InputStream wasmStream =
37+
FlagdWasmRuntime.class.getResourceAsStream("/flagd_evaluator.wasm")) {
38+
if (wasmStream == null) {
39+
throw new IOException("WASM resource 'flagd_evaluator.wasm' not found in classpath");
40+
}
41+
wasmBytes = wasmStream.readAllBytes();
3742
} catch (IOException e) {
3843
throw new RuntimeException("Failed to load flagd_evaluator.wasm", e);
3944
}
@@ -148,7 +153,7 @@ private static HostFunction createGetRandomValues() {
148153
// The WASM code expects a 32-byte buffer at bufferPtr
149154
// Fill it with cryptographically secure random bytes
150155
byte[] randomBytes = new byte[32];
151-
new java.security.SecureRandom().nextBytes(randomBytes);
156+
SECURE_RANDOM.nextBytes(randomBytes);
152157

153158
Memory memory = instance.memory();
154159
memory.write(bufferPtr, randomBytes);

providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessWasmResolver.java

Lines changed: 27 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,16 @@
1111
import com.dylibso.chicory.runtime.Memory;
1212
import com.dylibso.chicory.wasm.types.FunctionType;
1313
import com.dylibso.chicory.wasm.types.ValType;
14-
import com.fasterxml.jackson.core.JsonGenerator;
1514
import com.fasterxml.jackson.core.JsonProcessingException;
1615
import com.fasterxml.jackson.databind.JavaType;
17-
import com.fasterxml.jackson.databind.JsonDeserializer;
18-
import com.fasterxml.jackson.databind.JsonMappingException;
19-
import com.fasterxml.jackson.databind.JsonSerializer;
2016
import com.fasterxml.jackson.databind.ObjectMapper;
21-
import com.fasterxml.jackson.databind.SerializerProvider;
2217
import com.fasterxml.jackson.databind.module.SimpleModule;
2318
import com.google.protobuf.Struct;
2419
import dev.openfeature.contrib.providers.flagd.FlagdOptions;
2520
import dev.openfeature.contrib.providers.flagd.resolver.Resolver;
2621
import dev.openfeature.contrib.providers.flagd.resolver.common.FlagdProviderEvent;
27-
import dev.openfeature.contrib.providers.flagd.resolver.process.model.FeatureFlag;
28-
import dev.openfeature.contrib.providers.flagd.resolver.process.storage.StorageQueryResult;
22+
import dev.openfeature.contrib.providers.flagd.resolver.process.jackson.ImmutableMetadataDeserializer;
23+
import dev.openfeature.contrib.providers.flagd.resolver.process.jackson.LayeredEvalContextSerializer;
2924
import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.QueuePayload;
3025
import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.QueueSource;
3126
import dev.openfeature.contrib.providers.flagd.resolver.process.storage.connector.file.FileQueueSource;
@@ -38,8 +33,7 @@
3833
import dev.openfeature.sdk.ProviderEvent;
3934
import dev.openfeature.sdk.Structure;
4035
import dev.openfeature.sdk.Value;
41-
import java.io.IOException;
42-
import java.util.List;
36+
import dev.openfeature.sdk.exceptions.GeneralError;
4337
import java.util.Map;
4438
import java.util.function.Consumer;
4539
import lombok.extern.slf4j.Slf4j;
@@ -54,9 +48,8 @@ public class InProcessWasmResolver implements Resolver {
5448

5549
private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
5650

57-
{
58-
59-
// Register this module with your ObjectMapper
51+
static {
52+
// Register custom serializers/deserializers with the ObjectMapper
6053
SimpleModule module = new SimpleModule();
6154
module.addDeserializer(ImmutableMetadata.class, new ImmutableMetadataDeserializer());
6255
module.addSerializer(LayeredEvaluationContext.class, new LayeredEvalContextSerializer());
@@ -66,6 +59,7 @@ public class InProcessWasmResolver implements Resolver {
6659
private final Consumer<FlagdProviderEvent> onConnectionEvent;
6760
private final String scope;
6861
private final QueueSource connector;
62+
private Thread stateWatcher;
6963
private final ExportFunction validationMode;
7064
private ExportFunction updateStore;
7165
private ExportFunction alloc;
@@ -109,14 +103,13 @@ public InProcessWasmResolver(FlagdOptions options, Consumer<FlagdProviderEvent>
109103
public void init() throws Exception {
110104

111105
connector.init();
112-
final Thread stateWatcher = new Thread(() -> {
113-
try {
114-
var streamPayloads = connector.getStreamQueue();
115-
while (true) {
106+
this.stateWatcher = new Thread(() -> {
107+
var streamPayloads = connector.getStreamQueue();
108+
while (!Thread.currentThread().isInterrupted()) {
109+
try {
116110
final QueuePayload payload = streamPayloads.take();
117111
switch (payload.getType()) {
118112
case DATA:
119-
List<String> changedFlagsKeys;
120113
var data = payload.getFlagData().getBytes();
121114
long dataPtr = alloc.apply(data.length)[0];
122115
memory.write((int) dataPtr, data);
@@ -146,26 +139,30 @@ public void init() throws Exception {
146139
default:
147140
log.warn(String.format("Payload with unknown type: %s", payload.getType()));
148141
}
142+
} catch (InterruptedException e) {
143+
log.debug("Storage state watcher interrupted", e);
144+
Thread.currentThread().interrupt();
145+
break;
146+
} catch (JsonProcessingException e) {
147+
log.error("Error processing flag data, skipping update", e);
149148
}
150-
} catch (InterruptedException e) {
151-
log.warn("Storage state watcher interrupted", e);
152-
Thread.currentThread().interrupt();
153-
} catch (JsonMappingException e) {
154-
throw new RuntimeException(e);
155-
} catch (JsonProcessingException e) {
156-
throw new RuntimeException(e);
157149
}
158150
});
159-
stateWatcher.setDaemon(true);
160-
stateWatcher.start();
151+
this.stateWatcher.setDaemon(true);
152+
this.stateWatcher.start();
161153
}
162154

163155
/**
164156
* Shutdown in-process resolver.
165157
*
166158
* @throws InterruptedException if stream can't be closed within deadline.
167159
*/
168-
public void shutdown() throws InterruptedException {}
160+
public void shutdown() throws InterruptedException {
161+
if (stateWatcher != null) {
162+
stateWatcher.interrupt();
163+
}
164+
connector.shutdown();
165+
}
169166

170167
/**
171168
* Resolve a boolean flag.
@@ -253,62 +250,16 @@ private <T> ProviderEvaluation<T> resolve(Class<T> type, String key, EvaluationC
253250
ProviderEvaluation<T> providerEvaluation = OBJECT_MAPPER.readValue(result, javaType);
254251

255252
return providerEvaluation;
253+
} catch (JsonProcessingException e) {
254+
throw new GeneralError("Error deserializing WASM evaluation result", e);
256255
} catch (Exception e) {
257-
throw new RuntimeException(e);
256+
throw new GeneralError("Error during WASM evaluation", e);
258257
} finally {
259258
dealloc.apply(flagPtr, flagBytes.length);
260259
dealloc.apply(ctxPtr, ctxBytes.length);
261260
}
262261
}
263262

264-
private ImmutableMetadata getFlagMetadata(StorageQueryResult storageQueryResult) {
265-
ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder = ImmutableMetadata.builder();
266-
for (Map.Entry<String, Object> entry :
267-
storageQueryResult.getFlagSetMetadata().entrySet()) {
268-
addEntryToMetadataBuilder(metadataBuilder, entry.getKey(), entry.getValue());
269-
}
270-
271-
if (scope != null) {
272-
metadataBuilder.addString("scope", scope);
273-
}
274-
275-
FeatureFlag flag = storageQueryResult.getFeatureFlag();
276-
if (flag != null) {
277-
for (Map.Entry<String, Object> entry : flag.getMetadata().entrySet()) {
278-
addEntryToMetadataBuilder(metadataBuilder, entry.getKey(), entry.getValue());
279-
}
280-
}
281-
282-
return metadataBuilder.build();
283-
}
284-
285-
private void addEntryToMetadataBuilder(
286-
ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, String key, Object value) {
287-
if (value instanceof Number) {
288-
if (value instanceof Long) {
289-
metadataBuilder.addLong(key, (Long) value);
290-
return;
291-
} else if (value instanceof Double) {
292-
metadataBuilder.addDouble(key, (Double) value);
293-
return;
294-
} else if (value instanceof Integer) {
295-
metadataBuilder.addInteger(key, (Integer) value);
296-
return;
297-
} else if (value instanceof Float) {
298-
metadataBuilder.addFloat(key, (Float) value);
299-
return;
300-
}
301-
} else if (value instanceof Boolean) {
302-
metadataBuilder.addBoolean(key, (Boolean) value);
303-
return;
304-
} else if (value instanceof String) {
305-
metadataBuilder.addString(key, (String) value);
306-
return;
307-
}
308-
throw new IllegalArgumentException(
309-
"The type of the Metadata entry with key " + key + " and value " + value + " is not supported");
310-
}
311-
312263
private Structure parseSyncContext(Struct syncContext) {
313264
if (syncContext != null) {
314265
try {
@@ -319,36 +270,4 @@ private Structure parseSyncContext(Struct syncContext) {
319270
}
320271
return new ImmutableStructure();
321272
}
322-
323-
// Implement a custom deserializer for ImmutableMetadata
324-
public class ImmutableMetadataDeserializer extends JsonDeserializer<ImmutableMetadata> {
325-
@Override
326-
public ImmutableMetadata deserialize(
327-
com.fasterxml.jackson.core.JsonParser p, com.fasterxml.jackson.databind.DeserializationContext ctxt)
328-
throws IOException {
329-
// Deserialize into a Map or DTO, then use the builder
330-
Map<String, Object> map = p.readValueAs(Map.class);
331-
ImmutableMetadata.ImmutableMetadataBuilder builder = ImmutableMetadata.builder();
332-
for (Map.Entry<String, Object> entry : map.entrySet()) {
333-
builder.addString(entry.getKey(), entry.getValue().toString());
334-
}
335-
return builder.build();
336-
}
337-
}
338-
339-
public class LayeredEvalContextSerializer extends JsonSerializer<LayeredEvaluationContext> {
340-
@Override
341-
public void serialize(LayeredEvaluationContext ctx, JsonGenerator gen, SerializerProvider serializers)
342-
throws IOException {
343-
gen.writeStartObject();
344-
345-
// Use the keySet and getValue to stream the entries
346-
for (String key : ctx.keySet()) {
347-
Object value = ctx.getValue(key);
348-
gen.writeObjectField(key, value);
349-
}
350-
351-
gen.writeEndObject();
352-
}
353-
}
354273
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package dev.openfeature.contrib.providers.flagd.resolver.process.jackson;
2+
3+
import com.fasterxml.jackson.databind.JsonDeserializer;
4+
import dev.openfeature.sdk.ImmutableMetadata;
5+
import java.io.IOException;
6+
import java.util.Map;
7+
8+
/**
9+
* Custom deserializer for ImmutableMetadata that preserves value types.
10+
*
11+
* <p>This deserializer properly handles different value types (Boolean, String, Integer, Long,
12+
* Double, Float) instead of converting everything to strings.
13+
*/
14+
public class ImmutableMetadataDeserializer extends JsonDeserializer<ImmutableMetadata> {
15+
16+
@Override
17+
public ImmutableMetadata deserialize(
18+
com.fasterxml.jackson.core.JsonParser p, com.fasterxml.jackson.databind.DeserializationContext ctxt)
19+
throws IOException {
20+
@SuppressWarnings("unchecked")
21+
Map<String, Object> map = p.readValueAs(Map.class);
22+
ImmutableMetadata.ImmutableMetadataBuilder builder = ImmutableMetadata.builder();
23+
for (Map.Entry<String, Object> entry : map.entrySet()) {
24+
addEntryToMetadataBuilder(builder, entry.getKey(), entry.getValue());
25+
}
26+
return builder.build();
27+
}
28+
29+
private static void addEntryToMetadataBuilder(
30+
ImmutableMetadata.ImmutableMetadataBuilder metadataBuilder, String key, Object value) {
31+
if (value instanceof Number) {
32+
if (value instanceof Long) {
33+
metadataBuilder.addLong(key, (Long) value);
34+
} else if (value instanceof Double) {
35+
metadataBuilder.addDouble(key, (Double) value);
36+
} else if (value instanceof Integer) {
37+
metadataBuilder.addInteger(key, (Integer) value);
38+
} else if (value instanceof Float) {
39+
metadataBuilder.addFloat(key, (Float) value);
40+
} else {
41+
// Fallback for other Number types
42+
metadataBuilder.addDouble(key, ((Number) value).doubleValue());
43+
}
44+
} else if (value instanceof Boolean) {
45+
metadataBuilder.addBoolean(key, (Boolean) value);
46+
} else if (value instanceof String) {
47+
metadataBuilder.addString(key, (String) value);
48+
} else if (value != null) {
49+
// Fallback to string for unknown types
50+
metadataBuilder.addString(key, value.toString());
51+
}
52+
}
53+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package dev.openfeature.contrib.providers.flagd.resolver.process.jackson;
2+
3+
import com.fasterxml.jackson.core.JsonGenerator;
4+
import com.fasterxml.jackson.databind.JsonSerializer;
5+
import com.fasterxml.jackson.databind.SerializerProvider;
6+
import dev.openfeature.sdk.LayeredEvaluationContext;
7+
import java.io.IOException;
8+
9+
/**
10+
* Custom serializer for LayeredEvaluationContext.
11+
*
12+
* <p>This serializer iterates through the context's keys and writes each key-value pair
13+
* to the JSON output.
14+
*/
15+
public class LayeredEvalContextSerializer extends JsonSerializer<LayeredEvaluationContext> {
16+
17+
@Override
18+
public void serialize(LayeredEvaluationContext ctx, JsonGenerator gen, SerializerProvider serializers)
19+
throws IOException {
20+
gen.writeStartObject();
21+
22+
// Use the keySet and getValue to stream the entries
23+
for (String key : ctx.keySet()) {
24+
Object value = ctx.getValue(key);
25+
gen.writeObjectField(key, value);
26+
}
27+
28+
gen.writeEndObject();
29+
}
30+
}

providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/resolver/process/storage/connector/sync/SyncStreamQueueSource.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import dev.openfeature.flagd.grpc.sync.Sync.SyncFlagsResponse;
1818
import dev.openfeature.sdk.Awaitable;
1919
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
20-
import io.grpc.Metadata;
2120
import io.grpc.Status;
2221
import io.grpc.StatusRuntimeException;
2322
import io.grpc.stub.StreamObserver;
@@ -38,9 +37,6 @@
3837
public class SyncStreamQueueSource implements QueueSource {
3938
private static final int QUEUE_SIZE = 5;
4039

41-
private static final Metadata.Key<String> FLAGD_SELECTOR_KEY =
42-
Metadata.Key.of("Flagd-Selector", Metadata.ASCII_STRING_MARSHALLER);
43-
4440
private final AtomicBoolean shutdown = new AtomicBoolean(false);
4541
private final AtomicBoolean shouldThrottle = new AtomicBoolean(false);
4642
private final int streamDeadline;

0 commit comments

Comments
 (0)