fix: enhance code

This commit is contained in:
hexastack 2024-11-20 19:41:11 +01:00
parent 5f73fecf69
commit 286beee5e6
3 changed files with 137 additions and 252 deletions

View File

@ -9,7 +9,7 @@
import { EventEmitter2 } from '@nestjs/event-emitter'; import { EventEmitter2 } from '@nestjs/event-emitter';
import { MongooseModule, getModelToken } from '@nestjs/mongoose'; import { MongooseModule, getModelToken } from '@nestjs/mongoose';
import { Test } from '@nestjs/testing'; import { Test } from '@nestjs/testing';
import mongoose, { Model } from 'mongoose'; import { Model } from 'mongoose';
import { import {
blockFixtures, blockFixtures,
@ -36,7 +36,6 @@ describe('BlockRepository', () => {
let hasNextBlocks: Block; let hasNextBlocks: Block;
let validIds: string[]; let validIds: string[];
let validCategory: string; let validCategory: string;
let objCategory: mongoose.Types.ObjectId;
beforeAll(async () => { beforeAll(async () => {
const module = await Test.createTestingModule({ const module = await Test.createTestingModule({
@ -51,7 +50,6 @@ describe('BlockRepository', () => {
blockModel = module.get<Model<Block>>(getModelToken('Block')); blockModel = module.get<Model<Block>>(getModelToken('Block'));
validIds = ['64abc1234def567890fedcba', '64abc1234def567890fedcbc']; validIds = ['64abc1234def567890fedcba', '64abc1234def567890fedcbc'];
validCategory = '64def5678abc123490fedcba'; validCategory = '64def5678abc123490fedcba';
objCategory = new mongoose.Types.ObjectId('64def5678abc123490fedcba');
category = await categoryRepository.findOne({ label: 'default' }); category = await categoryRepository.findOne({ label: 'default' });
hasPreviousBlocks = await blockRepository.findOne({ hasPreviousBlocks = await blockRepository.findOne({
@ -117,269 +115,161 @@ describe('BlockRepository', () => {
}); });
describe('preUpdate', () => { describe('preUpdate', () => {
it('should update blocks referencing the moved block when category is updated with valid criteria', async () => { it('should remove references to a moved block when updating category', async () => {
const mockUpdateMany = jest.spyOn(blockModel, 'updateMany'); const mockUpdateMany = jest.spyOn(blockRepository, 'updateMany');
const criteria = { _id: validIds[0] };
const updates = { $set: { category: validCategory } };
const criteria = { _id: hasPreviousBlocks.id }; const mockFindOne = jest
const updates = { $set: { category: 'newCategory' } }; .spyOn(blockRepository, 'findOne')
const mockQuery = {} as any; .mockResolvedValue({
id: validIds[0],
category: 'oldCategory',
} as Block);
await blockRepository.preUpdate(mockQuery, criteria, updates); await blockRepository.preUpdate({} as any, criteria, updates);
expect(mockFindOne).toHaveBeenCalledWith(criteria);
expect(mockUpdateMany).toHaveBeenCalledTimes(2);
expect(mockUpdateMany).toHaveBeenNthCalledWith( expect(mockUpdateMany).toHaveBeenNthCalledWith(
1, 1,
{ nextBlocks: hasPreviousBlocks.id }, { nextBlocks: validIds[0] },
{ $pull: { nextBlocks: hasPreviousBlocks.id } }, { $pull: { nextBlocks: validIds[0] } },
); );
expect(mockUpdateMany).toHaveBeenNthCalledWith( expect(mockUpdateMany).toHaveBeenNthCalledWith(
2, 2,
{ attachedBlock: hasPreviousBlocks.id }, { attachedBlock: validIds[0] },
{ $set: { attachedBlock: null } }, { $set: { attachedBlock: null } },
); );
}); });
it('should throw an error if category is updated without a valid _id in criteria', async () => { it('should do nothing if no block is found for the criteria', async () => {
const updates = { $set: { category: 'newCategory' } }; jest.spyOn(blockRepository, 'findOne').mockResolvedValue(null);
const mockQuery = {} as any; const mockUpdateMany = jest.spyOn(blockRepository, 'updateMany');
await expect( await blockRepository.preUpdate({} as any, { _id: 'nonexistent' }, {});
blockRepository.preUpdate(mockQuery, {}, updates),
).rejects.toThrowError(
'Criteria must include a valid id to update category.',
);
});
it('should call checkDeprecatedAttachmentUrl with the correct update object', async () => {
const mockCheckDeprecatedAttachmentUrl = jest.spyOn(
blockRepository,
'checkDeprecatedAttachmentUrl',
);
const criteria = { _id: hasPreviousBlocks.id };
const updates = {
$set: { category: 'newCategory', attachedBlock: 'someUrl' },
};
const mockQuery = {} as any;
await blockRepository.preUpdate(mockQuery, criteria, updates);
expect(mockCheckDeprecatedAttachmentUrl).toHaveBeenCalledWith(
updates.$set,
);
});
it('should not call updateMany if no category update is provided', async () => {
const mockUpdateMany = jest.spyOn(blockModel, 'updateMany');
const criteria = { _id: hasPreviousBlocks.id };
const updates = { $set: { name: 'newName' } };
const mockQuery = {} as any;
await blockRepository.preUpdate(mockQuery, criteria, updates);
expect(mockUpdateMany).not.toHaveBeenCalled(); expect(mockUpdateMany).not.toHaveBeenCalled();
}); });
}); });
describe('mapIdsAndCategory', () => {
it('should map string IDs and category to Mongoose ObjectIDs', async () => {
const result = blockRepository.mapIdsAndCategory(validIds, validCategory);
expect(result.objIds).toHaveLength(validIds.length);
validIds.forEach((id, index) => {
expect(result.objIds[index].toHexString()).toBe(id);
});
expect(result.objCategory.toHexString()).toBe(validCategory);
});
it('should throw an error if invalid IDs or category are provided', () => {
const ids = ['invalidId', '64xyz6789abc1234567defca'];
const category = 'invalidCategory';
expect(() =>
blockRepository.mapIdsAndCategory(ids, category),
).toThrowError(
'input must be a 24 character hex string, 12 byte Uint8Array, or an integer',
);
});
});
describe('updateBlocksInScope', () => { describe('updateBlocksInScope', () => {
it('should update blocks within the scope', async () => { it('should update blocks within the scope based on category and ids', async () => {
const mockFindOne = jest.spyOn(blockModel, 'findOne').mockResolvedValue({ jest.spyOn(blockRepository, 'findOne').mockResolvedValue({
_id: validIds[0], id: validIds[0],
category: new mongoose.Types.ObjectId('64abc1234def567890fedcbc'), category: 'oldCategory',
nextBlocks: [new mongoose.Types.ObjectId(validIds[1])], nextBlocks: [validIds[1]],
attachedBlock: new mongoose.Types.ObjectId(validIds[1]), attachedBlock: validIds[1],
} as Block);
const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');
await blockRepository.updateBlocksInScope(validCategory, validIds);
expect(mockUpdateOne).toHaveBeenCalledWith(validIds[0], {
nextBlocks: [validIds[1]],
attachedBlock: validIds[1],
}); });
const mockUpdateOne = jest.spyOn(blockModel, 'updateOne');
await blockRepository.updateBlocksInScope(objCategory, validIds);
expect(mockFindOne).toHaveBeenCalledWith({
_id: new mongoose.Types.ObjectId(validIds[0]),
});
expect(mockUpdateOne).toHaveBeenCalledWith(
{ _id: new mongoose.Types.ObjectId(validIds[0]) },
{
nextBlocks: [new mongoose.Types.ObjectId(validIds[1])],
attachedBlock: new mongoose.Types.ObjectId(validIds[1]),
},
);
}); });
it('should not update blocks if category matches', async () => { it('should not update blocks if the category already matches', async () => {
jest.spyOn(blockModel, 'findOne').mockResolvedValue({ jest.spyOn(blockRepository, 'findOne').mockResolvedValue({
_id: validIds[0], id: validIds[0],
category: objCategory, category: validCategory,
nextBlocks: [], nextBlocks: [],
attachedBlock: null, attachedBlock: null,
}); } as Block);
const mockUpdateOne = jest.spyOn(blockModel, 'updateOne'); const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');
await blockRepository.updateBlocksInScope(objCategory, validIds); await blockRepository.updateBlocksInScope(validCategory, validIds);
expect(mockUpdateOne).not.toHaveBeenCalled(); expect(mockUpdateOne).not.toHaveBeenCalled();
}); });
}); });
describe('updateExternalBlocks', () => { describe('updateExternalBlocks', () => {
let validIds: string[]; it('should update blocks outside the scope by removing references from attachedBlock', async () => {
beforeAll(() => {
validIds = ['64abc1234def567890fedcba', '64def5678abc123490fedcbc'];
});
it('should update external blocks with attachedBlock or nextBlocks', async () => {
const otherBlocks = [ const otherBlocks = [
{ {
id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), id: '64abc1234def567890fedcab',
attachedBlock: new mongoose.Types.ObjectId(validIds[0]), attachedBlock: validIds[0],
nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], nextBlocks: [validIds[0]],
}, },
]; ] as Block[];
const mockUpdateOne = jest.spyOn(blockModel, 'updateOne');
await blockRepository.updateExternalBlocks(otherBlocks, [ const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');
new mongoose.Types.ObjectId(validIds[0]),
]);
expect(mockUpdateOne).toHaveBeenCalledWith( await blockRepository.updateExternalBlocks(otherBlocks, validIds);
{ _id: otherBlocks[0].id },
{ attachedBlock: null },
);
expect(mockUpdateOne).toHaveBeenCalledWith( expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', {
{ _id: otherBlocks[0].id }, attachedBlock: null,
{ nextBlocks: [] }, });
);
}); });
it('should not update if no changes are necessary', async () => { it('should update blocks outside the scope by removing references from nextBlocks', async () => {
const otherBlocks = [ const otherBlocks = [
{ {
id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), id: '64abc1234def567890fedcab',
attachedBlock: null, attachedBlock: null,
nextBlocks: [], nextBlocks: [validIds[0], validIds[1]],
}, },
]; ] as Block[];
const mockUpdateOne = jest.spyOn(blockModel, 'updateOne');
await blockRepository.updateExternalBlocks(otherBlocks, [ const mockUpdateOne = jest.spyOn(blockRepository, 'updateOne');
new mongoose.Types.ObjectId(validIds[0]),
]);
expect(mockUpdateOne).not.toHaveBeenCalled(); await blockRepository.updateExternalBlocks(otherBlocks, [validIds[0]]);
expect(mockUpdateOne).toHaveBeenCalledWith('64abc1234def567890fedcab', {
$pull: { nextBlocks: [validIds[1]] },
});
}); });
}); });
describe('preUpdateMany', () => { describe('preUpdateMany', () => {
let validIds: string[]; it('should update blocks in and out of the scope', async () => {
let objCategory: mongoose.Types.ObjectId; const mockFind = jest.spyOn(blockRepository, 'find').mockResolvedValue([
beforeAll(() => {
validIds = ['64abc1234def567890fedcba', '64def5678abc123490fedcbc'];
objCategory = new mongoose.Types.ObjectId('64def5678abc123490fedcbc');
});
it('should map IDs, find other blocks, and update blocks in scope and external blocks', async () => {
const mockMapIdsAndCategory = jest
.spyOn(blockRepository, 'mapIdsAndCategory')
.mockReturnValue({
objIds: validIds.map((id) => new mongoose.Types.ObjectId(id)),
objCategory,
});
const mockFind = jest.spyOn(blockModel, 'find').mockResolvedValue([
{ {
id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), id: '64abc1234def567890fedcab',
attachedBlock: new mongoose.Types.ObjectId(validIds[0]), attachedBlock: validIds[0],
nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], nextBlocks: [validIds[0]],
}, },
]); ] as Block[]);
const mockUpdateBlocksInScope = jest const mockUpdateBlocksInScope = jest.spyOn(
.spyOn(blockRepository, 'updateBlocksInScope') blockRepository,
.mockResolvedValue(undefined); 'updateBlocksInScope',
);
const mockUpdateExternalBlocks = jest const mockUpdateExternalBlocks = jest.spyOn(
.spyOn(blockRepository, 'updateExternalBlocks') blockRepository,
.mockResolvedValue(undefined); 'updateExternalBlocks',
);
await blockRepository.preUpdateMany( await blockRepository.preUpdateMany(
{} as any, {} as any,
{ _id: { $in: validIds } }, { _id: { $in: validIds } },
{ $set: { category: objCategory.toHexString() } }, { $set: { category: validCategory } },
); );
expect(mockMapIdsAndCategory).toHaveBeenCalledWith( expect(mockFind).toHaveBeenCalled();
validIds,
objCategory.toHexString(),
);
expect(mockFind).toHaveBeenCalledWith({
_id: { $nin: validIds.map((id) => new mongoose.Types.ObjectId(id)) },
category: { $ne: objCategory },
$or: [
{
attachedBlock: {
$in: validIds.map((id) => new mongoose.Types.ObjectId(id)),
},
},
{
nextBlocks: {
$in: validIds.map((id) => new mongoose.Types.ObjectId(id)),
},
},
],
});
expect(mockUpdateBlocksInScope).toHaveBeenCalledWith( expect(mockUpdateBlocksInScope).toHaveBeenCalledWith(
objCategory, validCategory,
validIds, validIds,
); );
expect(mockUpdateExternalBlocks).toHaveBeenCalledWith( expect(mockUpdateExternalBlocks).toHaveBeenCalledWith(
[ [
{ {
id: new mongoose.Types.ObjectId('64abc1234def567890fedcba'), id: '64abc1234def567890fedcab',
attachedBlock: new mongoose.Types.ObjectId(validIds[0]), attachedBlock: validIds[0],
nextBlocks: [new mongoose.Types.ObjectId(validIds[0])], nextBlocks: [validIds[0]],
}, },
], ],
validIds.map((id) => new mongoose.Types.ObjectId(id)), validIds,
); );
}); });
it('should not perform updates if criteria or updates are missing', async () => { it('should not perform updates if no category is provided', async () => {
const mockMapIdsAndCategory = jest.spyOn( const mockFind = jest.spyOn(blockRepository, 'find');
blockRepository,
'mapIdsAndCategory',
);
const mockFind = jest.spyOn(blockModel, 'find');
const mockUpdateBlocksInScope = jest.spyOn( const mockUpdateBlocksInScope = jest.spyOn(
blockRepository, blockRepository,
'updateBlocksInScope', 'updateBlocksInScope',
@ -391,7 +281,6 @@ describe('BlockRepository', () => {
await blockRepository.preUpdateMany({} as any, {}, {}); await blockRepository.preUpdateMany({} as any, {}, {});
expect(mockMapIdsAndCategory).not.toHaveBeenCalled();
expect(mockFind).not.toHaveBeenCalled(); expect(mockFind).not.toHaveBeenCalled();
expect(mockUpdateBlocksInScope).not.toHaveBeenCalled(); expect(mockUpdateBlocksInScope).not.toHaveBeenCalled();
expect(mockUpdateExternalBlocks).not.toHaveBeenCalled(); expect(mockUpdateExternalBlocks).not.toHaveBeenCalled();

View File

@ -94,22 +94,24 @@ export class BlockRepository extends BaseRepository<
| UpdateWithAggregationPipeline | UpdateWithAggregationPipeline
| UpdateQuery<Document<Block, any, any>>, | UpdateQuery<Document<Block, any, any>>,
): Promise<void> { ): Promise<void> {
const movedBlock = await this.findOne(criteria);
if (!movedBlock) {
return;
}
const update: BlockUpdateDto = updates?.['$set']; const update: BlockUpdateDto = updates?.['$set'];
if (update?.category && criteria._id) { if (update?.category) {
const movedBlockId = criteria._id; const movedBlockId = criteria._id;
// Find and update blocks that reference the moved block // Find and update blocks that reference the moved block
await this.model.updateMany( await this.updateMany(
{ nextBlocks: movedBlockId }, { nextBlocks: movedBlockId },
{ $pull: { nextBlocks: movedBlockId } }, { $pull: { nextBlocks: movedBlockId } },
); );
await this.model.updateMany( await this.updateMany(
{ attachedBlock: movedBlockId }, { attachedBlock: movedBlockId },
{ $set: { attachedBlock: null } }, { $set: { attachedBlock: null } },
); );
} else if (update?.category && !criteria._id) {
throw new Error('Criteria must include a valid id to update category.');
} }
this.checkDeprecatedAttachmentUrl(update); this.checkDeprecatedAttachmentUrl(update);
@ -139,10 +141,11 @@ export class BlockRepository extends BaseRepository<
const category: string = updates.$set.category; const category: string = updates.$set.category;
// Step 1: Map IDs and Category // Step 1: Map IDs and Category
const { objIds, objCategory } = this.mapIdsAndCategory(ids, category); const objIds = ids.map((id) => new mongoose.Types.ObjectId(id));
const objCategory = new mongoose.Types.ObjectId(category);
// Step 2: Find other blocks // Step 2: Find other blocks
const otherBlocks = await this.model.find({ const otherBlocks = await this.find({
_id: { $nin: objIds }, _id: { $nin: objIds },
category: { $ne: objCategory }, category: { $ne: objCategory },
$or: [ $or: [
@ -152,76 +155,66 @@ export class BlockRepository extends BaseRepository<
}); });
// Step 3: Update blocks in the provided scope // Step 3: Update blocks in the provided scope
await this.updateBlocksInScope(objCategory, ids); await this.updateBlocksInScope(category, ids);
// Step 4: Update external blocks // Step 4: Update external blocks
await this.updateExternalBlocks(otherBlocks, objIds); await this.updateExternalBlocks(otherBlocks, ids);
} }
} }
mapIdsAndCategory( /**
ids: string[], * Updates blocks within a specified category scope.
category: string, * Ensures nextBlocks and attachedBlock are consistent with the provided IDs and category.
): { *
objIds: mongoose.Types.ObjectId[]; * @param category - The category
objCategory: mongoose.Types.ObjectId; * @param ids - IDs representing the blocks to update.
} { * @returns A promise that resolves once all updates within the scope are complete.
const objIds = ids.map((id) => new mongoose.Types.ObjectId(id)); */
const objCategory = new mongoose.Types.ObjectId(category); async updateBlocksInScope(category: string, ids: string[]): Promise<void> {
return { objIds, objCategory };
}
async updateBlocksInScope(
objCategory: mongoose.Types.ObjectId,
ids: string[],
): Promise<void> {
for (const id of ids) { for (const id of ids) {
const oldState = await this.model.findOne({ const oldState = await this.findOne(id);
_id: new mongoose.Types.ObjectId(id), if (oldState.category !== category) {
});
if (oldState.category.toString() !== objCategory.toString()) {
const updatedNextBlocks = oldState.nextBlocks.filter((nextBlock) => const updatedNextBlocks = oldState.nextBlocks.filter((nextBlock) =>
ids.includes(nextBlock.toString()), ids.includes(nextBlock),
); );
const updatedAttachedBlock = ids.includes( const updatedAttachedBlock = ids.includes(oldState.attachedBlock || '')
oldState.attachedBlock?.toString() || '',
)
? oldState.attachedBlock ? oldState.attachedBlock
: null; : null;
await this.model.updateOne( await this.updateOne(id, {
{ _id: new mongoose.Types.ObjectId(id) }, nextBlocks: updatedNextBlocks,
{ attachedBlock: updatedAttachedBlock,
nextBlocks: updatedNextBlocks, });
attachedBlock: updatedAttachedBlock,
},
);
} }
} }
} }
/**
* Updates blocks outside the specified category scope by removing references to the provided IDs.
* Handles updates to both attachedBlock and nextBlocks.
*
* @param otherBlocks - An array of blocks outside the provided category scope.
* @param ids - An array of the Ids to disassociate.
* @returns A promise that resolves once all external block updates are complete.
*/
async updateExternalBlocks( async updateExternalBlocks(
otherBlocks, otherBlocks: Block[],
objIds: Types.ObjectId[], ids: string[],
): Promise<void> { ): Promise<void> {
for (const block of otherBlocks) { for (const block of otherBlocks) {
if ( if (ids.includes(block.attachedBlock)) {
objIds.some((id) => id.toString() === block.attachedBlock?.toString()) await this.updateOne(block.id, { attachedBlock: null });
) {
await this.model.updateOne({ _id: block.id }, { attachedBlock: null });
} }
const updatedNextBlocks = block.nextBlocks.filter( const updatedNextBlocks = block.nextBlocks.filter(
(nextBlock) => (nextBlock) => !ids.includes(nextBlock),
!objIds.some((id) => id.toString() === nextBlock.toString()),
); );
if (updatedNextBlocks.length !== block.nextBlocks.length) { if (updatedNextBlocks.length > 0) {
await this.model.updateOne( await this.updateOne(block.id, {
{ _id: block.id }, $pull: { nextBlocks: updatedNextBlocks },
{ nextBlocks: updatedNextBlocks }, });
);
} }
} }
} }

View File

@ -343,7 +343,7 @@ export abstract class BaseRepository<
async updateOne<D extends Partial<U>>( async updateOne<D extends Partial<U>>(
criteria: string | TFilterQuery<T>, criteria: string | TFilterQuery<T>,
dto: D, dto: UpdateQuery<D>,
): Promise<T> { ): Promise<T> {
const query = this.model.findOneAndUpdate<T>( const query = this.model.findOneAndUpdate<T>(
{ {
@ -359,7 +359,10 @@ export abstract class BaseRepository<
return await this.executeOne(query, this.cls); return await this.executeOne(query, this.cls);
} }
async updateMany<D extends Partial<U>>(filter: TFilterQuery<T>, dto: D) { async updateMany<D extends Partial<U>>(
filter: TFilterQuery<T>,
dto: UpdateQuery<D>,
) {
return await this.model.updateMany<T>(filter, { return await this.model.updateMany<T>(filter, {
$set: dto, $set: dto,
}); });