From 9b61ca45f25ff3816b5247d7c57bb43c5cb9039e Mon Sep 17 00:00:00 2001 From: Lautaro Petaccio <1120791+LautaroPetaccio@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:34:01 -0300 Subject: [PATCH] feat: Return full updated items after upsert (#764) * feat: Return full updated items after upsert * fix: Add comments --- spec/mocks/items.ts | 8 +++--- spec/utils.ts | 41 ++++++++++++++++++++++++++++++ src/Item/Item.router.spec.ts | 48 ++++++++++++++++++++++++++++++++--- src/Item/Item.service.spec.ts | 21 ++++++++++++--- src/Item/Item.service.ts | 45 ++++++++++++++++++++------------ src/ethereum/api/Bridge.ts | 10 ++++++-- 6 files changed, 143 insertions(+), 30 deletions(-) diff --git a/spec/mocks/items.ts b/spec/mocks/items.ts index 371ad67b..c543fb6c 100644 --- a/spec/mocks/items.ts +++ b/spec/mocks/items.ts @@ -83,9 +83,9 @@ export function toResultTPItem( ...itemAttributes, created_at: itemAttributes.created_at.toISOString(), updated_at: itemAttributes.updated_at.toISOString(), - is_approved: true, - in_catalyst: true, - is_published: true, + is_approved: !!catalystItem, + in_catalyst: !!catalystItem, + is_published: !!catalystItem, urn: hasURN ? buildTPItemURN( dbCollection!.third_party_id!, @@ -169,7 +169,7 @@ export const dbTPItemMock: ThirdPartyItemAttributes = { export const itemFragmentMock = { id: dbCollectionMock.contract_address + '-' + dbItemMock.blockchain_item_id, blockchainId: '0', - urn: `urn:decentraland:mumbai:collections-v2:${dbCollectionMock.contract_address}:${dbItemMock.blockchain_item_id}`, + urn: `urn:decentraland:amoy:collections-v2:${dbCollectionMock.contract_address}:${dbItemMock.blockchain_item_id}`, totalSupply: '1', price: dbItemMock.price!.toString(), beneficiary: 'aBeneficiary', diff --git a/spec/utils.ts b/spec/utils.ts index ac8fd250..ab3b20d1 100644 --- a/spec/utils.ts +++ b/spec/utils.ts @@ -1,10 +1,12 @@ import { Authenticator, AuthIdentity } from '@dcl/crypto' +import { Wearable } from '@dcl/schemas' import { Model, QueryPart } from 'decentraland-server' import { env } from 'decentraland-commons' import { collectionAPI } from '../src/ethereum/api/collection' import { peerAPI } from '../src/ethereum/api/peer' import { isPublished } from '../src/utils/eth' import { AUTH_CHAIN_HEADER_PREFIX } from '../src/middleware/authentication' +import { CollectionFragment, ItemFragment } from '../src/ethereum/api/fragments' import { Collection } from '../src/Collection' import { ItemCuration } from '../src/Curation/ItemCuration' import { Ownable } from '../src/Ownable/Ownable' @@ -405,4 +407,43 @@ export function mockThirdPartyCollectionURNExists( }) } +/** + * Mocks the result of the fetchCollectionWithItem method from the collections client. + * This mock requires collectionAPI to be mocked first. + * + * @param collectionFragment - The blockchain collection to return. + * @param itemFragment - The blockchain item to return. + */ +export function mockFetchCollectionWithItem( + collectionFragment: CollectionFragment | null, + itemFragment: ItemFragment | null +) { + if (!(collectionAPI.fetchCollectionWithItem as jest.Mock).mock) { + throw new Error( + "collectionAPI.fetchCollectionWithItem should be mocked to mock the fetchCollectionWithItem method but it isn't" + ) + } + + ;(collectionAPI.fetchCollectionWithItem as jest.Mock).mockResolvedValueOnce({ + collection: collectionFragment, + item: itemFragment, + }) +} + +/** + * Mocks the result of the fetch items method from the catalyst client. + * This mock requires peerAPI to be mocked first. + * + * @param items - The Catalyst items to return. + */ +export function mockFetchCatalystItems(items: Wearable[]) { + if (!(peerAPI.fetchItems as jest.Mock).mock) { + throw new Error( + "peerAPI.fetchItems should be mocked to mock the fetchItems method but it isn't" + ) + } + + ;(peerAPI.fetchItems as jest.Mock).mockResolvedValueOnce(items) +} + export const isoDateStringMatcher = /\d{4}-[01]\d-[0-3]\dT[0-2]\d:[0-5]\d:[0-5]\d\.\d+([+-][0-2]\d:[0-5]\d|Z)/ diff --git a/src/Item/Item.router.spec.ts b/src/Item/Item.router.spec.ts index 7fe5344e..7fc39293 100644 --- a/src/Item/Item.router.spec.ts +++ b/src/Item/Item.router.spec.ts @@ -1,7 +1,7 @@ import { MappingType, Rarity, Wearable } from '@dcl/schemas' import supertest from 'supertest' import { v4 as uuidv4 } from 'uuid' -import { Wallet } from 'ethers' +import { ethers, Wallet } from 'ethers' import { utils } from 'decentraland-commons' import { omit } from 'decentraland-commons/dist/utils' import { @@ -15,6 +15,8 @@ import { mockThirdPartyItemCurationExists, mockThirdPartyURNExists, isoDateStringMatcher, + mockFetchCollectionWithItem, + mockFetchCatalystItems, } from '../../spec/utils' import { dbCollectionMock, @@ -129,8 +131,6 @@ describe('Item router', () => { resultTPItemNotPublished = asResultItem(dbTPItemNotPublished) // no itemCuration & no catalyst, should be regular Item resultTPItemPublished = { ...toResultTPItem(dbTPItemPublished, dbTPCollectionMock), - is_approved: false, - in_catalyst: false, } }) @@ -262,7 +262,6 @@ describe('Item router', () => { ;(peerAPI.fetchWearables as jest.Mock).mockResolvedValueOnce([tpWearable]) url = '/items' }) - it('should return all the items that are published with URN and the ones that are not without it', () => { return server .get(buildURL(url)) @@ -1003,8 +1002,14 @@ describe('Item router', () => { undefined, tpCollectionMock ), + beneficiary: ethers.constants.AddressZero, + blockchain_item_id: updatedItem.urn_suffix, + price: '0', updated_at: expect.stringMatching(isoDateStringMatcher), } + // Mock get TP item + mockThirdPartyItemCurationExists(dbTPItem.id, false) + mockThirdPartyURNExists(itemToUpsert.urn!, false) }) it('should respond with a 200, update the item and return the updated item', () => { @@ -1073,8 +1078,14 @@ describe('Item router', () => { undefined, tpCollectionMock ), + beneficiary: ethers.constants.AddressZero, + blockchain_item_id: updatedItem.urn_suffix, + price: '0', updated_at: expect.stringMatching(isoDateStringMatcher), } + // Mock get TP item + mockThirdPartyItemCurationExists(dbTPItem.id, false) + mockThirdPartyURNExists(itemToUpsert.urn!, false) }) it('it should respond with a 400 when the urn in the url does not match the one in the body', () => { @@ -1247,7 +1258,13 @@ describe('Item router', () => { ), updated_at: expect.stringMatching(isoDateStringMatcher), created_at: expect.stringMatching(isoDateStringMatcher), + beneficiary: ethers.constants.AddressZero, + blockchain_item_id: updatedItem.urn_suffix, + price: '0', } + // Mock get TP item + mockThirdPartyItemCurationExists(dbTPItem.id, false) + mockThirdPartyURNExists(itemToUpsert.urn!, false) }) it('should respond with a 200, update the item and return the updated item', () => { @@ -1273,6 +1290,9 @@ describe('Item router', () => { collection_id: tpCollectionMock.id, urn: null, } + // Mock get TP item + mockThirdPartyItemCurationExists(dbTPItem.id, false) + mockThirdPartyURNExists(itemToUpsert.urn!, false) }) it('should respond with a 400, signaling that the URN is not valid', () => { @@ -1978,6 +1998,12 @@ describe('Item router', () => { managers: [wallet.address], }) ) + // Mock get item + mockFetchCollectionWithItem(itemFragment.collection, { + ...itemFragment, + totalSupply: '0', + }) + mockFetchCatalystItems([wearable]) }) afterEach(() => { @@ -1996,6 +2022,10 @@ describe('Item router', () => { ...Bridge.toFullItem(dbItem), local_content_hash: expect.any(String), eth_address: ethAddress, + beneficiary: 'aBeneficiary', + in_catalyst: true, + is_published: true, + urn: wearable.id, created_at: dbItem.created_at.toISOString(), updated_at: currentDate.toISOString(), }, @@ -2105,6 +2135,12 @@ describe('Item router', () => { creator: wallet.address, }) ) + // Mock get item + mockFetchCollectionWithItem(itemFragment.collection, { + ...itemFragment, + totalSupply: '0', + }) + mockFetchCatalystItems([wearable]) }) afterEach(() => { @@ -2121,6 +2157,10 @@ describe('Item router', () => { expect(response.body).toEqual({ data: { ...Bridge.toFullItem(dbItem), + beneficiary: itemFragment.beneficiary, + in_catalyst: true, + is_published: true, + urn: wearable.id, local_content_hash: expect.any(String), eth_address: wallet.address, created_at: dbItem.created_at.toISOString(), diff --git a/src/Item/Item.service.spec.ts b/src/Item/Item.service.spec.ts index 747db4c3..3a493834 100644 --- a/src/Item/Item.service.spec.ts +++ b/src/Item/Item.service.spec.ts @@ -8,7 +8,11 @@ import { itemFragmentMock, } from '../../spec/mocks/items' import { wearableMock } from '../../spec/mocks/peer' -import { mockOwnableCanUpsert } from '../../spec/utils' +import { + mockFetchCatalystItems, + mockFetchCollectionWithItem, + mockOwnableCanUpsert, +} from '../../spec/utils' import { CollectionAttributes } from '../Collection' import { Collection } from '../Collection/Collection.model' import { CollectionService } from '../Collection/Collection.service' @@ -139,6 +143,7 @@ describe('Item Service', () => { beforeEach(() => { ;(Item.findByURNSuffix as jest.Mock).mockResolvedValueOnce(undefined) }) + describe('and it is inserting less than the maximun amount of tags', () => { beforeEach(() => { dbItem = { @@ -164,7 +169,11 @@ describe('Item Service', () => { jest .spyOn(CollectionService.prototype, 'isDCLPublished') .mockResolvedValueOnce(false) + // Mock get item + mockFetchCollectionWithItem(null, null) + mockFetchCatalystItems([]) }) + it('should not throw any errors and return the inserted item', () => { const result = service.upsertItem( Bridge.toFullItem(dbItem, dbTPCollectionMock), @@ -201,11 +210,11 @@ describe('Item Service', () => { beforeEach(() => { ;(Item.findByURNSuffix as jest.Mock).mockResolvedValueOnce(dbItem) }) - describe('and the item already has the maximun amount of tags', () => { + describe('and the item already has the maximum amount of tags', () => { it('should throw the MaximunAmountOfTagsReachedError error', () => { return expect( service.upsertItem( - Bridge.toFullItem(dbItem, dbTPCollectionMock), + Bridge.toFullItem(dbItem, dbCollectionMock), dbItem.eth_address ) ).rejects.toThrowError(MaximunAmountOfTagsReachedError) @@ -237,10 +246,14 @@ describe('Item Service', () => { .spyOn(CollectionService.prototype, 'isDCLPublished') .mockResolvedValueOnce(false) ;(Item.upsert as jest.Mock).mockResolvedValueOnce(dbItem) + // Mock get item + mockFetchCollectionWithItem(null, null) + mockFetchCatalystItems([]) }) + it('should not throw any error and return the inserted item', () => { const result = service.upsertItem( - Bridge.toFullItem(dbItem, dbTPCollectionMock), + Bridge.toFullItem(dbItem, dbCollectionMock), dbItem.eth_address ) return expect(result).resolves.toEqual( diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts index 4fb61a18..f1a47086 100644 --- a/src/Item/Item.service.ts +++ b/src/Item/Item.service.ts @@ -355,10 +355,12 @@ export class ItemService { } private async getTPItem( - dbItem: ThirdPartyItemAttributes + dbItem: ThirdPartyItemAttributes, + dbCollection?: CollectionAttributes ): Promise<{ item: FullItem; collection?: CollectionAttributes }> { let item: FullItem = Bridge.toFullItem(dbItem) - let collection = await Collection.findOne(dbItem.collection_id) + let collection = + dbCollection ?? (await Collection.findOne(dbItem.collection_id)) if (collection && isTPCollection(collection)) { const urn = buildTPItemURN( @@ -375,24 +377,27 @@ export class ItemService { : collection const catalystItems = await peerAPI.fetchWearables([urn]) - if (catalystItems.length > 0) { - item = Bridge.mergeTPItem(dbItem, collection, catalystItems[0]) - } + item = Bridge.mergeTPItem( + dbItem, + collection as ThirdPartyCollectionAttributes, + catalystItems[0] + ) } return { item, collection } } private async getDCLItem( - dbItem: ItemAttributes + dbItem: ItemAttributes, + dbItemCollection?: CollectionAttributes ): Promise<{ item: FullItem; collection?: CollectionAttributes }> { let item: FullItem = Bridge.toFullItem(dbItem) let collection: CollectionAttributes | undefined if (dbItem.collection_id && dbItem.blockchain_item_id) { - const dbCollection = await Collection.findOne( - dbItem.collection_id - ) + const dbCollection = + dbItemCollection ?? + (await Collection.findOne(dbItem.collection_id)) if (!dbCollection) { throw new InconsistentItemError( @@ -426,7 +431,6 @@ export class ItemService { ) } } - // Set the item's URN item.urn = item.urn ?? @@ -664,12 +668,20 @@ export class ItemService { // Compute the content hash of the item to later store it in the DB attributes.local_content_hash = - dbCollection && isStandardItemPublished(attributes, dbCollection) - ? await calculateItemContentHash(attributes, dbCollection) + dbCollection && + isStandardItemPublished(attributes, itemCollection ?? dbCollection) + ? await calculateItemContentHash( + attributes, + itemCollection ?? dbCollection + ) : null const upsertedItem: ItemAttributes = await Item.upsert(attributes) - return Bridge.toFullItem(upsertedItem, dbCollection) + const { item: fullItem } = await this.getDCLItem( + upsertedItem, + itemCollection ?? dbCollection + ) + return fullItem } /** @@ -786,10 +798,10 @@ export class ItemService { ? await calculateItemContentHash(attributes, dbCollection) : null - const upsertedItem: ItemAttributes = await Item.upsert({ + const upsertedItem = (await Item.upsert({ ...attributes, ...(attributes.mappings ? { mappings: attributes.mappings } : {}), - }) + })) as ThirdPartyItemAttributes if (dbItem && attributes.local_content_hash) { // Update the Item Curation content_hash await ItemCuration.update( @@ -798,7 +810,8 @@ export class ItemService { ) } - return Bridge.toFullItem(upsertedItem, dbCollection) + const { item: fullItem } = await this.getTPItem(upsertedItem, dbCollection) + return fullItem } private async checkIfThirdPartyItemURNExists( diff --git a/src/ethereum/api/Bridge.ts b/src/ethereum/api/Bridge.ts index 36c6785f..104180e6 100644 --- a/src/ethereum/api/Bridge.ts +++ b/src/ethereum/api/Bridge.ts @@ -9,7 +9,11 @@ import { import { ItemAttributes, FullItem } from '../../Item' import { fromUnixTimestamp } from '../../utils/parse' import { buildTPItemURN } from '../../Item/utils' -import { decodeThirdPartyItemURN, isTPCollection } from '../../utils/urn' +import { + decodeThirdPartyItemURN, + getDecentralandItemURN, + isTPCollection, +} from '../../utils/urn' import { ItemCuration, ItemCurationAttributes, @@ -189,7 +193,7 @@ export class Bridge { blockchain_item_id: decodeThirdPartyItemURN(urn).item_urn_suffix, urn, in_catalyst: !!catalystItem, - is_published: true, + is_published: !!catalystItem, // For now, items are always approved. Rejecting (or disabling) items will be done at the record level, for all collections that apply. is_approved: !!catalystItem, content_hash: null, @@ -429,6 +433,8 @@ export class Bridge { dbCollection.urn_suffix, dbItem.urn_suffix! ) + : dbCollection && !isTPCollection(dbCollection) + ? getDecentralandItemURN(dbItem, dbCollection.contract_address!) : null, in_catalyst: false, is_approved: false,