Mirrors/Rocket.Chat
1
0
Fork 0
mirror of https://github.com/RocketChat/Rocket.Chat.git synced 2025-12-28 06:47:25 +00:00
Code Issues Actions 4 Packages Projects Releases Wiki Activity

feat(apps): stop persisting intermediate status transitions to database (#37167)

Browse Source
This commit is contained in:
Douglas Gubert 2025-11-10 15:57:28 -03:00 committed by GitHub
parent 7b176ffb6b
commit a05b8f72ca
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 39 additions and 73 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
.changeset/real-pans-confess.md Normal file
View File

@ -0,0 +1,8 @@
---
'@rocket.chat/apps-engine': minor
'@rocket.chat/meteor': minor
---
Changes a behavior that would store the result of every status transition that happened to apps
This caused intermediate status to be saved to the database, which could prevent apps from being restored to the desired status when restarted or during server startup.

4
apps/meteor/ee/server/apps/communication/rest.ts
View File

@ -474,8 +474,8 @@ export class AppsRestApi {
try {
await canEnableApp(aff.getApp().getStorageItem());
const success = await manager.enable(info.id);
info.status = success ? AppStatus.AUTO_ENABLED : info.status;
const success = await manager.changeStatus(info.id, AppStatus.MANUALLY_ENABLED);
info.status = await success.getStatus();
} catch (error) {
orchestrator.getRocketChatLogger().warn(`App "${info.id}" was installed but could not be enabled: `, error);
}

2
apps/meteor/tests/end-to-end/api/livechat/00-rooms.ts
View File

@ -101,7 +101,7 @@ describe('LIVECHAT - rooms', () => {
expect(res.body).to.have.a.property('app');
expect(res.body.app).to.have.a.property('id');
expect(res.body.app).to.have.a.property('version');
expect(res.body.app).to.have.a.property('status').and.to.be.equal('auto_enabled');
expect(res.body.app).to.have.a.property('status').and.to.be.equal('manually_enabled');
appId = res.body.app.id;
});

2
apps/meteor/tests/end-to-end/apps/installation.ts
View File

@ -52,7 +52,7 @@ describe('Apps - Installation', () => {
expect(res.body).to.have.a.property('app');
expect(res.body.app).to.have.a.property('id');
expect(res.body.app).to.have.a.property('version');
expect(res.body.app).to.have.a.property('status').and.to.be.equal('auto_enabled');
expect(res.body.app).to.have.a.property('status').and.to.be.equal('manually_enabled');
app = res.body.app;
})

96
packages/apps-engine/src/server/AppManager.ts
View File

@ -310,7 +310,7 @@ export class AppManager {
continue;
}
await this.initializeApp(rl.getStorageItem(), rl, false, true).catch(console.error);
await this.initializeApp(rl, true).catch(console.error);
}
// Let's ensure the required settings are all set
@ -329,7 +329,7 @@ export class AppManager {
for (const app of this.apps.values()) {
const status = await app.getStatus();
if (!AppStatusUtils.isDisabled(status) && AppStatusUtils.isEnabled(app.getPreviousStatus())) {
await this.enableApp(app.getStorageItem(), app, true, app.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error);
await this.enableApp(app).catch(console.error);
} else if (!AppStatusUtils.isError(status)) {
this.listenerManager.lockEssentialEvents(app);
this.uiActionButtonManager.clearAppActionButtons(app.getID());
@ -457,14 +457,7 @@ export class AppManager {
throw new Error(`Could not enable an App with the id of "${id}" as it doesn't exist.`);
}
const isSetup = await this.runStartUpProcess(storageItem, rl, true, false);
if (isSetup) {
storageItem.status = await rl.getStatus();
// This is async, but we don't care since it only updates in the database
// and it should not mutate any properties we care about
await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {});
}
const isSetup = await this.runStartUpProcess(storageItem, rl, false);
return isSetup;
}
@ -495,12 +488,7 @@ export class AppManager {
const storageItem = await this.appMetadataStorage.retrieveOne(id);
app.getStorageItem().marketplaceInfo = storageItem.marketplaceInfo;
await app.validateLicense().catch();
storageItem.status = await app.getStatus();
// This is async, but we don't care since it only updates in the database
// and it should not mutate any properties we care about
await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {});
await app.validateLicense().catch(() => {});
return true;
}
@ -570,7 +558,7 @@ export class AppManager {
const descriptor: IAppStorageItem = {
id: result.info.id,
info: result.info,
status: AppStatus.UNKNOWN,
status: enable ? AppStatus.MANUALLY_ENABLED : AppStatus.MANUALLY_DISABLED,
settings: {},
implemented: result.implemented.getValues(),
installationSource: marketplaceInfo ? AppInstallationSource.MARKETPLACE : AppInstallationSource.PRIVATE,
@ -645,15 +633,15 @@ export class AppManager {
// If an error occurs during this, oh well.
});
await this.installApp(created, app, user);
await this.installApp(app, user);
// Should enable === true, then we go through the entire start up process
// Otherwise, we only initialize it.
if (enable) {
// Start up the app
await this.runStartUpProcess(created, app, false, false);
await this.runStartUpProcess(created, app, false);
} else {
await this.initializeApp(created, app, true);
await this.initializeApp(app);
}
return aff;
@ -727,15 +715,10 @@ export class AppManager {
const descriptor: IAppStorageItem = {
...old,
createdAt: old.createdAt,
id: result.info.id,
info: result.info,
status: (await this.apps.get(old.id)?.getStatus()) || old.status,
languageContent: result.languageContent,
settings: old.settings,
implemented: result.implemented.getValues(),
...(old.marketplaceInfo && { marketplaceInfo: old.marketplaceInfo }),
...(old.sourcePath && { sourcePath: old.sourcePath }),
};
if (!permissionsGranted) {
@ -833,12 +816,12 @@ export class AppManager {
public async updateAndStartupLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) {
const app = await this.updateLocal(stored, appPackageOrInstance);
await this.runStartUpProcess(stored, app, false, true);
await this.runStartUpProcess(stored, app, true);
}
public async updateAndInitializeLocal(stored: IAppStorageItem, appPackageOrInstance: ProxiedApp | Buffer) {
const app = await this.updateLocal(stored, appPackageOrInstance);
await this.initializeApp(stored, app, true, true);
await this.initializeApp(app, true);
}
public getLanguageContent(): { [key: string]: object } {
@ -870,6 +853,8 @@ export class AppManager {
throw new Error('Can not change the status of an App which does not currently exist.');
}
const storageItem = await rl.getStorageItem();
if (AppStatusUtils.isEnabled(status)) {
// Then enable it
if (AppStatusUtils.isEnabled(await rl.getStatus())) {
@ -877,12 +862,18 @@ export class AppManager {
}
await this.enable(rl.getID());
storageItem.status = AppStatus.MANUALLY_ENABLED;
await this.appMetadataStorage.updateStatus(storageItem._id, AppStatus.MANUALLY_ENABLED);
} else {
if (!AppStatusUtils.isEnabled(await rl.getStatus())) {
throw new Error('Can not disable an App which is not enabled.');
}
await this.disable(rl.getID(), AppStatus.MANUALLY_DISABLED);
storageItem.status = AppStatus.MANUALLY_DISABLED;
await this.appMetadataStorage.updateStatus(storageItem._id, AppStatus.MANUALLY_DISABLED);
}
return rl;
@ -973,27 +964,22 @@ export class AppManager {
const item = rl.getStorageItem();
await this.initializeApp(item, rl, false, silenceStatus);
await this.initializeApp(rl, silenceStatus);
if (!this.areRequiredSettingsSet(item)) {
await rl.setStatus(AppStatus.INVALID_SETTINGS_DISABLED);
}
if (!AppStatusUtils.isDisabled(await rl.getStatus()) && AppStatusUtils.isEnabled(rl.getPreviousStatus())) {
await this.enableApp(item, rl, false, rl.getPreviousStatus() === AppStatus.MANUALLY_ENABLED, silenceStatus);
await this.enableApp(rl, silenceStatus);
}
return this.apps.get(item.id);
}
private async runStartUpProcess(
storageItem: IAppStorageItem,
app: ProxiedApp,
isManual: boolean,
silenceStatus: boolean,
): Promise<boolean> {
private async runStartUpProcess(storageItem: IAppStorageItem, app: ProxiedApp, silenceStatus: boolean): Promise<boolean> {
if ((await app.getStatus()) !== AppStatus.INITIALIZED) {
const isInitialized = await this.initializeApp(storageItem, app, true, silenceStatus);
const isInitialized = await this.initializeApp(app, silenceStatus);
if (!isInitialized) {
return false;
}
@ -1004,10 +990,10 @@ export class AppManager {
return false;
}
return this.enableApp(storageItem, app, true, isManual, silenceStatus);
return this.enableApp(app, silenceStatus);
}
private async installApp(_storageItem: IAppStorageItem, app: ProxiedApp, user: IUser): Promise<boolean> {
private async installApp(app: ProxiedApp, user: IUser): Promise<boolean> {
let result: boolean;
const context = { user };
@ -1044,7 +1030,7 @@ export class AppManager {
return result;
}
private async initializeApp(storageItem: IAppStorageItem, app: ProxiedApp, saveToDb = true, silenceStatus = false): Promise<boolean> {
private async initializeApp(app: ProxiedApp, silenceStatus = false): Promise<boolean> {
let result: boolean;
try {
@ -1072,17 +1058,6 @@ export class AppManager {
result = false;
await app.setStatus(status, silenceStatus);
// If some error has happened in initialization, like license or installations invalidation
// we need to store this on the DB regardless of what the parameter requests
saveToDb = true;
}
if (saveToDb) {
// This is async, but we don't care since it only updates in the database
// and it should not mutate any properties we care about
storageItem.status = await app.getStatus();
await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {});
}
return result;
@ -1133,13 +1108,7 @@ export class AppManager {
return result;
}
private async enableApp(
storageItem: IAppStorageItem,
app: ProxiedApp,
saveToDb = true,
isManual: boolean,
silenceStatus = false,
): Promise<boolean> {
private async enableApp(app: ProxiedApp, silenceStatus = false): Promise<boolean> {
let enable: boolean;
let status = AppStatus.ERROR_DISABLED;
@ -1150,7 +1119,7 @@ export class AppManager {
enable = (await app.call(AppMethod.ONENABLE)) as boolean;
if (enable) {
status = isManual ? AppStatus.MANUALLY_ENABLED : AppStatus.AUTO_ENABLED;
status = AppStatus.MANUALLY_ENABLED;
} else {
status = AppStatus.DISABLED;
console.warn(`The App (${app.getID()}) disabled itself when being enabled. \nCheck the "onEnable" implementation for details.`);
@ -1167,10 +1136,6 @@ export class AppManager {
}
console.error(e);
// If some error has happened during enabling, like license or installations invalidation
// we need to store this on the DB regardless of what the parameter requests
saveToDb = true;
}
if (enable) {
@ -1188,13 +1153,6 @@ export class AppManager {
});
}
if (saveToDb) {
storageItem.status = status;
// This is async, but we don't care since it only updates in the database
// and it should not mutate any properties we care about
await this.appMetadataStorage.updateStatus(storageItem._id, storageItem.status).catch(() => {});
}
await app.setStatus(status, silenceStatus);
return enable;