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

New room event #266

Merged
merged 12 commits into from
May 19, 2020
7 changes: 7 additions & 0 deletions src/definition/AppStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ export class AppStatusUtilsDef {
return false;
}
}

public isError(status: AppStatus): boolean {
return [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a micro-optimization, but wouldn't it be better to have this array created static and as a constant instead of having to be created every time this method is called?

Copy link
Member Author

@d-gubert d-gubert May 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed it would be slightly more efficient, indeed it is a micro-optimization. IMO it's clearer to read it that way, everything is inside the method's body.

If time comes we need this list elsewhere, no doubt we should extract to a static array. Right now I'm more inclined to go with the "clearer" version

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, I’m of the opposite opinion. I think it is more clear to read the code when it is reading a named variable/constant that expresses what it is for than to declare it in the body of the method being called.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking what you said as a rule is generally safer, but in this specific case, we have a bunch of things helping us: short method body; short method name; clear enum items named inside the array; few items inside it.

But should the situation be different, it would be worth it to go down a different path 😃

AppStatus.ERROR_DISABLED,
AppStatus.COMPILER_ERROR_DISABLED,
].includes(status);
}
}

export const AppStatusUtils = new AppStatusUtilsDef();
14 changes: 14 additions & 0 deletions src/definition/exceptions/AppsEngineException.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* The internal exception from the framework
*
* It's used to signal to the outside world that
* a _known_ exception has happened during the execution
* of the apps.
*
* It's the base exception for other known classes
* such as UserNotAllowedException, which is used
* to inform the host that an app identified
* that a user cannot perform some action, e.g.
* join a room
*/
export class AppsEngineException extends Error {}
17 changes: 17 additions & 0 deletions src/definition/exceptions/EssentialAppDisabledException.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { AppsEngineException } from '.';

/**
* This exception informs the host system that an
* app essential to the execution of a system action
* is disabled, so the action should be halted.
*
* Apps can register to be considered essential to
* the execution of internal events of the framework
* such as `IPreMessageSentPrevent`, `IPreRoomUserJoined`,
* etc.
*
* This is used interally by the framework and is not
* intended to be thrown manually by apps.
*/
export class EssentialAppDisabledException extends AppsEngineException {
}
14 changes: 14 additions & 0 deletions src/definition/exceptions/UserNotAllowedException.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { AppsEngineException } from '.';

/**
* This exception informs the host system that an
* app has determined that an user is not allowed
* to perform a specific action.
*
* Currently it is expected to be thrown by the
* following events:
* - IPreRoomCreatePrevent
* - IPreRoomUserJoined
*/
export class UserNotAllowedException extends AppsEngineException {
}
3 changes: 3 additions & 0 deletions src/definition/exceptions/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export * from './AppsEngineException';
export * from './EssentialAppDisabledException';
export * from './UserNotAllowedException';
36 changes: 36 additions & 0 deletions src/definition/metadata/AppInterface.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
export enum AppInterface {
// Messages
IPreMessageSentPrevent = 'IPreMessageSentPrevent',
IPreMessageSentExtend = 'IPreMessageSentExtend',
IPreMessageSentModify = 'IPreMessageSentModify',
IPostMessageSent = 'IPostMessageSent',
IPreMessageDeletePrevent = 'IPreMessageDeletePrevent',
IPostMessageDeleted = 'IPostMessageDeleted',
IPreMessageUpdatedPrevent = 'IPreMessageUpdatedPrevent',
IPreMessageUpdatedExtend = 'IPreMessageUpdatedExtend',
IPreMessageUpdatedModify = 'IPreMessageUpdatedModify',
IPostMessageUpdated = 'IPostMessageUpdated',
// Rooms
IPreRoomCreatePrevent = 'IPreRoomCreatePrevent',
IPreRoomCreateExtend = 'IPreRoomCreateExtend',
IPreRoomCreateModify = 'IPreRoomCreateModify',
IPostRoomCreate = 'IPostRoomCreate',
IPreRoomDeletePrevent = 'IPreRoomDeletePrevent',
IPostRoomDeleted = 'IPostRoomDeleted',
IPreRoomUserJoined = 'IPreRoomUserJoined',
IPostRoomUserJoined = 'IPostRoomUserJoined',
// External Components
IPostExternalComponentOpened = 'IPostExternalComponentOpened',
IPostExternalComponentClosed = 'IPostExternalComponentClosed',
// Blocks
IUIKitInteractionHandler = 'IUIKitInteractionHandler',
// Livechat
IPostLivechatRoomStarted = 'IPostLivechatRoomStarted',
IPostLivechatRoomClosed = 'IPostLivechatRoomClosed',
/**
* @deprecated please use the AppMethod.EXECUTE_POST_LIVECHAT_ROOM_CLOSED method
*/
ILivechatRoomClosedHandler = 'ILivechatRoomClosedHandler',
IPostLivechatAgentAssigned = 'IPostLivechatAgentAssigned',
IPostLivechatAgentUnassigned = 'IPostLivechatAgentUnassigned',
}
2 changes: 2 additions & 0 deletions src/definition/metadata/AppMethod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ export enum AppMethod {
EXECUTEPREROOMDELETEPREVENT = 'executePreRoomDeletePrevent',
CHECKPOSTROOMDELETED = 'checkPostRoomDeleted',
EXECUTEPOSTROOMDELETED = 'executePostRoomDeleted',
EXECUTE_PRE_ROOM_USER_JOINED = 'executePreRoomUserJoined',
EXECUTE_POST_ROOM_USER_JOINED = 'executePostRoomUserJoined',
// External Component handlers
EXECUTEPOSTEXTERNALCOMPONENTOPENED = 'executePostExternalComponentOpened',
EXECUTEPOSTEXTERNALCOMPONENTCLOSED = 'executePostExternalComponentClosed',
Expand Down
2 changes: 2 additions & 0 deletions src/definition/metadata/IAppInfo.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AppInterface } from './AppInterface';
import { IAppAuthorInfo } from './IAppAuthorInfo';

export interface IAppInfo {
Expand All @@ -12,4 +13,5 @@ export interface IAppInfo {
iconFile: string;
/** Base64 string of the App's icon. */
iconFileContent?: string;
essentials?: Array<AppInterface>;
}
2 changes: 2 additions & 0 deletions src/definition/metadata/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import { IAppAuthorInfo } from './IAppAuthorInfo';
import { IAppInfo } from './IAppInfo';
import { RocketChatAssociationModel, RocketChatAssociationRecord } from './RocketChatAssociations';

export * from './AppInterface';

export {
AppMethod,
IAppAuthorInfo,
Expand Down
19 changes: 19 additions & 0 deletions src/definition/rooms/IPostRoomUserJoined.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { IHttp, IPersistence, IRead } from '../accessors';
import { IRoomUserJoinedContext } from './IRoomUserJoinedContext';

/**
* Event interface that allows an app to
* register as a handler of the `IPostRoomUserJoined`
* event
*
* This event is triggered after an user succcessfully joined
* a room.
*
* This event does not allow an app to prevent any action from
* happening. For that, see its "pre counterpart(s)":
*
* - IPreRoomUserJoined
*/
export interface IPostRoomUserJoined {
executePostRoomUserJoined(context: IRoomUserJoinedContext, read: IRead, http: IHttp, persistence: IPersistence): Promise<void>;
}
18 changes: 18 additions & 0 deletions src/definition/rooms/IPreRoomUserJoined.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { IHttp, IPersistence, IRead } from '../accessors';
import { IRoomUserJoinedContext } from './IRoomUserJoinedContext';

/**
* Event interface that allows an app to
* register as a handler of the `IPreRoomUserJoined`
* event
*
* This event is triggered prior to an user succcessfully
* joining a room. To prevent the user from executing
* such action, an app should throw the `UserNotAllowedException`.
*
* This event is not triggered before a room has been created. For that,
* check the `IPreRoomCreate` events
*/
export interface IPreRoomUserJoined {
executePreRoomUserJoined(context: IRoomUserJoinedContext, read: IRead, http: IHttp, persistence: IPersistence): Promise<void>;
}
23 changes: 23 additions & 0 deletions src/definition/rooms/IRoomUserJoinedContext.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { IUser } from '../users';
import { IRoom } from './IRoom';

/**
* The context of execution for the following events:
* - IPreRoomUserJoined
* - IPostRoomUserJoined
*/
export interface IRoomUserJoinedContext {
/**
* The user that is being added to the room
*/
joiningUser: IUser;
/**
* The room to which the user is being added
*/
room: IRoom;
/**
* The user that has invited `joiningUser` to `room`,
* if any.
*/
inviter?: IUser;
}
4 changes: 4 additions & 0 deletions src/definition/rooms/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,7 @@ export {
IPreRoomCreatePrevent,
IPreRoomDeletePrevent,
};

export * from './IPreRoomUserJoined';
rodrigok marked this conversation as resolved.
Show resolved Hide resolved
export * from './IPostRoomUserJoined';
export * from './IRoomUserJoinedContext';
61 changes: 32 additions & 29 deletions src/server/AppManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,11 @@ export class AppManager {

// Now let's enable the apps which were once enabled
// but are not currently disabled.
for (const rl of this.apps.values()) {
if (!AppStatusUtils.isDisabled(rl.getStatus()) && AppStatusUtils.isEnabled(rl.getPreviousStatus())) {
await this.enableApp(items.get(rl.getID()), rl, true, rl.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error);
for (const app of this.apps.values()) {
if (!AppStatusUtils.isDisabled(app.getStatus()) && AppStatusUtils.isEnabled(app.getPreviousStatus())) {
await this.enableApp(items.get(app.getID()), app, true, app.getPreviousStatus() === AppStatus.MANUALLY_ENABLED).catch(console.error);
} else if (!AppStatusUtils.isError(app.getStatus())) {
this.listenerManager.lockEssentialEvents(app);
}
}

Expand All @@ -239,21 +241,18 @@ export class AppManager {
return;
}

for (const rl of this.apps.values()) {
if (AppStatusUtils.isDisabled(rl.getStatus())) {
continue;
}

if (rl.getStatus() === AppStatus.INITIALIZED) {
this.listenerManager.unregisterListeners(rl);
this.commandManager.unregisterCommands(rl.getID());
this.externalComponentManager.unregisterExternalComponents(rl.getID());
this.apiManager.unregisterApis(rl.getID());
this.accessorManager.purifyApp(rl.getID());
continue;
for (const app of this.apps.values()) {
if (app.getStatus() === AppStatus.INITIALIZED) {
this.listenerManager.unregisterListeners(app);
this.commandManager.unregisterCommands(app.getID());
this.externalComponentManager.unregisterExternalComponents(app.getID());
this.apiManager.unregisterApis(app.getID());
this.accessorManager.purifyApp(app.getID());
} else if (!AppStatusUtils.isDisabled(app.getStatus())) {
await this.disable(app.getID(), isManual ? AppStatus.MANUALLY_DISABLED : AppStatus.DISABLED);
}

await this.disable(rl.getID(), isManual ? AppStatus.MANUALLY_DISABLED : AppStatus.DISABLED);
this.listenerManager.releaseEssentialEvents(app);
}

// Remove all the apps from the system now that we have unloaded everything
Expand Down Expand Up @@ -350,33 +349,34 @@ export class AppManager {
throw new Error('Invalid disabled status');
}

const rl = this.apps.get(id);
const app = this.apps.get(id);

if (!rl) {
if (!app) {
throw new Error(`No App by the id "${id}" exists.`);
}

if (AppStatusUtils.isEnabled(rl.getStatus())) {
await rl.call(AppMethod.ONDISABLE, this.accessorManager.getConfigurationModify(rl.getID()))
if (AppStatusUtils.isEnabled(app.getStatus())) {
await app.call(AppMethod.ONDISABLE, this.accessorManager.getConfigurationModify(app.getID()))
.catch((e) => console.warn('Error while disabling:', e));
}

this.listenerManager.unregisterListeners(rl);
this.commandManager.unregisterCommands(rl.getID());
this.externalComponentManager.unregisterExternalComponents(rl.getID());
this.apiManager.unregisterApis(rl.getID());
this.accessorManager.purifyApp(rl.getID());
this.listenerManager.unregisterListeners(app);
this.listenerManager.lockEssentialEvents(app);
this.commandManager.unregisterCommands(app.getID());
this.externalComponentManager.unregisterExternalComponents(app.getID());
this.apiManager.unregisterApis(app.getID());
this.accessorManager.purifyApp(app.getID());

await rl.setStatus(status, silent);
await app.setStatus(status, silent);

const storageItem = await this.storage.retrieveOne(id);

rl.getStorageItem().marketplaceInfo = storageItem.marketplaceInfo;
await rl.validateLicense().catch();
app.getStorageItem().marketplaceInfo = storageItem.marketplaceInfo;
await app.validateLicense().catch();

// 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 = rl.getStatus();
storageItem.status = app.getStatus();
await this.storage.update(storageItem).catch();

return true;
Expand Down Expand Up @@ -462,6 +462,7 @@ export class AppManager {
}

this.listenerManager.unregisterListeners(app);
this.listenerManager.releaseEssentialEvents(app);
this.commandManager.unregisterCommands(app.getID());
this.externalComponentManager.purgeExternalComponents(app.getID());
this.apiManager.unregisterApis(app.getID());
Expand Down Expand Up @@ -796,10 +797,12 @@ export class AppManager {
this.externalComponentManager.registerExternalComponents(app.getID());
this.apiManager.registerApis(app.getID());
this.listenerManager.registerListeners(app);
this.listenerManager.releaseEssentialEvents(app);
} else {
this.commandManager.unregisterCommands(app.getID());
this.externalComponentManager.unregisterExternalComponents(app.getID());
this.apiManager.unregisterApis(app.getID());
this.listenerManager.lockEssentialEvents(app);
}

if (saveToDb) {
Expand Down
10 changes: 9 additions & 1 deletion src/server/ProxiedApp.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import * as vm from 'vm';

import { IAppAccessors, ILogger } from '../definition/accessors';
import { App } from '../definition/App';
import { AppStatus } from '../definition/AppStatus';
import { AppsEngineException } from '../definition/exceptions';
import { IApp } from '../definition/IApp';
import { AppMethod, IAppAuthorInfo, IAppInfo } from '../definition/metadata';
import { AppManager } from './AppManager';
Expand Down Expand Up @@ -88,6 +88,10 @@ export class ProxiedApp implements IApp {
} catch (e) {
logger.error(e);
logger.debug(`'${method}' was unsuccessful.`);

if (e instanceof AppsEngineException) {
throw e;
}
}

this.manager.getLogStorage().storeEntries(this.getID(), logger);
Expand Down Expand Up @@ -150,6 +154,10 @@ export class ProxiedApp implements IApp {
return this.app.getAccessors();
}

public getEssentials(): IAppInfo['essentials'] {
return this.getInfo().essentials;
}

public getLatestLicenseValidationResult(): AppLicenseValidationResult {
return this.latestLicenseValidationResult;
}
Expand Down
2 changes: 1 addition & 1 deletion src/server/bridges/IListenerBridge.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { IMessage } from '../../definition/messages';
import { AppInterface } from '../../definition/metadata';
import { IRoom } from '../../definition/rooms';
import { IUIKitIncomingInteraction } from '../../definition/uikit';
import { AppInterface } from '../compiler';

export interface IListenerBridge {
messageEvent(int: AppInterface, message: IMessage): Promise<void | boolean | IMessage>;
Expand Down
Loading