Skip to content

Commit

Permalink
Remove old Notification tokens (#2173)
Browse files Browse the repository at this point in the history
This PR ensures that duplicate push notifications are avoided by removing the registered push notification token from the TX service during device registration through the Client Gateway (CGW). This prevents multiple registrations from causing redundant notifications.
  • Loading branch information
PooyaRaki authored Dec 5, 2024
1 parent e455fba commit 530e230
Show file tree
Hide file tree
Showing 4 changed files with 217 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { notificationDeviceBuilder } from '@/datasources/notifications/entities/
import { NotificationType as NotificationTypeEnum } from '@/domain/notifications/v2/entities/notification.entity';
import { DatabaseMigrator } from '@/datasources/db/v2/database-migrator.service';
import type { ConfigService } from '@nestjs/config';
import { NotFoundException } from '@nestjs/common';

describe('NotificationsRepositoryV2', () => {
const mockLoggingService = {
Expand Down Expand Up @@ -527,6 +528,34 @@ describe('NotificationsRepositoryV2', () => {
expect(subscriptionAfterRemoval).toHaveLength(0);
});

it('Should throw NotFoundException if a subscription does not exist', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const authPayload = new AuthPayload(authPayloadDto);
const upsertSubscriptionsDto = upsertSubscriptionsDtoBuilder().build();

const notificationSubscriptionRepository = dataSource.getRepository(
NotificationSubscription,
);
jest.spyOn(notificationSubscriptionRepository, 'remove');

const subscription = await notificationSubscriptionRepository.findBy({
safe_address: upsertSubscriptionsDto.safes[0].address,
chain_id: upsertSubscriptionsDto.safes[0].chainId,
signer_address: authPayload.signer_address,
});

const result = notificationsRepositoryService.deleteSubscription({
deviceUuid: upsertSubscriptionsDto.deviceUuid as UUID,
chainId: upsertSubscriptionsDto.safes[0].chainId,
safeAddress: upsertSubscriptionsDto.safes[0].address,
});

await expect(result).rejects.toThrow(
new NotFoundException('No Subscription Found!'),
);
expect(subscription).toHaveLength(0);
});

it('Should not try to remove if a subscription does not exist', async () => {
const authPayloadDto = authPayloadDtoBuilder().build();
const authPayload = new AuthPayload(authPayloadDto);
Expand All @@ -543,12 +572,13 @@ describe('NotificationsRepositoryV2', () => {
signer_address: authPayload.signer_address,
});

await notificationsRepositoryService.deleteSubscription({
const result = notificationsRepositoryService.deleteSubscription({
deviceUuid: upsertSubscriptionsDto.deviceUuid as UUID,
chainId: upsertSubscriptionsDto.safes[0].chainId,
safeAddress: upsertSubscriptionsDto.safes[0].address,
});

await expect(result).rejects.toThrow();
expect(subscription).toHaveLength(0);
expect(notificationSubscriptionRepository.remove).not.toHaveBeenCalled();
});
Expand All @@ -573,24 +603,18 @@ describe('NotificationsRepositoryV2', () => {
expect(findDevice).toBeNull();
});

it('Should not throw if a uuid does not exist', async () => {
it('Should throw NotFoundException if a uuid does not exist', async () => {
const deviceDto = notificationDeviceBuilder()
.with('id', faker.number.int({ min: 1, max: 1999 }))
.build();

const notificationDeviceRepository =
dataSource.getRepository(NotificationDevice);

const result = await notificationsRepositoryService.deleteDevice(
const result = notificationsRepositoryService.deleteDevice(
deviceDto.device_uuid,
);

const findDevice = await notificationDeviceRepository.findOneBy({
device_uuid: deviceDto.device_uuid,
});

expect(findDevice).toBeNull();
expect(result).toBeUndefined();
await expect(result).rejects.toThrow(
new NotFoundException('No Device Found!'),
);
});

it('Should delete a device with its subscription', async () => {
Expand Down
63 changes: 61 additions & 2 deletions src/domain/notifications/v2/notifications.repository.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { UUID } from 'crypto';
import { faker } from '@faker-js/faker/.';
import { UnauthorizedException } from '@nestjs/common';
import { NotFoundException, UnauthorizedException } from '@nestjs/common';
import { type ILoggingService } from '@/logging/logging.interface';
import { AuthPayload } from '@/domain/auth/entities/auth-payload.entity';
import { NotificationsRepositoryV2 } from '@/domain/notifications/v2/notifications.repository';
Expand All @@ -16,6 +16,7 @@ import { NotificationDevice } from '@/datasources/notifications/entities/notific
import { mockEntityManager } from '@/datasources/db/v2/__tests__/entity-manager.mock';
import { mockPostgresDatabaseService } from '@/datasources/db/v2/__tests__/postgresql-database.service.mock';
import { mockRepository } from '@/datasources/db/v2/__tests__/repository.mock';
import { getAddress } from 'viem';

describe('NotificationsRepositoryV2', () => {
let notificationsRepository: INotificationsRepositoryV2;
Expand Down Expand Up @@ -395,8 +396,9 @@ describe('NotificationsRepositoryV2', () => {
safeAddress: faker.string.hexadecimal({ length: 32 }) as `0x${string}`,
};

await notificationsRepository.deleteSubscription(args);
const result = notificationsRepository.deleteSubscription(args);

await expect(result).rejects.toThrow();
expect(notificationSubscriptionRepository.findOne).toHaveBeenCalledTimes(
1,
);
Expand All @@ -411,10 +413,45 @@ describe('NotificationsRepositoryV2', () => {
});
expect(notificationSubscriptionRepository.remove).not.toHaveBeenCalled();
});

it('Should throw NotFoundException if no subscription is found', async () => {
notificationSubscriptionRepository.findOne.mockResolvedValue(null);
mockPostgresDatabaseService.getRepository.mockResolvedValue(
notificationSubscriptionRepository,
);

const args = {
deviceUuid: faker.string.uuid() as UUID,
chainId: faker.number.int({ min: 0 }).toString(),
safeAddress: getAddress(faker.finance.ethereumAddress()),
};

const result = notificationsRepository.deleteSubscription(args);

await expect(result).rejects.toThrow(
new NotFoundException('No Subscription Found!'),
);
expect(notificationSubscriptionRepository.findOne).toHaveBeenCalledTimes(
1,
);
expect(notificationSubscriptionRepository.findOne).toHaveBeenCalledWith({
where: {
chain_id: args.chainId,
safe_address: args.safeAddress,
push_notification_device: {
device_uuid: args.deviceUuid,
},
},
});
});
});

describe('deleteDevice()', () => {
it('Should delete a device successfully', async () => {
notificationDeviceRepository.delete.mockResolvedValue({
affected: 1,
raw: [],
});
mockPostgresDatabaseService.getRepository.mockResolvedValue(
notificationDeviceRepository,
);
Expand All @@ -428,5 +465,27 @@ describe('NotificationsRepositoryV2', () => {
device_uuid: deviceUuid,
});
});

it('Should throw if a device uuid does not exist', async () => {
notificationDeviceRepository.delete.mockResolvedValue({
affected: 0,
raw: [],
});
mockPostgresDatabaseService.getRepository.mockResolvedValue(
notificationDeviceRepository,
);

const deviceUuid = faker.string.uuid() as UUID;

const result = notificationsRepository.deleteDevice(deviceUuid);

await expect(result).rejects.toThrow(
new NotFoundException('No Device Found!'),
);
expect(notificationDeviceRepository.delete).toHaveBeenCalled();
expect(notificationDeviceRepository.delete).toHaveBeenCalledWith({
device_uuid: deviceUuid,
});
});
});
});
13 changes: 10 additions & 3 deletions src/domain/notifications/v2/notifications.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { INotificationsRepositoryV2 } from '@/domain/notifications/v2/notif
import {
Inject,
Injectable,
NotFoundException,
UnauthorizedException,
UnprocessableEntityException,
} from '@nestjs/common';
Expand Down Expand Up @@ -368,9 +369,11 @@ export class NotificationsRepositoryV2 implements INotificationsRepositoryV2 {
},
});

if (subscription) {
await notificationsSubscriptionsRepository.remove(subscription);
if (!subscription) {
throw new NotFoundException('No Subscription Found!');
}

await notificationsSubscriptionsRepository.remove(subscription);
}

public async deleteDevice(deviceUuid: UUID): Promise<void> {
Expand All @@ -379,8 +382,12 @@ export class NotificationsRepositoryV2 implements INotificationsRepositoryV2 {
NotificationDevice,
);

await notificationsDeviceRepository.delete({
const deleteResult = await notificationsDeviceRepository.delete({
device_uuid: deviceUuid,
});

if (!deleteResult.affected) {
throw new NotFoundException('No Device Found!');
}
}
}
Loading

0 comments on commit 530e230

Please sign in to comment.