[No QA] Fix import/no-cycle - part 1

.storybook/preview.tsx

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import EnvironmentProvider from '@components/EnvironmentContextProvider';
 import OnyxListItemProvider from '@components/OnyxListItemProvider';
 import ScreenWrapperStatusContext from '@components/ScreenWrapper/ScreenWrapperStatusContext';

src/libs/API/makeRequest.ts

@@ -1,23 +1,8 @@
 import Log from '@libs/Log';
-import {
-    FailureTracking,
-    handleDeletedAccount,
-    HandleUnusedOptimisticID,
-    LoadTest,
-    Logging,
-    Pagination,
-    Reauthentication,
-    RecordFullReconnectTime,
-    SaveResponseInOnyx,
-    SupportalPermission,
-} from '@libs/Middleware';
-import FraudMonitoring from '@libs/Middleware/FraudMonitoring';
-import LoadPostDataForOpenOrReconnect from '@libs/Middleware/LoadPostDataForOpenOrReconnect';
-import SentryServerTiming from '@libs/Middleware/SentryServerTiming';
 import {push as pushToSequentialQueue} from '@libs/Network/SequentialQueue';
 import {getIsOffline} from '@libs/NetworkState';
 import Pusher from '@libs/Pusher';
-import {addMiddleware, processWithMiddleware} from '@libs/Request';
+import {processWithMiddleware} from '@libs/Request';
 import sanitizeLogParams from '@libs/sanitizeLogParams';
 
 import {getAll} from '@userActions/PersistedRequests';
@@ -34,50 +19,6 @@ import Onyx from 'react-native-onyx';
 
 import type {ApiCommand, ApiRequestCommandParameters, ApiRequestType} from './types';
 
-// Setup API middlewares. Each request made will pass through a series of middleware functions that will get called in sequence (each one passing the result of the previous to the next).
-// Note: The ordering here is intentional as we want to Log, Recheck Connection, Reauthenticate, and Save the Response in Onyx. Errors thrown in one middleware will bubble to the next.
-// e.g. an error thrown in Logging or Reauthenticate logic will be caught by the next middleware or the SequentialQueue which retries failing requests.
-// This lives alongside prepareRequest/processRequest (rather than in index.ts) so registration happens for any entry point that can process a request, not just the barrel.
-
-// Logging - Logs request details and errors.
-addMiddleware(Logging);
-
-// Duplicates API calls (tagged with mockRequest=true) when the server sends load-test parameters via the X-Load-Test response header.
-addMiddleware(LoadTest);
-
-// FailureTracking - Observes request outcomes and feeds them to FailureTracker for sustained failure detection.
-addMiddleware(FailureTracking);
-
-// Reauthentication - Handles jsonCode 407 which indicates an expired authToken. We need to reauthenticate and get a new authToken with our stored credentials.
-addMiddleware(Reauthentication);
-
-// Handles the case when the copilot has been deleted. The response contains jsonCode 408 and a message indicating account deletion
-addMiddleware(handleDeletedAccount);
-
-// Handle supportal permission denial centrally
-addMiddleware(SupportalPermission);
-
-// If an optimistic ID is not used by the server, this will update the remaining serialized requests using that optimistic ID to use the correct ID instead.
-addMiddleware(HandleUnusedOptimisticID);
-
-addMiddleware(Pagination);
-
-// SentryServerTiming - Tracks server round-trip time for configured command groups via Sentry spans.
-addMiddleware(SentryServerTiming);
-
-// RecordFullReconnectTime - Records the full-reconnect time into an OpenApp/full-ReconnectApp response. Must run before SaveResponseInOnyx applies the response.
-addMiddleware(RecordFullReconnectTime);
-
-// LoadPostDataForOpenOrReconnect - Sends the reads that OpenApp/ReconnectApp does not return, once per response that reaches the server.
-addMiddleware(LoadPostDataForOpenOrReconnect);
-
-// SaveResponseInOnyx - Merges either the successData or failureData (or finallyData, if included in place of the former two values) into Onyx depending on if the call was successful or not. This must be the last middleware that applies Onyx data
-// (middlewares after it, like FraudMonitoring, must not write Onyx), because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.
-addMiddleware(SaveResponseInOnyx);
-
-// FraudMonitoring - Tags the request with the appropriate Fraud Protection event.
-addMiddleware(FraudMonitoring);
-
 // Use timestamp-based IDs to avoid collisions between browser tabs.
 // Each tab has its own JS context with its own counter, so a simple
 // incrementing number would collide across tabs.

src/libs/Middleware/index.ts

@@ -1,12 +1,29 @@
 import FailureTracking from './FailureTracking';
+import FraudMonitoring from './FraudMonitoring';
 import handleDeletedAccount from './HandleDeletedAccount';
 import HandleUnusedOptimisticID from './HandleUnusedOptimisticID';
+import LoadPostDataForOpenOrReconnect from './LoadPostDataForOpenOrReconnect';
 import LoadTest from './LoadTest';
 import Logging from './Logging';
 import {Pagination} from './Pagination';
 import Reauthentication from './Reauthentication';
 import RecordFullReconnectTime from './RecordFullReconnectTime';
 import SaveResponseInOnyx from './SaveResponseInOnyx';
+import SentryServerTiming from './SentryServerTiming';
 import SupportalPermission from './SupportalPermission';
 
-export {HandleUnusedOptimisticID, LoadTest, Logging, Reauthentication, RecordFullReconnectTime, FailureTracking, SaveResponseInOnyx, Pagination, handleDeletedAccount, SupportalPermission};
+export {
+    HandleUnusedOptimisticID,
+    LoadTest,
+    Logging,
+    Reauthentication,
+    RecordFullReconnectTime,
+    FailureTracking,
+    SaveResponseInOnyx,
+    Pagination,
+    handleDeletedAccount,
+    SupportalPermission,
+    FraudMonitoring,
+    LoadPostDataForOpenOrReconnect,
+    SentryServerTiming,
+};

src/libs/Middleware/register.ts

@@ -0,0 +1,66 @@
+import {addMiddleware} from '@libs/Request';
+
+import {
+    FailureTracking,
+    FraudMonitoring,
+    handleDeletedAccount,
+    HandleUnusedOptimisticID,
+    LoadPostDataForOpenOrReconnect,
+    LoadTest,
+    Logging,
+    Pagination,
+    Reauthentication,
+    RecordFullReconnectTime,
+    SaveResponseInOnyx,
+    SentryServerTiming,
+    SupportalPermission,
+} from './index';
+
+// Setup API middlewares. Each request made will pass through a series of middleware functions that will get called in sequence (each one passing the result of the previous to the next).
+// Note: The ordering here is intentional as we want to Log, Recheck Connection, Reauthenticate, and Save the Response in Onyx. Errors thrown in one middleware will bubble to the next.
+// e.g. an error thrown in Logging or Reauthenticate logic will be caught by the next middleware or the SequentialQueue which retries failing requests.
+//
+// This lives here rather than in libs/API because six of the middlewares below import user actions, and every
+// user action imports libs/API. Registering from inside libs/API therefore closes an import cycle: libs/API
+// imports Middleware, Middleware imports an action, and that action imports libs/API again. Instead the
+// composition root imports this module for its side effect, before anything can call processWithMiddleware:
+// see src/setup/index.ts.
+
+// Logging - Logs request details and errors.
+addMiddleware(Logging);
+
+// Duplicates API calls (tagged with mockRequest=true) when the server sends load-test parameters via the X-Load-Test response header.
+addMiddleware(LoadTest);
+
+// FailureTracking - Observes request outcomes and feeds them to FailureTracker for sustained failure detection.
+addMiddleware(FailureTracking);
+
+// Reauthentication - Handles jsonCode 407 which indicates an expired authToken. We need to reauthenticate and get a new authToken with our stored credentials.
+addMiddleware(Reauthentication);
+
+// Handles the case when the copilot has been deleted. The response contains jsonCode 408 and a message indicating account deletion
+addMiddleware(handleDeletedAccount);
+
+// Handle supportal permission denial centrally
+addMiddleware(SupportalPermission);
+
+// If an optimistic ID is not used by the server, this will update the remaining serialized requests using that optimistic ID to use the correct ID instead.
+addMiddleware(HandleUnusedOptimisticID);
+
+addMiddleware(Pagination);
+
+// SentryServerTiming - Tracks server round-trip time for configured command groups via Sentry spans.
+addMiddleware(SentryServerTiming);
+
+// RecordFullReconnectTime - Records the full-reconnect time into an OpenApp/full-ReconnectApp response. Must run before SaveResponseInOnyx applies the response.
+addMiddleware(RecordFullReconnectTime);
+
+// LoadPostDataForOpenOrReconnect - Sends the reads that OpenApp/ReconnectApp does not return, once per response that reaches the server.
+addMiddleware(LoadPostDataForOpenOrReconnect);
+
+// SaveResponseInOnyx - Merges either the successData or failureData (or finallyData, if included in place of the former two values) into Onyx depending on if the call was successful or not. This must be the last middleware that applies Onyx data
+// (middlewares after it, like FraudMonitoring, must not write Onyx), because the SequentialQueue depends on the result of this middleware to pause the queue (if needed) to bring the app to an up-to-date state.
+addMiddleware(SaveResponseInOnyx);
+
+// FraudMonitoring - Tags the request with the appropriate Fraud Protection event.
+addMiddleware(FraudMonitoring);

src/setup/index.ts

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import {finishCloudflareSignInFromURL} from '@libs/CloudflareAccess/finishSignInFromURL';
 import intlPolyfill from '@libs/IntlPolyfill';
 

tests/ui/SearchPageTest.tsx

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import {act, render, screen} from '@testing-library/react-native';
 
 import ComposeProviders from '@components/ComposeProviders';

tests/unit/MiddlewareEntryPointTest.ts

@@ -0,0 +1,17 @@
+import type * as RequestModule from '@libs/Request';
+import {addMiddleware} from '@libs/Request';
+
+jest.mock('@libs/Request', () => ({
+    ...jest.requireActual<typeof RequestModule>('@libs/Request'),
+    addMiddleware: jest.fn(),
+}));
+
+describe('src/setup attaches the API middlewares', () => {
+    it('registers all 13 middlewares at module scope when the composition root loads', () => {
+        expect(jest.mocked(addMiddleware)).not.toHaveBeenCalled();
+
+        require('@src/setup');
+
+        expect(jest.mocked(addMiddleware)).toHaveBeenCalledTimes(13);
+    });
+});

tests/unit/MiddlewareRegistrationTest.ts

@@ -0,0 +1,67 @@
+import {
+    FailureTracking,
+    FraudMonitoring,
+    handleDeletedAccount,
+    HandleUnusedOptimisticID,
+    LoadPostDataForOpenOrReconnect,
+    LoadTest,
+    Logging,
+    Pagination,
+    Reauthentication,
+    RecordFullReconnectTime,
+    SaveResponseInOnyx,
+    SentryServerTiming,
+    SupportalPermission,
+} from '@libs/Middleware';
+import type * as RequestModule from '@libs/Request';
+import {addMiddleware} from '@libs/Request';
+
+jest.mock('@libs/Request', () => ({
+    ...jest.requireActual<typeof RequestModule>('@libs/Request'),
+    addMiddleware: jest.fn(),
+}));
+
+const EXPECTED_ORDER: RequestModule.Middleware[] = [
+    Logging,
+    LoadTest,
+    FailureTracking,
+    Reauthentication,
+    handleDeletedAccount,
+    SupportalPermission,
+    HandleUnusedOptimisticID,
+    Pagination,
+    SentryServerTiming,
+    RecordFullReconnectTime,
+    LoadPostDataForOpenOrReconnect,
+    SaveResponseInOnyx,
+    FraudMonitoring,
+];
+
+describe('Middleware registration', () => {
+    let registered: RequestModule.Middleware[] = [];
+
+    beforeAll(() => {
+        // jest.isolateModules would give register.ts its own module registry, so the middlewares it resolves
+        // would be distinct function objects from the ones imported above and every identity check would fail.
+        require('@libs/Middleware/register');
+        registered = jest.mocked(addMiddleware).mock.calls.map(([middleware]) => middleware);
+    });
+
+    it('registers every middleware exactly once, in the documented order', () => {
+        expect(registered).toEqual(EXPECTED_ORDER);
+    });
+
+    it('registers all 13 middlewares with no duplicates', () => {
+        expect(registered).toHaveLength(13);
+        expect(new Set(registered).size).toBe(13);
+    });
+
+    it('keeps SaveResponseInOnyx after every other Onyx-writing middleware and before FraudMonitoring', () => {
+        const indexOf = (middleware: RequestModule.Middleware) => registered.indexOf(middleware);
+
+        expect(indexOf(SaveResponseInOnyx)).toBeGreaterThanOrEqual(0);
+        expect(indexOf(RecordFullReconnectTime)).toBeLessThan(indexOf(SaveResponseInOnyx));
+        expect(indexOf(LoadPostDataForOpenOrReconnect)).toBeLessThan(indexOf(SaveResponseInOnyx));
+        expect(indexOf(FraudMonitoring)).toBeGreaterThan(indexOf(SaveResponseInOnyx));
+    });
+});

tests/unit/TransactionGroupListItemTest.tsx

@@ -1,3 +1,4 @@
+import '@libs/Middleware/register';
 import {fireEvent, render, screen} from '@testing-library/react-native';
 
 import ComposeProviders from '@components/ComposeProviders';