Skip to content

Commit

Permalink
fix(web-extension): decouple/detach objects returned by remote api fa…
Browse files Browse the repository at this point in the history
…ctory

their lifetime is no longer tied to the parent object
  • Loading branch information
mkazlauskas committed Jun 2, 2023
1 parent ece8cc6 commit a418169
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 10 deletions.
15 changes: 12 additions & 3 deletions packages/web-extension/src/messaging/NonBackgroundMessenger.ts
Expand Up @@ -14,7 +14,14 @@ import {
takeWhile,
tap
} from 'rxjs';
import { Messenger, MessengerDependencies, MessengerPort, PortMessage, ReconnectConfig } from './types';
import {
DeriveChannelOptions,
Messenger,
MessengerDependencies,
MessengerPort,
PortMessage,
ReconnectConfig
} from './types';
import { deriveChannelName } from './util';
import { isNotNil } from '@cardano-sdk/util';
import { retryBackoff } from 'backoff-rxjs';
Expand Down Expand Up @@ -82,15 +89,17 @@ export const createNonBackgroundMessenger = (
return {
channel,
connect$,
deriveChannel(path) {
deriveChannel(path, { detached }: DeriveChannelOptions = {}) {
const messenger = createNonBackgroundMessenger(
{
baseChannel: deriveChannelName(channel, path),
reconnectConfig: { initialDelay, maxDelay }
},
{ logger, runtime }
);
derivedMessengers.add(messenger);
if (!detached) {
derivedMessengers.add(messenger);
}
return messenger;
},
get isShutdown() {
Expand Down
6 changes: 2 additions & 4 deletions packages/web-extension/src/messaging/remoteApi.ts
Expand Up @@ -126,7 +126,7 @@ const consumeFactory =
messageId: newMessageId()
} as FactoryCallMessage)
.subscribe();
const apiMessenger = messenger.deriveChannel(channel);
const apiMessenger = messenger.deriveChannel(channel, { detached: true });
// eslint-disable-next-line no-use-before-define
const api = consumeMessengerRemoteApi(
{
Expand Down Expand Up @@ -312,7 +312,7 @@ export const bindFactoryMethods = <API extends object>(
try {
const args = fromSerializableObject<MethodRequest>(data.factoryCall.args);
const { method, channel } = data.factoryCall;
const factoryMessenger = messenger.deriveChannel(channel);
const factoryMessenger = messenger.deriveChannel(channel, { detached: true });
// eslint-disable-next-line no-use-before-define
const api = exposeMessengerApi(
{
Expand Down Expand Up @@ -345,11 +345,9 @@ export const bindFactoryMethods = <API extends object>(
};
completeSubscription = factoryMessenger.message$.subscribe((msg) => {
if (isCompletionMessage(msg.data)) {
subscription.remove(teardown);
teardown();
}
});
subscription.add(teardown);
} catch (error) {
logger.debug('[bindFactoryMethods] error exposing api', data, error);
}
Expand Down
9 changes: 8 additions & 1 deletion packages/web-extension/src/messaging/types.ts
Expand Up @@ -152,13 +152,20 @@ export interface ConsumeRemoteApiOptions<T> {
getErrorPrototype?: GetErrorPrototype;
}

export interface DeriveChannelOptions {
/**
* If true, shutting down base messenger will not shut down the derived messenger
*/
detached?: boolean;
}

export interface Messenger extends Shutdown {
channel: ChannelName;
connect$: Observable<MinimalPort>;
postMessage(message: unknown): Observable<void>;
message$: Observable<PortMessage>;
isShutdown: boolean;
deriveChannel(path: string): Messenger;
deriveChannel(path: string, options?: DeriveChannelOptions): Messenger;
}

export interface MessengerApiDependencies {
Expand Down
103 changes: 101 additions & 2 deletions packages/web-extension/test/remoteApi/remoteApi.test.ts
@@ -1,13 +1,16 @@
import { EMPTY, Observable, Subject, map } from 'rxjs';
import {
ChannelName,
FactoryCallMessage,
Messenger,
MinimalPort,
PortMessage,
RemoteApiProperties,
RemoteApiPropertyType,
RequestMessage,
bindFactoryMethods,
exposeMessengerApi
} from '../../src/messaging';
import { EMPTY, Observable, Subject, map, of } from 'rxjs';
import { dummyLogger } from 'ts-log';

const logger = dummyLogger;
Expand All @@ -25,6 +28,41 @@ type SimpleApi = {
};
};

// eslint-disable-next-line no-use-before-define
type TestMessenger = Messenger & { derivedMessengers: DerivedMessenger[] };
type DerivedMessenger = { messenger: TestMessenger; detached?: boolean };
const createMockMessenger = (channel: ChannelName) => {
const derivedMessengers = [] as Array<DerivedMessenger>;
const message$ = new Subject<PortMessage<unknown>>();
const connect$ = new Subject<MinimalPort>();
let isShutdown = false;
return {
channel,
connect$,
deriveChannel: jest.fn().mockImplementation((derivedChannel, { detached } = {}) => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const messenger: any = createMockMessenger(`${channel}-${derivedChannel}`);
derivedMessengers.push({ detached, messenger });
return messenger;
}),
derivedMessengers,
get isShutdown() {
return isShutdown;
},
message$,
postMessage: jest.fn().mockImplementation(() => EMPTY),
shutdown: jest.fn().mockImplementation(() => {
isShutdown = true;
message$.complete();
connect$.complete();
for (const { messenger, detached } of derivedMessengers) {
!detached && messenger.shutdown();
}
})
};
};

// TODO: refactor to use createMockMessenger
const setUp = (mode: ApiObjectType) => {
const incomingMsg$ = new Subject<PortMessage<unknown>>();

Expand Down Expand Up @@ -203,7 +241,68 @@ describe('remoteApi', () => {
});
});

describe('ApiFactory', () => {
describe('bindFactoryMethods', () => {
describe('factory invocations create "detached" API objects', () => {
it('does not shut down returned API objects when the factory is shut down', (done) => {
const messenger = createMockMessenger('pingPong');
const factory = bindFactoryMethods(
{
api$: of({
pingApi: () => ({
ping: async () => 'pong'
})
}),
properties: {
pingApi: {
getApiProperties() {
return {
ping: RemoteApiPropertyType.MethodReturningPromise
};
},
propType: RemoteApiPropertyType.ApiFactory
}
}
},
{
logger,
messenger
}
);
messenger.message$.next({
data: {
factoryCall: {
args: [],
channel: 'pingApi-1',
method: 'pingApi'
},
messageId: '1'
} as FactoryCallMessage,
port: {} as MinimalPort
});
factory.shutdown();
expect(messenger.derivedMessengers.length).toBe(1);
expect(messenger.derivedMessengers[0].detached).toBe(true);
expect(messenger.derivedMessengers[0].messenger.isShutdown).toBe(false);
expect(messenger.derivedMessengers[0].messenger.postMessage).not.toBeCalled();
const pingMessage$ = messenger.derivedMessengers[0].messenger.message$ as Subject<PortMessage<unknown>>;
pingMessage$.next({
data: {
messageId: '2',
request: {
args: [],
method: 'ping'
}
} as RequestMessage,
port: {} as MinimalPort
});
setTimeout(() => {
// Sends a response, verifying that it wasn't shutdown
expect(messenger.derivedMessengers[0].messenger.postMessage).toBeCalledTimes(1);
done();
});
});
});

it('each factory call exposes a new api object', (done) => {
const activate1: FactoryCallMessage = {
factoryCall: {
Expand Down

0 comments on commit a418169

Please sign in to comment.