Skip to content

Commit 8c94340

Browse files
committed
fix(predict): use singleton to avoid race conditions
1 parent ea8019e commit 8c94340

File tree

2 files changed

+237
-62
lines changed

2 files changed

+237
-62
lines changed

app/components/UI/Predict/hooks/usePredictEligibility.test.ts

Lines changed: 88 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ import { AppState, AppStateStatus } from 'react-native';
33
import { useSelector } from 'react-redux';
44
import Engine from '../../../../core/Engine';
55
import DevLogger from '../../../../core/SDKConnect/utils/DevLogger';
6-
import { usePredictEligibility } from './usePredictEligibility';
6+
import {
7+
usePredictEligibility,
8+
getRefreshManagerForTesting,
9+
} from './usePredictEligibility';
710

811
jest.mock('react-redux', () => ({
912
useSelector: jest.fn(),
@@ -51,6 +54,10 @@ describe('usePredictEligibility', () => {
5154
jest.useFakeTimers();
5255
jest.clearAllMocks();
5356

57+
// Reset the singleton manager FIRST, before setting up mocks
58+
const manager = getRefreshManagerForTesting();
59+
manager.reset();
60+
5461
// Reset AppState mock
5562
(AppState as jest.Mocked<typeof AppState>).currentState = 'active';
5663

@@ -118,25 +125,74 @@ describe('usePredictEligibility', () => {
118125
});
119126
});
120127

121-
describe('AppState listener setup', () => {
122-
it('sets up AppState listener on mount', () => {
128+
describe('singleton manager registration', () => {
129+
it('sets up AppState listener when first hook mounts', () => {
123130
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
124131

125132
expect(mockAppStateAddEventListener).toHaveBeenCalledTimes(1);
126133
expect(mockAppStateAddEventListener).toHaveBeenCalledWith(
127134
'change',
128135
expect.any(Function),
129136
);
137+
expect(mockDevLogger).toHaveBeenCalledWith(
138+
'PredictController: Starting eligibility refresh manager',
139+
expect.objectContaining({
140+
activeListeners: 1,
141+
}),
142+
);
130143
});
131144

132-
it('removes AppState listener on unmount', () => {
145+
it('does not create additional listeners when second hook mounts', () => {
146+
const { unmount: unmount1 } = renderHook(() =>
147+
usePredictEligibility({ providerId: 'polymarket' }),
148+
);
149+
150+
jest.clearAllMocks();
151+
152+
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
153+
154+
expect(mockAppStateAddEventListener).not.toHaveBeenCalled();
155+
expect(mockDevLogger).toHaveBeenCalledWith(
156+
'PredictController: Additional listener registered',
157+
expect.objectContaining({
158+
activeListeners: 2,
159+
}),
160+
);
161+
162+
unmount1();
163+
});
164+
165+
it('removes AppState listener when last hook unmounts', () => {
133166
const { unmount } = renderHook(() =>
134167
usePredictEligibility({ providerId: 'polymarket' }),
135168
);
136169

137170
unmount();
138171

139172
expect(mockSubscriptionRemove).toHaveBeenCalledTimes(1);
173+
expect(mockDevLogger).toHaveBeenCalledWith(
174+
'PredictController: Stopping eligibility refresh manager',
175+
);
176+
});
177+
178+
it('keeps listener active when one of multiple hooks unmounts', () => {
179+
const { unmount: unmount1 } = renderHook(() =>
180+
usePredictEligibility({ providerId: 'polymarket' }),
181+
);
182+
183+
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
184+
185+
jest.clearAllMocks();
186+
187+
unmount1();
188+
189+
expect(mockSubscriptionRemove).not.toHaveBeenCalled();
190+
expect(mockDevLogger).toHaveBeenCalledWith(
191+
'PredictController: Listener unregistered',
192+
expect.objectContaining({
193+
activeListeners: 1,
194+
}),
195+
);
140196
});
141197
});
142198

@@ -156,7 +212,7 @@ describe('usePredictEligibility', () => {
156212
expect(mockDevLogger).toHaveBeenCalledWith(
157213
'PredictController: App became active, refreshing eligibility',
158214
expect.objectContaining({
159-
providerId: 'polymarket',
215+
previousState: 'background',
160216
}),
161217
);
162218
});
@@ -243,7 +299,6 @@ describe('usePredictEligibility', () => {
243299
expect(mockDevLogger).toHaveBeenCalledWith(
244300
'PredictController: Skipping refresh due to debounce',
245301
expect.objectContaining({
246-
providerId: 'polymarket',
247302
timeSinceLastRefresh: expect.any(Number),
248303
minimumInterval: 60000,
249304
}),
@@ -409,7 +464,6 @@ describe('usePredictEligibility', () => {
409464
expect(mockDevLogger).toHaveBeenCalledWith(
410465
'PredictController: Auto-refresh failed',
411466
expect.objectContaining({
412-
providerId: 'polymarket',
413467
error: 'Network error',
414468
}),
415469
);
@@ -431,7 +485,6 @@ describe('usePredictEligibility', () => {
431485
expect(mockDevLogger).toHaveBeenCalledWith(
432486
'PredictController: Auto-refresh failed',
433487
expect.objectContaining({
434-
providerId: 'polymarket',
435488
error: 'Unknown',
436489
}),
437490
);
@@ -492,9 +545,6 @@ describe('usePredictEligibility', () => {
492545
expect(mockRefreshEligibility).toHaveBeenCalledTimes(1);
493546
expect(mockDevLogger).toHaveBeenCalledWith(
494547
'PredictController: Refresh already in progress, reusing promise',
495-
expect.objectContaining({
496-
providerId: 'polymarket',
497-
}),
498548
);
499549

500550
resolveRefresh?.();
@@ -529,6 +579,33 @@ describe('usePredictEligibility', () => {
529579
});
530580
});
531581

582+
it('prevents concurrent calls from multiple hook instances', async () => {
583+
let resolveRefresh: (() => void) | undefined;
584+
const refreshPromise = new Promise<void>((resolve) => {
585+
resolveRefresh = resolve;
586+
});
587+
mockRefreshEligibility.mockReturnValueOnce(refreshPromise);
588+
589+
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
590+
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
591+
renderHook(() => usePredictEligibility({ providerId: 'polymarket' }));
592+
593+
const handleAppStateChange = mockAppStateAddEventListener.mock
594+
.calls[0][1] as (nextState: AppStateStatus) => void;
595+
596+
await act(async () => {
597+
handleAppStateChange('background');
598+
handleAppStateChange('active');
599+
});
600+
601+
expect(mockRefreshEligibility).toHaveBeenCalledTimes(1);
602+
603+
resolveRefresh?.();
604+
await act(async () => {
605+
await refreshPromise;
606+
});
607+
});
608+
532609
it('allows new refresh after previous one completes', async () => {
533610
let resolveFirstRefresh: (() => void) | undefined;
534611
const firstRefreshPromise = new Promise<void>((resolve) => {

0 commit comments

Comments
 (0)