Skip to content

Commit f529a27

Browse files
clauded-gubert
claude
authored and
d-gubert
committed
refactor(apps): replace RemoteBridges facade with a bridgeCall helper
The RemoteBridges facade built one Proxy per host bridge (25 getters) so ported accessors could keep their host call shape (this.bridge.doX(...)). Its RemoteBridge type was an index signature over unknown, so it provided no compile-time typing - every call site cast `as Promise<T>` regardless - and the `do*` restriction was only enforced at runtime by the proxy trap. With the host accessors now deleted, the "keep the host shape" rationale is spent. Replace it with a single free helper `bridgeCall<T>(senderFn, bridge, method, ...params)`: - `bridge` is a `BridgeName` union and `method` is constrained to `do${string}`, so the bridge-method gate is now checked at compile time instead of at runtime. - It owns the wire encoding (`bridges:<bridge>:<method>`), result unwrap and `formatErrorResponse` mapping - the one place that knows the encoding. - Accessors take `senderFn` and call `bridgeCall(this.senderFn, 'getUserBridge', 'doGetById', id, 'APP_ID')`; the `<T>` generic replaces the old `as Promise<T>` casts. Public accessor typing still comes from `implements I*`. No behavior change: `'APP_ID'` is still passed explicitly per accessor (not injected), params are unchanged, and every emitted `{ method, params }` is identical. Deletes RemoteBridges.ts (+ its test); adds bridgeCall.ts (+ test). Net -234 lines. base-runtime: 204/204 tests pass; host + base-runtime tsc clean; lint clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018FbxFGJWHiroxrdNPJRL3P
1 parent 09b42bc commit f529a27

47 files changed

Lines changed: 634 additions & 709 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎packages/apps/base-runtime/src/lib/accessors/Persistence.ts‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,53 @@
11
import type { IPersistence } from '@rocket.chat/apps-engine/definition/accessors';
22
import type { RocketChatAssociationRecord } from '@rocket.chat/apps-engine/definition/metadata';
33

4-
import type { RemoteBridges } from '../bridges/RemoteBridges';
4+
import { bridgeCall } from '../bridges/bridgeCall';
5+
import type * as Messenger from '../messenger';
56

67
export class Persistence implements IPersistence {
7-
constructor(private readonly bridges: RemoteBridges) {}
8+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
89

910
public create(data: object): Promise<string> {
10-
return this.bridges.getPersistenceBridge().doCreate(data, 'APP_ID') as Promise<string>;
11+
return bridgeCall<string>(this.senderFn, 'getPersistenceBridge', 'doCreate', data, 'APP_ID');
1112
}
1213

1314
public createWithAssociation(data: object, association: RocketChatAssociationRecord): Promise<string> {
14-
return this.bridges.getPersistenceBridge().doCreateWithAssociations(data, new Array(association), 'APP_ID') as Promise<string>;
15+
return bridgeCall<string>(this.senderFn, 'getPersistenceBridge', 'doCreateWithAssociations', data, new Array(association), 'APP_ID');
1516
}
1617

1718
public createWithAssociations(data: object, associations: Array<RocketChatAssociationRecord>): Promise<string> {
18-
return this.bridges.getPersistenceBridge().doCreateWithAssociations(data, associations, 'APP_ID') as Promise<string>;
19+
return bridgeCall<string>(this.senderFn, 'getPersistenceBridge', 'doCreateWithAssociations', data, associations, 'APP_ID');
1920
}
2021

2122
public update(id: string, data: object, upsert = false): Promise<string> {
22-
return this.bridges.getPersistenceBridge().doUpdate(id, data, upsert, 'APP_ID') as Promise<string>;
23+
return bridgeCall<string>(this.senderFn, 'getPersistenceBridge', 'doUpdate', id, data, upsert, 'APP_ID');
2324
}
2425

2526
public updateByAssociation(association: RocketChatAssociationRecord, data: object, upsert = false): Promise<string> {
26-
return this.bridges.getPersistenceBridge().doUpdateByAssociations(new Array(association), data, upsert, 'APP_ID') as Promise<string>;
27+
return bridgeCall<string>(
28+
this.senderFn,
29+
'getPersistenceBridge',
30+
'doUpdateByAssociations',
31+
new Array(association),
32+
data,
33+
upsert,
34+
'APP_ID',
35+
);
2736
}
2837

2938
public updateByAssociations(associations: Array<RocketChatAssociationRecord>, data: object, upsert = false): Promise<string> {
30-
return this.bridges.getPersistenceBridge().doUpdateByAssociations(associations, data, upsert, 'APP_ID') as Promise<string>;
39+
return bridgeCall<string>(this.senderFn, 'getPersistenceBridge', 'doUpdateByAssociations', associations, data, upsert, 'APP_ID');
3140
}
3241

3342
public remove(id: string): Promise<object> {
34-
return this.bridges.getPersistenceBridge().doRemove(id, 'APP_ID') as Promise<object>;
43+
return bridgeCall<object>(this.senderFn, 'getPersistenceBridge', 'doRemove', id, 'APP_ID');
3544
}
3645

3746
public removeByAssociation(association: RocketChatAssociationRecord): Promise<Array<object>> {
38-
return this.bridges.getPersistenceBridge().doRemoveByAssociations(new Array(association), 'APP_ID') as Promise<Array<object>>;
47+
return bridgeCall<Array<object>>(this.senderFn, 'getPersistenceBridge', 'doRemoveByAssociations', new Array(association), 'APP_ID');
3948
}
4049

4150
public removeByAssociations(associations: Array<RocketChatAssociationRecord>): Promise<Array<object>> {
42-
return this.bridges.getPersistenceBridge().doRemoveByAssociations(associations, 'APP_ID') as Promise<Array<object>>;
51+
return bridgeCall<Array<object>>(this.senderFn, 'getPersistenceBridge', 'doRemoveByAssociations', associations, 'APP_ID');
4352
}
4453
}

‎packages/apps/base-runtime/src/lib/accessors/environment/EnvironmentalVariableRead.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import type { IEnvironmentalVariableRead } from '@rocket.chat/apps-engine/definition/accessors';
22

3-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
3+
import { bridgeCall } from '../../bridges/bridgeCall';
4+
import type * as Messenger from '../../messenger';
45

56
export class EnvironmentalVariableRead implements IEnvironmentalVariableRead {
6-
constructor(private readonly bridges: RemoteBridges) {}
7+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
78

89
public getValueByName(envVarName: string): Promise<string> {
9-
return this.bridges.getEnvironmentalVariableBridge().doGetValueByName(envVarName, 'APP_ID') as Promise<string>;
10+
return bridgeCall<string>(this.senderFn, 'getEnvironmentalVariableBridge', 'doGetValueByName', envVarName, 'APP_ID');
1011
}
1112

1213
public isReadable(envVarName: string): Promise<boolean> {
13-
return this.bridges.getEnvironmentalVariableBridge().doIsReadable(envVarName, 'APP_ID') as Promise<boolean>;
14+
return bridgeCall<boolean>(this.senderFn, 'getEnvironmentalVariableBridge', 'doIsReadable', envVarName, 'APP_ID');
1415
}
1516

1617
public isSet(envVarName: string): Promise<boolean> {
17-
return this.bridges.getEnvironmentalVariableBridge().doIsSet(envVarName, 'APP_ID') as Promise<boolean>;
18+
return bridgeCall<boolean>(this.senderFn, 'getEnvironmentalVariableBridge', 'doIsSet', envVarName, 'APP_ID');
1819
}
1920
}

‎packages/apps/base-runtime/src/lib/accessors/environment/ServerSettingRead.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,18 @@
11
import type { IServerSettingRead } from '@rocket.chat/apps-engine/definition/accessors';
22
import type { ISetting } from '@rocket.chat/apps-engine/definition/settings';
33

4-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
4+
import { bridgeCall } from '../../bridges/bridgeCall';
5+
import type * as Messenger from '../../messenger';
56

67
export class ServerSettingRead implements IServerSettingRead {
7-
constructor(private readonly bridges: RemoteBridges) {}
8+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
89

910
public getOneById(id: string): Promise<ISetting> {
10-
return this.bridges.getServerSettingBridge().doGetOneById(id, 'APP_ID') as Promise<ISetting>;
11+
return bridgeCall<ISetting>(this.senderFn, 'getServerSettingBridge', 'doGetOneById', id, 'APP_ID');
1112
}
1213

1314
public async getValueById(id: string): Promise<any> {
14-
const set = (await this.bridges.getServerSettingBridge().doGetOneById(id, 'APP_ID')) as ISetting;
15+
const set = (await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doGetOneById', id, 'APP_ID')) as ISetting;
1516

1617
// The host accessor checks `typeof set === 'undefined'`, but across the RPC boundary an
1718
// absent (undefined) host return is serialized as null, so both must be treated as "not found".
@@ -31,6 +32,6 @@ export class ServerSettingRead implements IServerSettingRead {
3132
}
3233

3334
public isReadableById(id: string): Promise<boolean> {
34-
return this.bridges.getServerSettingBridge().doIsReadableById(id, 'APP_ID') as Promise<boolean>;
35+
return bridgeCall<boolean>(this.senderFn, 'getServerSettingBridge', 'doIsReadableById', id, 'APP_ID');
3536
}
3637
}

‎packages/apps/base-runtime/src/lib/accessors/environment/ServerSettingUpdater.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,17 @@
11
import type { IServerSettingUpdater } from '@rocket.chat/apps-engine/definition/accessors';
22
import type { ISetting } from '@rocket.chat/apps-engine/definition/settings';
33

4-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
4+
import { bridgeCall } from '../../bridges/bridgeCall';
5+
import type * as Messenger from '../../messenger';
56

67
export class ServerSettingUpdater implements IServerSettingUpdater {
7-
constructor(private readonly bridges: RemoteBridges) {}
8+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
89

910
public async updateOne(setting: ISetting): Promise<void> {
10-
await this.bridges.getServerSettingBridge().doUpdateOne(setting, 'APP_ID');
11+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doUpdateOne', setting, 'APP_ID');
1112
}
1213

1314
public async incrementValue(id: ISetting['id'], value = 1): Promise<void> {
14-
await this.bridges.getServerSettingBridge().doIncrementValue(id, value, 'APP_ID');
15+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doIncrementValue', id, value, 'APP_ID');
1516
}
1617
}

‎packages/apps/base-runtime/src/lib/accessors/environment/ServerSettingsModify.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,25 @@
11
import type { IServerSettingsModify } from '@rocket.chat/apps-engine/definition/accessors';
22
import type { ISetting } from '@rocket.chat/apps-engine/definition/settings';
33

4-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
4+
import { bridgeCall } from '../../bridges/bridgeCall';
5+
import type * as Messenger from '../../messenger';
56

67
export class ServerSettingsModify implements IServerSettingsModify {
7-
constructor(private readonly bridges: RemoteBridges) {}
8+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
89

910
public async hideGroup(name: string): Promise<void> {
10-
await this.bridges.getServerSettingBridge().doHideGroup(name, 'APP_ID');
11+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doHideGroup', name, 'APP_ID');
1112
}
1213

1314
public async hideSetting(id: string): Promise<void> {
14-
await this.bridges.getServerSettingBridge().doHideSetting(id, 'APP_ID');
15+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doHideSetting', id, 'APP_ID');
1516
}
1617

1718
public async modifySetting(setting: ISetting): Promise<void> {
18-
await this.bridges.getServerSettingBridge().doUpdateOne(setting, 'APP_ID');
19+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doUpdateOne', setting, 'APP_ID');
1920
}
2021

2122
public async incrementValue(id: ISetting['id'], value = 1): Promise<void> {
22-
await this.bridges.getServerSettingBridge().doIncrementValue(id, value, 'APP_ID');
23+
await bridgeCall(this.senderFn, 'getServerSettingBridge', 'doIncrementValue', id, value, 'APP_ID');
2324
}
2425
}

‎packages/apps/base-runtime/src/lib/accessors/environment/SettingRead.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,16 @@
11
import type { ISettingRead } from '@rocket.chat/apps-engine/definition/accessors';
22
import type { ISetting } from '@rocket.chat/apps-engine/definition/settings';
33

4-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
4+
import { bridgeCall } from '../../bridges/bridgeCall';
5+
import type * as Messenger from '../../messenger';
56

67
// App settings are host-persisted metadata (ProxiedApp storage item), fronted by the internal
78
// AppResourceBridge. The value fallback that used to run host-side now runs locally.
89
export class SettingRead implements ISettingRead {
9-
constructor(private readonly bridges: RemoteBridges) {}
10+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
1011

1112
public getById(id: string): Promise<ISetting> {
12-
return this.bridges.getAppResourceBridge().doGetSettingById(id, 'APP_ID') as Promise<ISetting>;
13+
return bridgeCall<ISetting>(this.senderFn, 'getAppResourceBridge', 'doGetSettingById', id, 'APP_ID');
1314
}
1415

1516
public async getValueById(id: string): Promise<any> {

‎packages/apps/base-runtime/src/lib/accessors/environment/SettingUpdater.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
import type { ISettingUpdater } from '@rocket.chat/apps-engine/definition/accessors/ISettingUpdater';
22
import type { ISetting } from '@rocket.chat/apps-engine/definition/settings';
33

4-
import type { RemoteBridges } from '../../bridges/RemoteBridges';
4+
import { bridgeCall } from '../../bridges/bridgeCall';
5+
import type * as Messenger from '../../messenger';
56

67
// The "not found" guard and the AppSettingsManager persistence run host-side in the
78
// AppResourceBridge (they depend on the ProxiedApp storage item and the settings manager); the
89
// runtime accessor is a thin forwarder.
910
export class SettingUpdater implements ISettingUpdater {
10-
constructor(private readonly bridges: RemoteBridges) {}
11+
constructor(private readonly senderFn: typeof Messenger.sendRequest) {}
1112

1213
public async updateValue(id: ISetting['id'], value: ISetting['value']): Promise<void> {
13-
await this.bridges.getAppResourceBridge().doUpdateSettingValue(id, value, 'APP_ID');
14+
await bridgeCall(this.senderFn, 'getAppResourceBridge', 'doUpdateSettingValue', id, value, 'APP_ID');
1415
}
1516

1617
public async updateSelectOptions(id: ISetting['id'], values: ISetting['values']): Promise<void> {
17-
await this.bridges.getAppResourceBridge().doUpdateSettingSelectOptions(id, values, 'APP_ID');
18+
await bridgeCall(this.senderFn, 'getAppResourceBridge', 'doUpdateSettingSelectOptions', id, values, 'APP_ID');
1819
}
1920
}

‎packages/apps/base-runtime/src/lib/accessors/environment/tests/environment.test.ts‎

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import * as assert from 'node:assert';
22
import { describe, it } from 'node:test';
33

4-
import { RemoteBridges } from '../../../bridges/RemoteBridges';
54
import { createRecordingSender } from '../../tests/helpers/parityHarness';
65
import { EnvironmentalVariableRead } from '../EnvironmentalVariableRead';
76
import { ServerSettingRead } from '../ServerSettingRead';
@@ -10,34 +9,34 @@ import { ServerSettingsModify } from '../ServerSettingsModify';
109

1110
const setup = (responses = {}) => {
1211
const rec = createRecordingSender(responses);
13-
return { rec, bridges: new RemoteBridges(rec.sender) };
12+
return { rec, senderFn: rec.sender };
1413
};
1514

1615
describe('Environment accessors (base-runtime)', () => {
1716
describe('ServerSettingRead', () => {
1817
it('getValueById returns the value when set', async () => {
19-
const { bridges } = setup({ 'bridges:getServerSettingBridge:doGetOneById': { value: 'v', packageValue: 'pv' } });
20-
assert.strictEqual(await new ServerSettingRead(bridges).getValueById('s1'), 'v');
18+
const { senderFn } = setup({ 'bridges:getServerSettingBridge:doGetOneById': { value: 'v', packageValue: 'pv' } });
19+
assert.strictEqual(await new ServerSettingRead(senderFn).getValueById('s1'), 'v');
2120
});
2221

2322
it('getValueById falls back to packageValue when value is null/undefined', async () => {
2423
const nullVal = setup({ 'bridges:getServerSettingBridge:doGetOneById': { value: null, packageValue: 'pv' } });
25-
assert.strictEqual(await new ServerSettingRead(nullVal.bridges).getValueById('s1'), 'pv');
24+
assert.strictEqual(await new ServerSettingRead(nullVal.senderFn).getValueById('s1'), 'pv');
2625
});
2726

2827
it('getValueById throws when the setting is not found', async () => {
29-
const { bridges } = setup({ 'bridges:getServerSettingBridge:doGetOneById': undefined });
30-
await assert.rejects(() => new ServerSettingRead(bridges).getValueById('missing'), /No Server Setting found/);
28+
const { senderFn } = setup({ 'bridges:getServerSettingBridge:doGetOneById': undefined });
29+
await assert.rejects(() => new ServerSettingRead(senderFn).getValueById('missing'), /No Server Setting found/);
3130
});
3231

3332
it('getAll throws (not implemented), matching the host accessor', () => {
34-
const { bridges } = setup();
35-
assert.throws(() => new ServerSettingRead(bridges).getAll(), /Method not implemented/);
33+
const { senderFn } = setup();
34+
assert.throws(() => new ServerSettingRead(senderFn).getAll(), /Method not implemented/);
3635
});
3736

3837
it('getOneById and isReadableById forward verbatim', async () => {
39-
const { rec, bridges } = setup();
40-
const read = new ServerSettingRead(bridges);
38+
const { rec, senderFn } = setup();
39+
const read = new ServerSettingRead(senderFn);
4140
await read.getOneById('s1');
4241
await read.isReadableById('s1');
4342
assert.deepStrictEqual(rec.emitted(), [
@@ -49,8 +48,8 @@ describe('Environment accessors (base-runtime)', () => {
4948

5049
describe('EnvironmentalVariableRead', () => {
5150
it('forwards each method to its bridge call', async () => {
52-
const { rec, bridges } = setup();
53-
const env = new EnvironmentalVariableRead(bridges);
51+
const { rec, senderFn } = setup();
52+
const env = new EnvironmentalVariableRead(senderFn);
5453
await env.getValueByName('X');
5554
await env.isReadable('X');
5655
await env.isSet('X');
@@ -64,8 +63,8 @@ describe('Environment accessors (base-runtime)', () => {
6463

6564
describe('ServerSettingUpdater', () => {
6665
it('incrementValue defaults the amount to 1', async () => {
67-
const { rec, bridges } = setup();
68-
await new ServerSettingUpdater(bridges).incrementValue('s1');
66+
const { rec, senderFn } = setup();
67+
await new ServerSettingUpdater(senderFn).incrementValue('s1');
6968
assert.deepStrictEqual(rec.emitted()[0], {
7069
method: 'bridges:getServerSettingBridge:doIncrementValue',
7170
params: ['s1', 1, 'APP_ID'],
@@ -75,8 +74,8 @@ describe('Environment accessors (base-runtime)', () => {
7574

7675
describe('ServerSettingsModify', () => {
7776
it('modifySetting maps to doUpdateOne and incrementValue defaults to 1', async () => {
78-
const { rec, bridges } = setup();
79-
const modify = new ServerSettingsModify(bridges);
77+
const { rec, senderFn } = setup();
78+
const modify = new ServerSettingsModify(senderFn);
8079
await modify.modifySetting({ id: 's1' } as any);
8180
await modify.incrementValue('s1');
8281
assert.deepStrictEqual(rec.emitted(), [

‎packages/apps/base-runtime/src/lib/accessors/http.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { IPersistence } from '@rocket.chat/apps-engine/definition/accessors
33
import type { IRead } from '@rocket.chat/apps-engine/definition/accessors/IRead';
44

55
import { AppObjectRegistry } from '../../AppObjectRegistry';
6-
import { RemoteBridges } from '../bridges/RemoteBridges';
6+
import { bridgeCall } from '../bridges/bridgeCall';
77
import type * as Messenger from '../messenger';
88

99
type RequestMethod = 'get' | 'post' | 'put' | 'head' | 'delete' | 'patch';
@@ -17,17 +17,11 @@ export class Http implements IHttp {
1717

1818
private senderFn: typeof Messenger.sendRequest;
1919

20-
private readonly bridges: RemoteBridges;
21-
2220
constructor(read: IRead, persistence: IPersistence, httpExtender: IHttpExtend, senderFn: typeof Messenger.sendRequest) {
2321
this.read = read;
2422
this.persistence = persistence;
2523
this.httpExtender = httpExtender;
2624
this.senderFn = senderFn;
27-
// The facade reads `this.senderFn` at call time (rather than capturing it) so
28-
// that tests which swap out `senderFn` after construction remain intercepted.
29-
this.bridges = new RemoteBridges((request) => this.senderFn(request));
30-
// this.httpExtender = new HttpExtend();
3125
}
3226

3327
public get(url: string, options?: IHttpRequest): Promise<IHttpResponse> {
@@ -80,7 +74,7 @@ export class Http implements IHttp {
8074
// `appId` here is a field inside the payload object, not a top-level positional
8175
// param, so the host's `'APP_ID'` sentinel substitution cannot reach it - we send
8276
// the resolved id. See docs/proposals/apps-accessor-consolidation/base-runtime-app-id-exceptions.md (bucket C).
83-
let response = await this.bridges.getHttpBridge().doCall({
77+
let response = await bridgeCall(this.senderFn, 'getHttpBridge', 'doCall', {
8478
appId: AppObjectRegistry.get<string>('id'),
8579
method,
8680
url,

0 commit comments

Comments
 (0)