Skip to content

Commit bc67f93

Browse files
authored
fix: replace insteance on panic, add tests (#315)
replace insteance on panic, add tests
1 parent 938c40a commit bc67f93

File tree

2 files changed

+119
-1
lines changed

2 files changed

+119
-1
lines changed

openfeature-provider-local/src/main/java/com/spotify/confidence/SwapWasmResolverApi.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.spotify.confidence;
22

3+
import com.dylibso.chicory.wasm.ChicoryException;
34
import com.spotify.confidence.flags.resolver.v1.MaterializationMap;
45
import com.spotify.confidence.flags.resolver.v1.ResolveWithStickyRequest;
56
import com.spotify.confidence.flags.resolver.v1.ResolveWithStickyResponse;
@@ -17,6 +18,7 @@ class SwapWasmResolverApi implements ResolverApi {
1718
private final AtomicReference<WasmResolveApi> wasmResolverApiRef = new AtomicReference<>();
1819
private final StickyResolveStrategy stickyResolveStrategy;
1920
private final WasmFlagLogger flagLogger;
21+
private final String accountId;
2022

2123
public SwapWasmResolverApi(
2224
WasmFlagLogger flagLogger,
@@ -25,6 +27,7 @@ public SwapWasmResolverApi(
2527
StickyResolveStrategy stickyResolveStrategy) {
2628
this.stickyResolveStrategy = stickyResolveStrategy;
2729
this.flagLogger = flagLogger;
30+
this.accountId = accountId;
2831

2932
// Create initial instance
3033
final WasmResolveApi initialInstance = new WasmResolveApi(flagLogger);
@@ -36,7 +39,9 @@ public SwapWasmResolverApi(
3639
public void updateStateAndFlushLogs(byte[] state, String accountId) {
3740
// Create new instance with updated state
3841
final WasmResolveApi newInstance = new WasmResolveApi(flagLogger);
39-
newInstance.setResolverState(state, accountId);
42+
if (state != null) {
43+
newInstance.setResolverState(state, accountId);
44+
}
4045

4146
// Get current instance before switching
4247
final WasmResolveApi oldInstance = wasmResolverApiRef.getAndSet(newInstance);
@@ -191,6 +196,9 @@ public ResolveFlagsResponse resolve(ResolveFlagsRequest request) {
191196
return instance.resolve(request);
192197
} catch (IsClosedException e) {
193198
return resolve(request);
199+
} catch (ChicoryException ce) {
200+
updateStateAndFlushLogs(null, accountId);
201+
throw ce;
194202
}
195203
}
196204
}
Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package com.spotify.confidence;
2+
3+
import static org.junit.jupiter.api.Assertions.assertNotNull;
4+
import static org.junit.jupiter.api.Assertions.assertThrows;
5+
import static org.mockito.ArgumentMatchers.any;
6+
import static org.mockito.Mockito.atLeastOnce;
7+
import static org.mockito.Mockito.mock;
8+
import static org.mockito.Mockito.spy;
9+
import static org.mockito.Mockito.times;
10+
import static org.mockito.Mockito.verify;
11+
import static org.mockito.Mockito.when;
12+
13+
import com.dylibso.chicory.wasm.ChicoryException;
14+
import com.spotify.confidence.shaded.flags.resolver.v1.ResolveFlagsRequest;
15+
import com.spotify.confidence.shaded.flags.resolver.v1.ResolveFlagsResponse;
16+
import java.util.concurrent.atomic.AtomicReference;
17+
import org.junit.jupiter.api.Test;
18+
19+
public class SwapWasmResolverApiTest {
20+
21+
@Test
22+
public void testChicoryExceptionTriggersStateReset() throws Exception {
23+
// Create a mock WasmFlagLogger
24+
final WasmFlagLogger mockLogger = mock(WasmFlagLogger.class);
25+
26+
// Use valid test state bytes and accountId
27+
final byte[] initialState = ResolveTest.exampleStateBytes;
28+
final String accountId = "test-account-id";
29+
30+
// Create a spy of SwapWasmResolverApi to verify method calls
31+
final SwapWasmResolverApi swapApi =
32+
spy(
33+
new SwapWasmResolverApi(
34+
mockLogger, initialState, accountId, mock(ResolverFallback.class)));
35+
36+
// Create a mock WasmResolveApi that throws ChicoryException
37+
final WasmResolveApi mockWasmApi = mock(WasmResolveApi.class);
38+
when(mockWasmApi.resolve(any(ResolveFlagsRequest.class)))
39+
.thenThrow(new ChicoryException("WASM runtime error"));
40+
41+
// Replace the internal WasmResolveApi with our mock
42+
final var field = SwapWasmResolverApi.class.getDeclaredField("wasmResolverApiRef");
43+
field.setAccessible(true);
44+
@SuppressWarnings("unchecked")
45+
final AtomicReference<WasmResolveApi> ref =
46+
(AtomicReference<WasmResolveApi>) field.get(swapApi);
47+
ref.set(mockWasmApi);
48+
49+
// Create a test resolve request
50+
final ResolveFlagsRequest request =
51+
ResolveFlagsRequest.newBuilder()
52+
.addFlags("test-flag")
53+
.setClientSecret("test-secret")
54+
.build();
55+
56+
// Execute the resolve and expect ChicoryException to be thrown
57+
assertThrows(ChicoryException.class, () -> swapApi.resolve(request));
58+
59+
// Verify that updateStateAndFlushLogs was called with null state and the accountId
60+
verify(swapApi, atLeastOnce()).updateStateAndFlushLogs(null, accountId);
61+
}
62+
63+
@Test
64+
public void testIsClosedExceptionTriggersRetry() throws Exception {
65+
// Create a mock WasmFlagLogger
66+
final WasmFlagLogger mockLogger = mock(WasmFlagLogger.class);
67+
68+
// Use valid test state bytes and accountId
69+
final byte[] initialState = ResolveTest.exampleStateBytes;
70+
final String accountId = "test-account-id";
71+
72+
// Create a spy of SwapWasmResolverApi to verify method calls
73+
final SwapWasmResolverApi swapApi =
74+
spy(
75+
new SwapWasmResolverApi(
76+
mockLogger, initialState, accountId, mock(ResolverFallback.class)));
77+
78+
// Create a mock WasmResolveApi that throws IsClosedException on first call, then succeeds
79+
final WasmResolveApi mockWasmApi = mock(WasmResolveApi.class);
80+
final ResolveFlagsResponse mockResponse = ResolveFlagsResponse.newBuilder().build();
81+
when(mockWasmApi.resolve(any(ResolveFlagsRequest.class)))
82+
.thenThrow(new IsClosedException())
83+
.thenReturn(mockResponse);
84+
85+
// Replace the internal WasmResolveApi with our mock
86+
final var field = SwapWasmResolverApi.class.getDeclaredField("wasmResolverApiRef");
87+
field.setAccessible(true);
88+
@SuppressWarnings("unchecked")
89+
final AtomicReference<WasmResolveApi> ref =
90+
(AtomicReference<WasmResolveApi>) field.get(swapApi);
91+
ref.set(mockWasmApi);
92+
93+
// Create a test resolve request
94+
final ResolveFlagsRequest request =
95+
ResolveFlagsRequest.newBuilder()
96+
.addFlags("flags/flag-1")
97+
.setClientSecret(TestBase.secret.getSecret())
98+
.build();
99+
100+
// Call resolve - it should retry when IsClosedException is thrown and succeed on second attempt
101+
final ResolveFlagsResponse response = swapApi.resolve(request);
102+
103+
// Verify response is not null
104+
assertNotNull(response);
105+
106+
// Verify that the mock WasmResolveApi.resolve was called twice (first threw exception, second
107+
// succeeded)
108+
verify(mockWasmApi, times(2)).resolve(request);
109+
}
110+
}

0 commit comments

Comments
 (0)