feat(apps): graduate getUserRoomIds experiment to the user bridge (#37547)

Co-authored-by: Douglas Gubert <1810309+d-gubert@users.noreply.github.com>

.changeset/calm-falcons-gather.md

@@ -0,0 +1,6 @@
+---
+"@rocket.chat/apps-engine": minor
+"@rocket.chat/meteor": minor
+---
+
+Adds the `getUserRoomIds` method to the `UserRead` accessor in the Apps-Engine, graduating it from the experimental bridge to the stable user bridge.

apps/meteor/app/apps/server/bridges/experimental.ts

@@ -1,31 +1,8 @@
 import type { IAppServerOrchestrator } from '@rocket.chat/apps';
 import { ExperimentalBridge } from '@rocket.chat/apps-engine/server/bridges';
-import { Subscriptions } from '@rocket.chat/models';
-
-import { metrics } from '../../../metrics/server/lib/metrics';
 
 export class AppExperimentalBridge extends ExperimentalBridge {
-	constructor(private readonly orch: IAppServerOrchestrator) {
+	constructor(protected readonly orch: IAppServerOrchestrator) {
 		super();
 	}
-
-	protected async getUserRoomIds(userId: string, appId: string): Promise<string[] | undefined> {
-		const stopTimer = metrics.appBridgeMethods.startTimer({
-			bridge: 'experimental',
-			method: 'getUserRoomIds',
-			app_id: appId,
-		});
-
-		try {
-			this.orch.debugLog(`The App ${appId} is getting the room ids for the user: "${userId}"`);
-
-			const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray();
-
-			const result = subscriptions.map((subscription) => subscription.rid);
-
-			return result;
-		} finally {
-			stopTimer();
-		}
-	}
 }

apps/meteor/app/apps/server/bridges/users.ts

@@ -168,4 +168,12 @@ export class AppUserBridge extends UserBridge {
 	protected async getUserUnreadMessageCount(uid: string): Promise<number> {
 		return Subscriptions.getBadgeCount(uid);
 	}
+
+	protected async getUserRoomIds(userId: string, appId: string): Promise<string[]> {
+		this.orch.debugLog(`The App ${appId} is getting the room ids for the user: "${userId}"`);
+
+		const subscriptions = await Subscriptions.findByUserId(userId, { projection: { rid: 1 } }).toArray();
+
+		return subscriptions.map((subscription) => subscription.rid);
+	}
 }

packages/apps-engine/src/definition/accessors/IExperimentalRead.ts

@@ -5,12 +5,5 @@
  * team evaluates the proper signature, underlying implementation and performance
  * impact of candidates for future APIs
  */
-export interface IExperimentalRead {
-	/**
-	 * Fetches the IDs of the rooms that the user is a member of.
-	 *
-	 * @returns an array of room ids or undefined if the app doesn't have the proper permission
-	 * @experimental
-	 */
-	getUserRoomIds(userId: string): Promise<string[] | undefined>;
-}
+// eslint-disable-next-line @typescript-eslint/no-empty-interface
+export interface IExperimentalRead {}

packages/apps-engine/src/definition/accessors/IUserRead.ts

@@ -19,4 +19,11 @@ export interface IUserRead {
 	 * @param uid user's id
 	 */
 	getUserUnreadMessageCount(uid: string): Promise<number | undefined>;
+
+	/**
+	 * Fetches the IDs of the rooms that the user is a member of.
+	 *
+	 * @param userId the user whose memberships should be returned
+	 */
+	getUserRoomIds(userId: string): Promise<string[]>;
 }

packages/apps-engine/src/server/accessors/ExperimentalRead.ts

@@ -3,11 +3,7 @@ import type { ExperimentalBridge } from '../bridges';
 
 export class ExperimentalRead implements IExperimentalRead {
 	constructor(
-		private experimentalBridge: ExperimentalBridge,
-		private appId: string,
+		protected readonly experimentalBridge: ExperimentalBridge,
+		protected readonly appId: string,
 	) {}
-
-	public async getUserRoomIds(userId: string): Promise<string[] | undefined> {
-		return this.experimentalBridge.doGetUserRoomIds(userId, this.appId);
-	}
 }

packages/apps-engine/src/server/accessors/UserRead.ts

@@ -23,4 +23,8 @@ export class UserRead implements IUserRead {
 	public getUserUnreadMessageCount(uid: string): Promise<number> {
 		return this.userBridge.doGetUserUnreadMessageCount(uid, this.appId);
 	}
+
+	public getUserRoomIds(userId: string): Promise<string[]> {
+		return this.userBridge.doGetUserRoomIds(userId, this.appId);
+	}
 }

packages/apps-engine/src/server/bridges/ExperimentalBridge.ts

@@ -1,7 +1,4 @@
 import { BaseBridge } from './BaseBridge';
-import { PermissionDeniedError } from '../errors/PermissionDeniedError';
-import { AppPermissionManager } from '../managers/AppPermissionManager';
-import { AppPermissions } from '../permissions/AppPermissions';
 
 /**
  * @description
@@ -10,31 +7,4 @@ import { AppPermissions } from '../permissions/AppPermissions';
  * team evaluates the proper signature, underlying implementation and performance
  * impact of candidates for future APIs
  */
-export abstract class ExperimentalBridge extends BaseBridge {
-	/**
-	 *
-	 * Candidate bridge: User bridge
-	 */
-	public async doGetUserRoomIds(userId: string, appId: string): Promise<string[] | undefined> {
-		if (this.hasPermission('getUserRoomIds', appId)) {
-			return this.getUserRoomIds(userId, appId);
-		}
-	}
-
-	protected abstract getUserRoomIds(userId: string, appId: string): Promise<string[] | undefined>;
-
-	private hasPermission(feature: keyof typeof AppPermissions.experimental, appId: string): boolean {
-		if (AppPermissionManager.hasPermission(appId, AppPermissions.experimental[feature])) {
-			return true;
-		}
-
-		AppPermissionManager.notifyAboutError(
-			new PermissionDeniedError({
-				appId,
-				missingPermissions: [AppPermissions.experimental[feature]],
-			}),
-		);
-
-		return false;
-	}
-}
+export abstract class ExperimentalBridge extends BaseBridge {}

packages/apps-engine/src/server/bridges/UserBridge.ts

@@ -45,6 +45,12 @@ export abstract class UserBridge extends BaseBridge {
 		}
 	}
 
+	public async doGetUserRoomIds(userId: string, appId: string): Promise<string[]> {
+		if (this.hasReadPermission(appId)) {
+			return this.getUserRoomIds(userId, appId);
+		}
+	}
+
 	public async doDeleteUsersCreatedByApp(appId: string, type: UserType.BOT | UserType.APP): Promise<boolean> {
 		if (this.hasWritePermission(appId)) {
 			return this.deleteUsersCreatedByApp(appId, type);
@@ -67,6 +73,8 @@ export abstract class UserBridge extends BaseBridge {
 
 	protected abstract getUserUnreadMessageCount(uid: string, appId: string): Promise<number>;
 
+	protected abstract getUserRoomIds(userId: string, appId: string): Promise<string[]>;
+
 	/**
 	 * Creates a user.
 	 * @param data the essential data for creating a user

packages/apps-engine/src/server/permissions/AppPermissions.ts

@@ -123,7 +123,7 @@ export const AppPermissions = {
 		provide: { name: 'outbound-communication.provide' },
 	},
 	'experimental': {
-		getUserRoomIds: { name: 'experimental.getUserRoomIds' },
+		default: { name: 'experimental.default' },
 	},
 };
 

packages/apps-engine/tests/server/accessors/UserRead.spec.ts

@@ -12,11 +12,14 @@ export class UserReadAccessorTestFixture {
 
 	private mockAppId: 'test-appId';
 
+	private roomIds: Array<string>;
+
 	@SetupFixture
 	public setupFixture() {
 		this.user = TestData.getUser();
+		this.roomIds = ['room-1', 'room-2'];
 
-		const theUser = this.user;
+		const { user: theUser, roomIds } = this;
 		this.mockUserBridge = {
 			doGetById(id, appId): Promise<IUser> {
 				return Promise.resolve(theUser);
@@ -27,6 +30,9 @@ export class UserReadAccessorTestFixture {
 			doGetAppUser(appId?: string): Promise<IUser> {
 				return Promise.resolve(theUser);
 			},
+			doGetUserRoomIds(userId: string): Promise<Array<string>> {
+				return Promise.resolve(roomIds);
+			},
 		} as UserBridge;
 	}
 
@@ -45,5 +51,6 @@ export class UserReadAccessorTestFixture {
 		Expect(await ur.getAppUser(this.mockAppId)).toBeDefined();
 		Expect(await ur.getAppUser(this.mockAppId)).toEqual(this.user);
 		Expect(await ur.getAppUser()).toEqual(this.user);
+		Expect(await ur.getUserRoomIds(this.user.id)).toEqual(this.roomIds);
 	}
 }

packages/apps-engine/tests/test-data/bridges/experimentalBridge.ts

@@ -1,7 +1,3 @@
 import { ExperimentalBridge } from '../../../src/server/bridges';
 
-export class TestExperimentalBridge extends ExperimentalBridge {
-	protected getUserRoomIds(userId: string, appId: string): Promise<string[] | undefined> {
-		throw new Error('Method not implemented.');
-	}
-}
+export class TestExperimentalBridge extends ExperimentalBridge {}

packages/apps-engine/tests/test-data/bridges/userBridge.ts

@@ -38,6 +38,10 @@ export class TestsUserBridge extends UserBridge {
 		throw new Error('Method not implemented.');
 	}
 
+	protected getUserRoomIds(userId: string, appId: string): Promise<string[]> {
+		throw new Error('Method not implemented.');
+	}
+
 	protected deactivate(userId: IUser['id'], confirmRelinquish: boolean, appId: string): Promise<boolean> {
 		throw new Error('Method not implemented.');
 	}

packages/i18n/src/locales/en.i18n.json

@@ -634,6 +634,7 @@
   "Apps_Permissions_message_read": "Access messages",
   "Apps_Permissions_message_write": "Send and modify messages",
   "Apps_Permissions_networking": "Access to this server network",
+  "Apps_Permissions_experimental_default": "Use experimental APIs",
   "Apps_Permissions_persistence": "Store internal data in the database",
   "Apps_Permissions_room_read": "Access room information",
   "Apps_Permissions_room_write": "Create and modify rooms",