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

feat: cache notification template by trigger identifier #3074

Merged
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ChangeEntityTypeEnum } from '@novu/shared';
import { ApplyChange, ApplyChangeCommand } from '../apply-change';
import { PromoteTypeChangeCommand } from '../promote-type-change.command';
import { InvalidateCacheService } from '../../../shared/services/cache';
import { entityBuilder } from '../../../shared/services/cache/keys';
import { notificationTemplateQueryKeyBuild } from '../../../shared/services/cache/keys';

@Injectable()
export class PromoteNotificationTemplateChange {
Expand Down Expand Up @@ -136,9 +136,8 @@ export class PromoteNotificationTemplateChange {
return;
}

await this.invalidateCache.invalidateByKey({
key: entityBuilder().notificationTemplate({
_id: item._id,
await this.invalidateCache.invalidateQuery({
key: notificationTemplateQueryKeyBuild().invalidate({
_environmentId: command.environmentId,
}),
});
Expand Down
7 changes: 3 additions & 4 deletions apps/api/src/app/events/e2e/process-subscriber.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import axios from 'axios';
import { ChannelTypeEnum, ISubscribersDefine, StepTypeEnum } from '@novu/shared';
import { UpdateSubscriberPreferenceRequestDto } from '../../widgets/dtos/update-subscriber-preference-request.dto';
import { CacheService, InvalidateCacheService } from '../../shared/services/cache';
import { entityBuilder } from '../../shared/services/cache/keys';
import { notificationTemplateQueryKeyBuild } from '../../shared/services/cache/keys';

const axiosInstance = axios.create();

Expand Down Expand Up @@ -194,9 +194,8 @@ describe('Trigger event - process subscriber /v1/events/trigger (POST)', functio

await updateSubscriberPreference(updateData, session.subscriberToken, template._id);

await invalidateCache.invalidateByKey({
key: entityBuilder().notificationTemplate({
_id: template._id,
await invalidateCache.invalidateQuery({
key: notificationTemplateQueryKeyBuild().invalidate({
_environmentId: session.environment._id,
}),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@ import { v4 as uuidv4 } from 'uuid';

import { ANALYTICS_SERVICE } from '../../../shared/shared.module';
import { ApiException } from '../../../shared/exceptions/api.exception';
import { VerifyPayload } from '../verify-payload/verify-payload.usecase';
import { VerifyPayloadCommand } from '../verify-payload/verify-payload.command';
import { VerifyPayload, VerifyPayloadCommand } from '../verify-payload';
import { StorageHelperService } from '../../services/storage-helper-service/storage-helper.service';
import { ParseEventRequestCommand } from './parse-event-request.command';
import { TriggerHandlerQueueService } from '../../services/workflow-queue/trigger-handler-queue.service';
import { MapTriggerRecipients, MapTriggerRecipientsCommand } from '../map-trigger-recipients';
import { CachedQuery } from '../../../shared/interceptors/cached-query.interceptor';
import { notificationTemplateQueryKeyBuild } from '../../../shared/services/cache/keys';

@Injectable()
export class ParseEventRequest {
Expand Down Expand Up @@ -48,10 +49,10 @@ export class ParseEventRequest {

await this.validateSubscriberIdProperty(mappedRecipients);

const template = await this.notificationTemplateRepository.findByTriggerIdentifier(
command.environmentId,
command.identifier
);
const template = await this.getNotificationTemplateByTriggerIdentifier({
environmentId: command.environmentId,
triggerIdentifier: command.identifier,
});

if (!template) {
throw new UnprocessableEntityException('template_not_found');
Expand Down Expand Up @@ -136,6 +137,23 @@ export class ParseEventRequest {
};
}

@CachedQuery({
builder: (command: { triggerIdentifier: string; environmentId: string }) =>
notificationTemplateQueryKeyBuild().cache({
_environmentId: command.environmentId,
identifiers: { triggerIdentifier: command.triggerIdentifier },
}),
})
private async getNotificationTemplateByTriggerIdentifier(command: {
triggerIdentifier: string;
environmentId: string;
}) {
return await this.notificationTemplateRepository.findByTriggerIdentifier(
command.environmentId,
command.triggerIdentifier
);
}

private async validateSubscriberIdProperty(to: ISubscribersDefine[]): Promise<boolean> {
for (const subscriber of to) {
const subscriberIdExists = typeof subscriber === 'string' ? subscriber : subscriber.subscriberId;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,13 @@ import {
import { ANALYTICS_SERVICE } from '../../../shared/shared.module';
import { ApiException } from '../../../shared/exceptions/api.exception';
import { CachedEntity } from '../../../shared/interceptors/cached-entity.interceptor';
import { buildCommonKey, CacheKeyPrefixEnum, CacheKeyTypeEnum } from '../../../shared/services/cache/keys';
import {
buildCommonKey,
CacheKeyPrefixEnum,
CacheKeyTypeEnum,
notificationTemplateQueryKeyBuild,
} from '../../../shared/services/cache/keys';
import { CachedQuery } from '../../../shared/interceptors/cached-query.interceptor';

@Injectable()
export class SendMessage {
Expand Down Expand Up @@ -233,13 +239,11 @@ export class SendMessage {
return result || template.critical;
}

@CachedEntity({
@CachedQuery({
builder: (command: { _id: string; environmentId: string }) =>
buildCommonKey({
type: CacheKeyTypeEnum.ENTITY,
keyEntity: CacheKeyPrefixEnum.NOTIFICATION_TEMPLATE,
environmentId: command.environmentId,
identifier: command._id,
notificationTemplateQueryKeyBuild().cache({
_environmentId: command.environmentId,
identifiers: { id: command._id },
}),
})
private async getNotificationTemplate({ _id, environmentId }: { _id: string; environmentId: string }) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import { ApiException } from '../../../shared/exceptions/api.exception';
const LOG_CONTEXT = 'TriggerEventUseCase';

import { PinoLogger } from '@novu/application-generic';
import { notificationTemplateQueryKeyBuild } from '../../../shared/services/cache/keys';
import { CachedQuery } from '../../../shared/interceptors/cached-query.interceptor';

@Injectable()
export class TriggerEvent {
Expand Down Expand Up @@ -54,10 +56,10 @@ export class TriggerEvent {
organizationId: command.organizationId,
});

const template = await this.notificationTemplateRepository.findByTriggerIdentifier(
command.environmentId,
command.identifier
);
const template = await this.getNotificationTemplateByTriggerIdentifier({
environmentId: command.environmentId,
triggerIdentifier: command.identifier,
});

/*
* Makes no sense to execute anything if template doesn't exist
Expand Down Expand Up @@ -136,6 +138,23 @@ export class TriggerEvent {
this.performanceService.setEnd(mark);
}

@CachedQuery({
builder: (command: { triggerIdentifier: string; environmentId: string }) =>
notificationTemplateQueryKeyBuild().cache({
_environmentId: command.environmentId,
identifiers: { triggerIdentifier: command.triggerIdentifier },
}),
})
private async getNotificationTemplateByTriggerIdentifier(command: {
triggerIdentifier: string;
environmentId: string;
}) {
return await this.notificationTemplateRepository.findByTriggerIdentifier(
command.environmentId,
command.triggerIdentifier
);
}

private async validateTransactionIdProperty(
transactionId: string,
organizationId: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ChangeEntityTypeEnum } from '@novu/shared';
import { ChangeTemplateActiveStatusCommand } from './change-template-active-status.command';
import { CreateChange, CreateChangeCommand } from '../../../change/usecases';
import { InvalidateCacheService } from '../../../shared/services/cache';
import { entityBuilder } from '../../../shared/services/cache/keys';
import { notificationTemplateQueryKeyBuild } from '../../../shared/services/cache/keys';

@Injectable()
export class ChangeTemplateActiveStatus {
Expand All @@ -29,9 +29,8 @@ export class ChangeTemplateActiveStatus {
throw new BadRequestException('You must provide a different status from the current status');
}

await this.invalidateCache.invalidateByKey({
key: entityBuilder().notificationTemplate({
_id: command.templateId,
await this.invalidateCache.invalidateQuery({
key: notificationTemplateQueryKeyBuild().invalidate({
_environmentId: command.environmentId,
}),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { ANALYTICS_SERVICE } from '../../../shared/shared.module';
import { CacheService, InvalidateCacheService } from '../../../shared/services/cache';
import { ApiException } from '../../../shared/exceptions/api.exception';
import { NotificationStep } from '../../../shared/dtos/notification-step';
import { entityBuilder } from '../../../shared/services/cache/keys';
import { notificationTemplateQueryKeyBuild } from '../../../shared/services/cache/keys';

@Injectable()
export class UpdateNotificationTemplate {
Expand All @@ -38,9 +38,8 @@ export class UpdateNotificationTemplate {
) {}

async execute(command: UpdateNotificationTemplateCommand): Promise<NotificationTemplateEntity> {
await this.invalidateCache.invalidateByKey({
key: entityBuilder().notificationTemplate({
_id: command.id,
await this.invalidateCache.invalidateQuery({
key: notificationTemplateQueryKeyBuild().invalidate({
_environmentId: command.environmentId,
}),
});
Expand Down
83 changes: 74 additions & 9 deletions apps/api/src/app/shared/services/cache/keys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const queryBuilder = () => {
};

export const entityBuilder = () => {
return { subscriber, integration, notificationTemplate, user, environmentByApiKey };
return { subscriber, integration, user, environmentByApiKey };
};

export const buildQueryKey = ({
Expand Down Expand Up @@ -121,14 +121,6 @@ const integration = ({ _id, _environmentId }: { _id: string; _environmentId: str
identifier: _id,
});

const notificationTemplate = ({ _id, _environmentId }: { _id: string; _environmentId: string }): string =>
buildCommonKey({
type: CacheKeyTypeEnum.ENTITY,
keyEntity: CacheKeyPrefixEnum.NOTIFICATION_TEMPLATE,
environmentId: _environmentId,
identifier: _id,
});

const user = ({ _id }: { _id: string }): string =>
buildKeyById({
type: CacheKeyTypeEnum.ENTITY,
Expand Down Expand Up @@ -170,3 +162,76 @@ export const buildKeyById = ({
identifierPrefix?: string;
identifier: string;
}): string => `${type}:${keyEntity}:${identifierPrefix}=${identifier}`;

export const notificationTemplateQueryKeyBuild = () => {
const cache = ({
_environmentId,
identifiers,
}: {
_environmentId: string;
identifiers: ({ id: string } & { triggerIdentifier?: string }) | ({ id?: string } & { triggerIdentifier: string });
Copy link
Contributor

Choose a reason for hiding this comment

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

I have been trying to find a way to be able to express mutually exclusive optional properties but I haven't been able to found it. Not sure if something along these lines would be possible but changed to make the other property optional.
microsoft/TypeScript#14094 (comment)

}): string =>
buildQueryKeyByEnvironment({
type: CacheKeyTypeEnum.QUERY,
keyEntity: CacheKeyPrefixEnum.NOTIFICATION_TEMPLATE,
environmentId: _environmentId,
query: identifiers as unknown as Record<string, unknown>,
});

const invalidate = ({ _environmentId }: { _environmentId: string }): string =>
buildKeyByEnvironment({
type: CacheKeyTypeEnum.QUERY,
keyEntity: CacheKeyPrefixEnum.NOTIFICATION_TEMPLATE,
environmentId: _environmentId,
});

return {
cache,
invalidate,
};
};

/*
* const notificationTemplate = ({ _id, _environmentId }: { _id: string; _environmentId: string }): string =>
* buildCommonKey({
* type: CacheKeyTypeEnum.ENTITY,
* keyEntity: CacheKeyPrefixEnum.NOTIFICATION_TEMPLATE,
* environmentId: _environmentId,
* identifier: _id,
* });
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

🗑️


export const buildQueryKeyByEnvironment = ({
type,
keyEntity,
environmentIdPrefix = 'e',
environmentId,
query,
}: {
type: CacheKeyTypeEnum;
keyEntity: CacheKeyPrefixEnum;
environmentIdPrefix?: string;
environmentId: string;
query: Record<string, unknown>;
}): string => {
const keyBase = buildKeyByEnvironment({
type,
keyEntity,
environmentIdPrefix,
environmentId,
});

return `${keyBase}:${QUERY_PREFIX}=${JSON.stringify(query)}`;
};

export const buildKeyByEnvironment = ({
type,
keyEntity,
environmentIdPrefix = 'e',
environmentId,
}: {
type: CacheKeyTypeEnum;
keyEntity: CacheKeyPrefixEnum;
environmentIdPrefix?: string;
environmentId: string;
}): string => `${type}:${keyEntity}:${environmentIdPrefix}=${environmentId}`;