From ec04a5aeb5a3d6c65fdf5d4a4f246b91523f2e1e Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Thu, 4 Apr 2024 16:02:11 +0200 Subject: [PATCH 1/5] feat: add validation for the amount the tags being added --- src/Item/Item.errors.ts | 10 +++ src/Item/Item.model.ts | 2 + src/Item/Item.service.spec.ts | 147 +++++++++++++++++++++++++++++++++- src/Item/Item.service.ts | 14 +++- 4 files changed, 169 insertions(+), 4 deletions(-) diff --git a/src/Item/Item.errors.ts b/src/Item/Item.errors.ts index 69f5d4da..df9131b9 100644 --- a/src/Item/Item.errors.ts +++ b/src/Item/Item.errors.ts @@ -1,3 +1,5 @@ +import { MAX_TAGS_LENGTH } from './Item.model' + export enum ItemAction { DELETE = 'deleted', INSERT = 'inserted', @@ -42,6 +44,14 @@ export class URNAlreadyInUseError extends Error { } } +export class MaximunAmountOfTagsReachedError extends Error { + constructor(public id: string) { + super( + `You hace exceeded the maximun amount of tags allowed (${MAX_TAGS_LENGTH}).` + ) + } +} + export class DCLItemAlreadyPublishedError extends Error { constructor( public id: string, diff --git a/src/Item/Item.model.ts b/src/Item/Item.model.ts index 83fea789..77334d71 100644 --- a/src/Item/Item.model.ts +++ b/src/Item/Item.model.ts @@ -5,6 +5,8 @@ import { ItemCuration } from '../Curation/ItemCuration' import { DEFAULT_LIMIT } from '../Pagination/utils' import { DBItemApprovalData, ItemAttributes } from './Item.types' +export const MAX_TAGS_LENGTH = 20 + type ItemWithTotalCount = ItemAttributes & { total_count: number } type ItemQueryOptions = { diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index afd660c0..ff0cf1ba 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -8,25 +8,48 @@ import { itemFragmentMock, } from '../../spec/mocks/items' import { wearableMock } from '../../spec/mocks/peer' +import { mockOwnableCanUpsert } from '../../spec/utils' import { Collection } from '../Collection/Collection.model' +import { CollectionService } from '../Collection/Collection.service' import { Bridge } from '../ethereum/api/Bridge' import { collectionAPI } from '../ethereum/api/collection' import { peerAPI } from '../ethereum/api/peer' import { ItemCantBeMovedFromCollectionError, + MaximunAmountOfTagsReachedError, ThirdPartyItemInsertByURNError, } from './Item.errors' -import { Item } from './Item.model' +import { Item, MAX_TAGS_LENGTH } from './Item.model' import { ItemService } from './Item.service' -import { ItemAttributes } from './Item.types' +import { FullItem, ItemAttributes } from './Item.types' import { VIDEO_PATH } from './utils' jest.mock('../ethereum/api/collection') jest.mock('../Collection/Collection.model') jest.mock('../ethereum/api/peer') jest.mock('./Item.model') +// jest.mock('../Collection/Collection.service', () => { +// // Require the actual module to not mock everything +// const actualModule = jest.requireActual('../Collection/Collection.service'); +// console.log('actualModule: ', actualModule); + +// // Create a mock for just the getDBCollection method +// const mockGetDBCollection = jest.fn(); + +// // Extend the actual CollectionService class and override the method you wish to mock +// class MockCollectionService extends actualModule.CollectionService { +// getDBCollection = mockGetDBCollection; +// } + +// // Return the modified module with the mocked class +// return { +// ...actualModule, +// CollectionService: MockCollectionService, +// }; +// }); describe('Item Service', () => { + let item: FullItem let dbItem: ItemAttributes let dbTPItem: ItemAttributes let service: ItemService @@ -37,6 +60,10 @@ describe('Item Service', () => { describe('isOwnedOrManagedBy', () => { describe('when the owner is the same as the one in the DB', () => { beforeEach(() => { + // ;(CollectionService.prototype + // .getDBCollection as jest.Mock).mockResolvedValueOnce( + // dbCollectionMock + // ) dbItem = { ...dbItemMock, eth_address: '0xoriginalAddress' } ;(Item.findOne as jest.Mock).mockResolvedValueOnce(dbItem) ;(Collection.findOne as jest.Mock).mockResolvedValueOnce({ @@ -123,6 +150,122 @@ describe('Item Service', () => { ).rejects.toThrowError(ThirdPartyItemInsertByURNError) }) }) + + describe('and the item being upserted contains tags', () => { + beforeEach(() => { + CollectionService.prototype.getDBCollection = jest.fn() + }) + describe('and it is an insert operation', () => { + beforeEach(() => { + ;(Item.findByURNSuffix as jest.Mock).mockResolvedValueOnce(undefined) + }) + describe('and it is inserting less than the maximun amount of tags', () => { + beforeEach(() => { + dbItem = { + ...dbItemMock, + eth_address: '0xAddress', + data: { + ...dbItemMock.data, + tags: Array(MAX_TAGS_LENGTH - 1).fill('tag'), + }, + } + dbCollectionMock.eth_address = dbItem.eth_address + mockOwnableCanUpsert(Item, dbItem.id, dbItem.eth_address, true) + mockOwnableCanUpsert( + Collection, + dbCollectionMock.id, + dbItem.eth_address, + true + ) + ;(CollectionService.prototype + .getDBCollection as jest.Mock).mockResolvedValueOnce( + dbCollectionMock + ) + ;(Item.upsert as jest.Mock).mockResolvedValueOnce(dbItem) + }) + it('should not throw any errors and return the inserted item', () => { + const result = service.upsertItem( + Bridge.toFullItem(dbItem, dbTPCollectionMock), + dbItem.eth_address + ) + return expect(result).resolves.toEqual( + Bridge.toFullItem(dbItem, dbCollectionMock) + ) + }) + }) + + describe('and it is inserting more than the maximun amount of tags', () => { + beforeEach(() => { + dbItem = { + ...dbItemMock, + data: { + ...dbItemMock.data, + tags: Array(MAX_TAGS_LENGTH + 1).fill('tag'), + }, + } + }) + it('should throw the MaximunAmountOfTagsReachedError error', () => { + return expect(() => + service.upsertItem( + Bridge.toFullItem(dbItem, dbTPCollectionMock), + dbItem.eth_address + ) + ).rejects.toThrowError(MaximunAmountOfTagsReachedError) + }) + }) + }) + + describe('and it is an update operation', () => { + beforeEach(() => { + ;(Item.findByURNSuffix as jest.Mock).mockResolvedValueOnce(dbItem) + }) + describe('and the item already has the maximun amount of tags', () => { + it('should throw the MaximunAmountOfTagsReachedError error', () => { + return expect(() => + service.upsertItem( + Bridge.toFullItem(dbItem, dbTPCollectionMock), + dbItem.eth_address + ) + ).rejects.toThrowError(MaximunAmountOfTagsReachedError) + }) + }) + + describe('and the item has less than the maximun amount of tags', () => { + beforeEach(() => { + dbItem = { + ...dbItemMock, + eth_address: '0xAddress', + data: { + ...dbItemMock.data, + tags: Array(MAX_TAGS_LENGTH - 1).fill('tag'), + }, + } + dbCollectionMock.eth_address = dbItem.eth_address + mockOwnableCanUpsert(Item, dbItem.id, dbItem.eth_address, true) + mockOwnableCanUpsert( + Collection, + dbCollectionMock.id, + dbItem.eth_address, + true + ) + ;(CollectionService.prototype + .getDBCollection as jest.Mock).mockResolvedValueOnce( + dbCollectionMock + ) + ;(Item.upsert as jest.Mock).mockResolvedValueOnce(dbItem) + }) + it('should not throw any error and return the inserted item', () => { + const result = service.upsertItem( + Bridge.toFullItem(dbItem, dbTPCollectionMock), + dbItem.eth_address + ) + return expect(result).resolves.toEqual( + Bridge.toFullItem(dbItem, dbCollectionMock) + ) + }) + }) + }) + }) }) describe('getItemByContractAddressAndTokenId', () => { diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts index 91f8ba76..fb064754 100644 --- a/src/Item/Item.service.ts +++ b/src/Item/Item.service.ts @@ -34,8 +34,9 @@ import { InvalidItemURNError, URNAlreadyInUseError, ThirdPartyItemInsertByURNError, + MaximunAmountOfTagsReachedError, } from './Item.errors' -import { Item } from './Item.model' +import { Item, MAX_TAGS_LENGTH } from './Item.model' import { FullItem, ItemAttributes, @@ -73,6 +74,16 @@ export class ItemService { ) : await Item.findOne(item.id) + if (item.data.tags.length > MAX_TAGS_LENGTH) { + const isAlreadyExcided = + !!dbItem && dbItem.data.tags.length > MAX_TAGS_LENGTH + const isAddingMoreTags = + !!dbItem && item.data.tags.length > dbItem.data.tags.length + if (!dbItem || (isAlreadyExcided && isAddingMoreTags)) { + throw new MaximunAmountOfTagsReachedError(item.id) + } + } + // Inserting by URN is not allowed if (!item.id && item.urn && !dbItem) { throw new ThirdPartyItemInsertByURNError(item.urn) @@ -515,7 +526,6 @@ export class ItemService { // Check if we have permissions to move or edit an orphaned item if (!dbItem?.collection_id) { const canUpsert = await new Ownable(Item).canUpsert(item.id, eth_address) - if (!canUpsert) { throw new UnauthorizedToUpsertError(item.id, eth_address) } From 8cc48c866fe9fe775bc6dab08e38c913fb4879a1 Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Thu, 4 Apr 2024 16:03:22 +0200 Subject: [PATCH 2/5] test: remove commented code --- src/Item/Item.service.spec.ts | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index ff0cf1ba..6eff5cf5 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -28,28 +28,8 @@ jest.mock('../ethereum/api/collection') jest.mock('../Collection/Collection.model') jest.mock('../ethereum/api/peer') jest.mock('./Item.model') -// jest.mock('../Collection/Collection.service', () => { -// // Require the actual module to not mock everything -// const actualModule = jest.requireActual('../Collection/Collection.service'); -// console.log('actualModule: ', actualModule); - -// // Create a mock for just the getDBCollection method -// const mockGetDBCollection = jest.fn(); - -// // Extend the actual CollectionService class and override the method you wish to mock -// class MockCollectionService extends actualModule.CollectionService { -// getDBCollection = mockGetDBCollection; -// } - -// // Return the modified module with the mocked class -// return { -// ...actualModule, -// CollectionService: MockCollectionService, -// }; -// }); describe('Item Service', () => { - let item: FullItem let dbItem: ItemAttributes let dbTPItem: ItemAttributes let service: ItemService @@ -60,10 +40,6 @@ describe('Item Service', () => { describe('isOwnedOrManagedBy', () => { describe('when the owner is the same as the one in the DB', () => { beforeEach(() => { - // ;(CollectionService.prototype - // .getDBCollection as jest.Mock).mockResolvedValueOnce( - // dbCollectionMock - // ) dbItem = { ...dbItemMock, eth_address: '0xoriginalAddress' } ;(Item.findOne as jest.Mock).mockResolvedValueOnce(dbItem) ;(Collection.findOne as jest.Mock).mockResolvedValueOnce({ From 3780d87db8d90c674cada63f81b5d81e93080ad2 Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Thu, 4 Apr 2024 16:49:10 +0200 Subject: [PATCH 3/5] chore: remove old import not used anymore --- src/Item/Item.service.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index 6eff5cf5..b0eba4a4 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -21,7 +21,7 @@ import { } from './Item.errors' import { Item, MAX_TAGS_LENGTH } from './Item.model' import { ItemService } from './Item.service' -import { FullItem, ItemAttributes } from './Item.types' +import { ItemAttributes } from './Item.types' import { VIDEO_PATH } from './utils' jest.mock('../ethereum/api/collection') From 498581c4586b9cad3583292cdd9df94a3c80cf3d Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Thu, 4 Apr 2024 17:16:44 +0200 Subject: [PATCH 4/5] test: add mock serviec method --- src/Item/Item.service.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index b0eba4a4..7e26da32 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -158,6 +158,7 @@ describe('Item Service', () => { dbCollectionMock ) ;(Item.upsert as jest.Mock).mockResolvedValueOnce(dbItem) + CollectionService.prototype.isDCLPublished = jest.fn() }) it('should not throw any errors and return the inserted item', () => { const result = service.upsertItem( From 99027aed26a9f826471e1162ba392620ec73b7fa Mon Sep 17 00:00:00 2001 From: Juanma Hidalgo Date: Fri, 5 Apr 2024 10:59:44 +0200 Subject: [PATCH 5/5] test: remove extra arrow fn not needed --- src/Item/Item.service.spec.ts | 10 +++++----- src/Item/Item.service.ts | 13 ++++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index 7e26da32..3a7fa507 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -83,7 +83,7 @@ describe('Item Service', () => { }) it('should throw the ItemCantBeMovedFromCollectionError error', () => { - return expect(() => + return expect( service.upsertItem( Bridge.toFullItem(dbTPItem, dbTPCollectionMock), '0xnewCreator' @@ -105,7 +105,7 @@ describe('Item Service', () => { }) it('should throw the ItemCantBeMovedFromCollectionError error', () => { - return expect(() => + return expect( service.upsertItem(Bridge.toFullItem(dbItem), '0xnewCreator') ).rejects.toThrowError(ItemCantBeMovedFromCollectionError) }) @@ -118,7 +118,7 @@ describe('Item Service', () => { }) it('should throw the ThirdPartyItemInsertByURNError error', () => { - return expect(() => + return expect( service.upsertItem( Bridge.toFullItem(dbTPItem, dbTPCollectionMock), '0xnewCreator' @@ -182,7 +182,7 @@ describe('Item Service', () => { } }) it('should throw the MaximunAmountOfTagsReachedError error', () => { - return expect(() => + return expect( service.upsertItem( Bridge.toFullItem(dbItem, dbTPCollectionMock), dbItem.eth_address @@ -198,7 +198,7 @@ describe('Item Service', () => { }) describe('and the item already has the maximun amount of tags', () => { it('should throw the MaximunAmountOfTagsReachedError error', () => { - return expect(() => + return expect( service.upsertItem( Bridge.toFullItem(dbItem, dbTPCollectionMock), dbItem.eth_address diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts index fb064754..20f467c8 100644 --- a/src/Item/Item.service.ts +++ b/src/Item/Item.service.ts @@ -63,6 +63,7 @@ export class ItemService { item: FullItem, eth_address: string ): Promise { + console.log('here1') const decodedItemURN = !item.id && item.urn ? decodeThirdPartyItemURN(item.urn) : null @@ -73,22 +74,25 @@ export class ItemService { decodedItemURN.item_urn_suffix ) : await Item.findOne(item.id) - + console.log('here2') if (item.data.tags.length > MAX_TAGS_LENGTH) { - const isAlreadyExcided = + const isAlreadyExceeded = !!dbItem && dbItem.data.tags.length > MAX_TAGS_LENGTH const isAddingMoreTags = !!dbItem && item.data.tags.length > dbItem.data.tags.length - if (!dbItem || (isAlreadyExcided && isAddingMoreTags)) { + if (!dbItem || (isAlreadyExceeded && isAddingMoreTags)) { throw new MaximunAmountOfTagsReachedError(item.id) } } + console.log('here3') // Inserting by URN is not allowed if (!item.id && item.urn && !dbItem) { throw new ThirdPartyItemInsertByURNError(item.urn) } + console.log('here4') + const isMovingItemFromACollectionToAnother = dbItem && this.isMovingItemFromACollectionToAnother(item, dbItem) const isMovingOrphanItemIntoACollection = @@ -126,6 +130,7 @@ export class ItemService { // Set the item dates item = { ...item, ...buildModelDates(dbItem?.created_at) } + console.log('here5') // An item is a third party item if it's current collection or the collection // that is going to be inserted into is a third party collection. if (dbItemCollection && isTPCollection(dbItemCollection)) { @@ -505,6 +510,7 @@ export class ItemService { const isMovingItemBetweenCollections = dbItem && this.isMovingItemFromACollectionToAnother(item, dbItem) + console.log('here6') const [ isDbItemCollectionPublished, isItemCollectionPublished, @@ -517,6 +523,7 @@ export class ItemService { itemCollection.contract_address && this.collectionService.isDCLPublished(itemCollection.contract_address), ]) + console.log('here7') const isDbItemCollectionOwner = dbCollection && this.isCollectionOwner(eth_address, dbCollection)