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

Fix : Number settings gets saved as strings in MongoDB #444

Merged
merged 10 commits into from
Dec 17, 2024
114 changes: 63 additions & 51 deletions api/src/setting/repositories/setting.repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,16 @@
*/

import { Injectable } from '@nestjs/common';
import {
EventEmitter2,
IHookSettingsGroupLabelOperationMap,
} from '@nestjs/event-emitter';
import { EventEmitter2 } from '@nestjs/event-emitter';
import { InjectModel } from '@nestjs/mongoose';
import { Document, Model, Query, Types } from 'mongoose';
import {
Document,
FilterQuery,
Model,
Types,
UpdateQuery,
UpdateWithAggregationPipeline,
} from 'mongoose';

import { I18nService } from '@/i18n/services/i18n.service';
import { BaseRepository } from '@/utils/generics/base-repository';
Expand All @@ -30,6 +34,27 @@ export class SettingRepository extends BaseRepository<Setting> {
super(eventEmitter, model, Setting);
}

async preCreateValidate(
doc: Document<unknown, unknown, Setting> &
Setting & { _id: Types.ObjectId },
) {
this.validateSettingValue(doc.type, doc.value);
}

async preUpdateValidate(
criteria: FilterQuery<Setting>,
updates: UpdateWithAggregationPipeline | UpdateQuery<Setting>,
): Promise<void> {
if (!Array.isArray(updates)) {
const payload = updates.$set;
if (typeof payload.value !== 'undefined') {
const { type } =
'type' in payload ? payload : await this.findOne(criteria);
this.validateSettingValue(type, payload.value);
}
}
}

/**
* Validates the `Setting` document after it has been retrieved.
*
Expand All @@ -40,64 +65,51 @@ export class SettingRepository extends BaseRepository<Setting> {
*
* @param setting The `Setting` document to be validated.
*/
async postValidate(
setting: Document<unknown, unknown, Setting> &
Setting & { _id: Types.ObjectId },
) {
private validateSettingValue(type: SettingType, value: any) {
if (
(setting.type === SettingType.text ||
setting.type === SettingType.textarea) &&
typeof setting.value !== 'string' &&
setting.value !== null
(type === SettingType.text || type === SettingType.textarea) &&
typeof value !== 'string' &&
value !== null
) {
throw new Error('Setting Model : Value must be a string!');
} else if (setting.type === SettingType.multiple_text) {
const isStringArray =
Array.isArray(setting.value) &&
setting.value.every((v) => {
return typeof v === 'string';
});
if (!isStringArray) {
throw new Error('Setting Model : Value must be a string array!');
} else if (type === SettingType.multiple_text) {
if (!this.isArrayOfString(value)) {
throw new Error(
'Setting Model (Multiple Text) : Value must be a string array!',
);
}
} else if (
setting.type === SettingType.checkbox &&
typeof setting.value !== 'boolean' &&
setting.value !== null
type === SettingType.checkbox &&
typeof value !== 'boolean' &&
value !== null
) {
throw new Error('Setting Model : Value must be a boolean!');
} else if (
setting.type === SettingType.number &&
typeof setting.value !== 'number' &&
setting.value !== null
type === SettingType.number &&
typeof value !== 'number' &&
value !== null
) {
throw new Error('Setting Model : Value must be a number!');
} else if (type === SettingType.multiple_attachment) {
if (!this.isArrayOfString(value)) {
throw new Error(
'Setting Model (Multiple Attachement): Value must be a string array!',
);
}
} else if (type === SettingType.attachment) {
if (typeof value !== 'string' && typeof value !== null) {
throw new Error(
'Setting Model (attachement): Value must be a string or null !',
);
}
} else if (type === SettingType.secret && typeof value !== 'string') {
throw new Error('Setting Model (secret) : Value must be a string');
} else if (type === SettingType.select && typeof value !== 'string') {
throw new Error('Setting Model (select): Value must be a string!');
}
}

/**
* Emits an event after a `Setting` has been updated.
*
* This method is used to synchronize global settings by emitting an event
* based on the `group` and `label` of the `Setting`.
*
* @param _query The Mongoose query object used to find and update the document.
* @param setting The updated `Setting` object.
*/
async postUpdate(
_query: Query<
Document<Setting, any, any>,
Document<Setting, any, any>,
unknown,
Setting,
'findOneAndUpdate'
>,
setting: Setting,
) {
const group = setting.group as keyof IHookSettingsGroupLabelOperationMap;
const label = setting.label as '*';

// Sync global settings var
this.eventEmitter.emit(`hook:${group}:${label}`, setting);
private isArrayOfString(value: any): boolean {
return Array.isArray(value) && value.every((v) => typeof v === 'string');
}
}
99 changes: 99 additions & 0 deletions api/src/utils/generics/base-repository.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,105 @@ describe('BaseRepository', () => {
expect.objectContaining({ dummy: 'updated dummy text' }),
);
});

it('should updateOne by id and trigger preUpdateValidate and postUpdateValidate methods', async () => {
const created = await dummyRepository.create({ dummy: 'initial text' });
const mockGetFilterValue = { _id: created.id };
const mockedGetFilter = jest.fn().mockReturnValue(mockGetFilterValue);

const mockGetUpdateValue = {
$set: {
value: 'updated dummy text',
},
};

const mockedGetUpdate = jest.fn().mockReturnValue(mockGetUpdateValue);
const mockQueryValue = {
getFilter: mockedGetFilter,
getUpdate: mockedGetUpdate,
lean: jest.fn(() => {
return {
exec: jest.fn(),
};
}),
};

jest
.spyOn(dummyModel, 'findOneAndUpdate')
.mockReturnValue(mockQueryValue as any);

const mockUpdate = { dummy: 'updated dummy text' };
const spyPreUpdateValidate = jest
.spyOn(dummyRepository, 'preUpdateValidate')
.mockResolvedValue();
const spyPostUpdateValidate = jest
.spyOn(dummyRepository, 'postUpdateValidate')
.mockResolvedValue();

const spyExecutoneOne = jest
.spyOn(
dummyRepository as DummyRepository & {
executeOne: () => Promise<{ dummy: string }>;
},
'executeOne',
)
.mockResolvedValue({ dummy: 'updated dummy text' });

await dummyRepository.updateOne(created.id, mockUpdate);

expect(spyPreUpdateValidate).toHaveBeenCalledWith(
mockGetFilterValue,
mockGetUpdateValue,
);
expect(spyPostUpdateValidate).toHaveBeenCalledWith(
mockGetFilterValue,
mockGetUpdateValue,
);
expect(spyExecutoneOne).toHaveBeenCalledWith(mockQueryValue, Dummy);
});

it('should throw an error while trying to updateOne when calling preUpdateValidate', async () => {
const created = await dummyRepository.create({ dummy: 'initial text' });
const mockGetFilterValue = { _id: created.id };
const mockedGetFilter = jest.fn().mockReturnValue(mockGetFilterValue);

const mockGetUpdateValue = {
$set: {
value: 10,
},
};

const mockedGetUpdate = jest.fn().mockReturnValue(mockGetUpdateValue);
const mockQueryValue = {
getFilter: mockedGetFilter,
getUpdate: mockedGetUpdate,
lean: jest.fn(() => {
return {
exec: jest.fn(),
};
}),
};

jest
.spyOn(dummyModel, 'findOneAndUpdate')
.mockReturnValue(mockQueryValue as any);

const mockUpdate = { dummy: 10 };
const spyPreUpdateValidate = jest
.spyOn(dummyRepository, 'preUpdateValidate')
.mockImplementation(() => {
throw new Error('Mocked error while validating dummy');
});

await expect(
dummyRepository.updateOne(created.id, mockUpdate),
).rejects.toThrow('Mocked error while validating dummy');

expect(spyPreUpdateValidate).toHaveBeenCalledWith(
mockGetFilterValue,
mockGetUpdateValue,
);
});
});

describe('deleteOne', () => {
Expand Down
69 changes: 61 additions & 8 deletions api/src/utils/generics/base-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
import { ClassTransformOptions, plainToClass } from 'class-transformer';
import {
Document,
FilterQuery,
FlattenMaps,
HydratedDocument,
Model,
Expand All @@ -38,18 +39,32 @@ export type DeleteResult = {
};

export enum EHook {
preCreateValidate = 'preCreateValidate',
preCreate = 'preCreate',
preUpdateValidate = 'preUpdateValidate',
preUpdate = 'preUpdate',
preUpdateMany = 'preUpdateMany',
preDelete = 'preDelete',
preValidate = 'preValidate',
postCreateValidate = 'postCreateValidate',
postCreate = 'postCreate',
postUpdateValidate = 'postUpdateValidate',
postUpdate = 'postUpdate',
postUpdateMany = 'postUpdateMany',
postDelete = 'postDelete',
postValidate = 'postValidate',
}

// ! ------------------------------------ Note --------------------------------------------
// Methods like `update()`, `updateOne()`, `updateMany()`, `findOneAndUpdate()`,
// `findByIdAndUpdate()`, `findOneAndReplace()`, `findOneAndDelete()`, and `findByIdAndDelete()`
// do not trigger Mongoose validation hooks by default. This is because these methods do not
// return Mongoose Documents but plain JavaScript objects (POJOs), which do not have Mongoose
// instance methods like `validate()` attached.
//
// Be cautious when using the `.lean()` function as well. It returns POJOs instead of Mongoose
// Documents, so methods and hooks like `validate()` will not be available when working with
// the returned data. If you need validation, ensure that you're working with a Mongoose Document
// or explicitly use `runValidators: true` in the options for update operations.

export abstract class BaseRepository<
T extends FlattenMaps<unknown>,
P extends string = never,
Expand Down Expand Up @@ -86,14 +101,17 @@ export abstract class BaseRepository<

hooks?.validate.pre.execute(async function () {
const doc = this as HydratedDocument<T>;
await repository.preValidate(doc);
repository.emitter.emit(repository.getEventName(EHook.preValidate), doc);
await repository.preCreateValidate(doc);
repository.emitter.emit(
repository.getEventName(EHook.preCreateValidate),
doc,
);
});

hooks?.validate.post.execute(async function (created: HydratedDocument<T>) {
await repository.postValidate(created);
await repository.postCreateValidate(created);
repository.emitter.emit(
repository.getEventName(EHook.postValidate),
repository.getEventName(EHook.postCreateValidate),
created,
);
});
Expand Down Expand Up @@ -457,6 +475,23 @@ export abstract class BaseRepository<
new: true,
},
);
const filterCriteria = query.getFilter();
const queryUpdates = query.getUpdate();

await this.preUpdateValidate(filterCriteria, queryUpdates);
this.emitter.emit(
this.getEventName(EHook.preUpdateValidate),
filterCriteria,
queryUpdates,
);

await this.postUpdateValidate(filterCriteria, queryUpdates);
this.emitter.emit(
this.getEventName(EHook.postUpdateValidate),
filterCriteria,
queryUpdates,
);

return await this.executeOne(query, this.cls);
}

Expand All @@ -479,11 +514,29 @@ export abstract class BaseRepository<
return await this.model.deleteMany(criteria);
}

async preValidate(_doc: HydratedDocument<T>): Promise<void> {
async preCreateValidate(
_doc: HydratedDocument<T>,
_filterCriteria?: FilterQuery<T>,
_updates?: UpdateWithAggregationPipeline | UpdateQuery<T>,
): Promise<void> {
// Nothing ...
}

async postCreateValidate(_validated: HydratedDocument<T>): Promise<void> {
// Nothing ...
}

async preUpdateValidate(
_filterCriteria: FilterQuery<T>,
_updates: UpdateWithAggregationPipeline | UpdateQuery<T>,
): Promise<void> {
// Nothing ...
}

async postValidate(_validated: HydratedDocument<T>): Promise<void> {
async postUpdateValidate(
_filterCriteria: FilterQuery<T>,
_updates: UpdateWithAggregationPipeline | UpdateQuery<T>,
): Promise<void> {
// Nothing ...
}

Expand Down
Loading
Loading