Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve runtime safety checks in connection manager functions #580

Merged
Merged
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
16 changes: 14 additions & 2 deletions lib/OnyxConnectionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,14 @@ class OnyxConnectionManager {
* Adds the connection to the eviction block list. Connections added to this list can never be evicted.
* */
addToEvictionBlockList(connection: Connection): void {
const connectionMetadata = this.connectionsMap.get(connection?.id);
if (!connection) {
Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list passing an undefined connection object.`);
return;
}

const connectionMetadata = this.connectionsMap.get(connection.id);
if (!connectionMetadata) {
Logger.logInfo(`[ConnectionManager] Attempted to add connection to eviction block list but no connection was found.`);
return;
}

Expand All @@ -245,8 +251,14 @@ class OnyxConnectionManager {
* which will enable it to be evicted again.
*/
removeFromEvictionBlockList(connection: Connection): void {
const connectionMetadata = this.connectionsMap.get(connection?.id);
if (!connection) {
Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list passing an undefined connection object.`);
return;
}

const connectionMetadata = this.connectionsMap.get(connection.id);
if (!connectionMetadata) {
Logger.logInfo(`[ConnectionManager] Attempted to remove connection from eviction block list but no connection was found.`);
return;
}

Expand Down
39 changes: 38 additions & 1 deletion tests/unit/OnyxConnectionManagerTest.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
// eslint-disable-next-line max-classes-per-file
import {act} from '@testing-library/react-native';
import Onyx from '../../lib';
import type {Connection} from '../../lib/OnyxConnectionManager';
import connectionManager from '../../lib/OnyxConnectionManager';
import OnyxUtils from '../../lib/OnyxUtils';
import StorageMock from '../../lib/storage';
import type {OnyxKey, WithOnyxConnectOptions} from '../../lib/types';
import type {WithOnyxInstance} from '../../lib/withOnyx/types';
import type GenericCollection from '../utils/GenericCollection';
import waitForPromisesToResolve from '../utils/waitForPromisesToResolve';
import OnyxUtils from '../../lib/OnyxUtils';

// We need access to `connectionsMap` and `generateConnectionID` during the tests but the properties are private,
// so this workaround allows us to have access to them.
Expand Down Expand Up @@ -270,6 +271,18 @@ describe('OnyxConnectionManager', () => {
expect(connectionsMap.size).toEqual(0);
});

it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside disconnect()', () => {
expect(connectionsMap.size).toEqual(0);

expect(() => {
connectionManager.disconnect(undefined as unknown as Connection);
}).not.toThrow();

expect(() => {
connectionManager.disconnect({id: 'connectionID1', callbackID: 'callbackID1'});
}).not.toThrow();
});

describe('withOnyx', () => {
it('should connect to a key two times with withOnyx and create two connections instead of one', async () => {
await StorageMock.setItem(ONYXKEYS.TEST_KEY, 'test');
Expand Down Expand Up @@ -390,5 +403,29 @@ describe('OnyxConnectionManager', () => {
// inexistent connection ID, shouldn't do anything
expect(() => connectionManager.removeFromEvictionBlockList({id: 'connectionID0', callbackID: 'callbackID0'})).not.toThrow();
});

it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside addToEvictionBlockList()', () => {
expect(connectionsMap.size).toEqual(0);

expect(() => {
connectionManager.addToEvictionBlockList(undefined as unknown as Connection);
}).not.toThrow();

expect(() => {
connectionManager.addToEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'});
}).not.toThrow();
});

it('should not throw any errors when passing an undefined connection or trying to access an inexistent one inside removeFromEvictionBlockList()', () => {
expect(connectionsMap.size).toEqual(0);

expect(() => {
connectionManager.removeFromEvictionBlockList(undefined as unknown as Connection);
}).not.toThrow();

expect(() => {
connectionManager.removeFromEvictionBlockList({id: 'connectionID1', callbackID: 'callbackID1'});
}).not.toThrow();
});
});
});
Loading