diff --git a/src/Item/Item.errors.ts b/src/Item/Item.errors.ts index 939a6947..7dca0c6f 100644 --- a/src/Item/Item.errors.ts +++ b/src/Item/Item.errors.ts @@ -1,5 +1,8 @@ export enum ItemAction { DELETE = 'deleted', + INSERT = 'inserted', + UPSERT = 'inserted or updated', + RARITY_UPDATE = 'updated with a new rarity', } export enum ItemType { @@ -22,7 +25,6 @@ export class ThirdPartyItemAlreadyPublishedError extends Error { export class DCLItemAlreadyPublishedError extends Error { constructor( public id: string, - public blockchainItemId: string, public contractAddress: string, action: ItemAction ) { @@ -43,3 +45,25 @@ export class InconsistentItemError extends Error { super(message) } } + +export class ItemCantBeMovedFromCollectionError extends Error { + constructor(public id: string) { + super("Item can't change between collections") + } +} + +export class UnauthorizedToUpsertError extends Error { + constructor(public id: string, public eth_address: string) { + super('The user is unauthorized to upsert the collection.') + } +} + +export class UnauthorizedToChangeToCollection extends Error { + constructor( + public id: string, + public eth_address: string, + public collection_id: string + ) { + super("The new collection for the item isn't owned by the same owner") + } +} diff --git a/src/Item/Item.router.spec.ts b/src/Item/Item.router.spec.ts index 86e866df..c7d62cb2 100644 --- a/src/Item/Item.router.spec.ts +++ b/src/Item/Item.router.spec.ts @@ -26,7 +26,7 @@ import { Item } from './Item.model' import { hasAccess } from './access' import { ItemAttributes, ItemRarity } from './Item.types' import { peerAPI, Wearable } from '../ethereum/api/peer' -import { CollectionFragment, ItemFragment } from '../ethereum/api/fragments' +import { ItemFragment } from '../ethereum/api/fragments' import { STATUS_CODES } from '../common/HTTPError' import { Bridge } from '../ethereum/api/Bridge' import { thirdPartyAPI } from '../ethereum/api/thirdParty' @@ -314,8 +314,6 @@ describe('Item router', () => { describe('when upserting an item', () => { const mockCollection = Collection as jest.Mocked - const mockPeer = peerAPI as jest.Mocked - const mockCollectionApi = collectionAPI as jest.Mocked const mockItem = Item as jest.MockedClass & jest.Mocked @@ -341,34 +339,6 @@ describe('Item router', () => { }) }) - describe('and is_approved or is_published exist in the payload item', () => { - const testProperty = async (prop: 'is_published' | 'is_approved') => { - const response = await server - .put(buildURL(url)) - .send({ item: { ...dbItem, [prop]: true } }) - .set(createAuthHeaders('put', url)) - .expect(STATUS_CODES.unauthorized) - - expect(response.body).toEqual({ - data: { id: dbItem.id, eth_address: wallet.address }, - error: 'Can not change is_published or is_approved property', - ok: false, - }) - } - - describe('having is_approved present', () => { - it('should fail with with cant set is approved or is published message', async () => { - testProperty('is_approved') - }) - }) - - describe('having is_published present', () => { - it('should fail with with cant set is approved or is published message', async () => { - testProperty('is_published') - }) - }) - }) - describe('and the user upserting is not authorized to do so', () => { beforeEach(() => { mockOwnableCanUpsert(Item, dbItem.id, wallet.address, false) @@ -382,7 +352,7 @@ describe('Item router', () => { expect(response.body).toEqual({ data: { id: dbItem.id, eth_address: wallet.address }, - error: 'Unauthorized user', + error: 'The user is unauthorized to upsert the collection.', ok: false, }) }) @@ -391,6 +361,7 @@ describe('Item router', () => { describe('and the collection provided in the payload does not exists in the db', () => { beforeEach(() => { mockOwnableCanUpsert(Item, dbItem.id, wallet.address, true) + mockCollection.findOne.mockResolvedValueOnce(undefined) }) it('should fail with collection not found message', async () => { @@ -402,7 +373,7 @@ describe('Item router', () => { expect(response.body).toEqual({ data: { collectionId: dbItem.collection_id }, - error: 'Collection not found', + error: "The collection doesn't exist.", ok: false, }) }) @@ -431,7 +402,8 @@ describe('Item router', () => { eth_address: wallet.address, collection_id: dbItem.collection_id, }, - error: 'Unauthorized user', + error: + "The new collection for the item isn't owned by the same owner", ok: false, }) }) @@ -466,7 +438,6 @@ describe('Item router', () => { describe('when the collection given for the item is locked', () => { beforeEach(() => { mockOwnableCanUpsert(Item, dbItem.id, wallet.address, true) - mockPeer.fetchWearables.mockResolvedValueOnce([{}] as Wearable[]) mockCollection.findOne.mockResolvedValueOnce({ collection_id: dbItem.collection_id, eth_address: wallet.address, @@ -492,7 +463,8 @@ describe('Item router', () => { data: { id: dbItem.id, }, - error: "Locked collection items can't be updated", + error: + "The collection for the item is locked. The item can't be inserted or updated.", ok: false, }) }) @@ -501,30 +473,33 @@ describe('Item router', () => { describe('when the collection given for the item is already published', () => { beforeEach(() => { + dbItem.collection_id = collectionAttributesMock.id mockOwnableCanUpsert(Item, dbItem.id, wallet.address, true) - mockPeer.fetchWearables.mockResolvedValueOnce([{}] as Wearable[]) mockCollection.findOne.mockResolvedValueOnce({ + ...collectionAttributesMock, collection_id: dbItem.collection_id, eth_address: wallet.address, contract_address: Wallet.createRandom().address, }) - - mockCollectionApi.fetchCollection.mockResolvedValueOnce( - {} as CollectionFragment - ) + mockIsCollectionPublished(collectionAttributesMock.id, true) }) - describe('and the item is being upserted for the first time', () => { + describe('and the item is being inserted', () => { + beforeEach(() => { + mockItem.findOne.mockResolvedValueOnce(undefined) + }) + it('should fail with can not add item to published collection message', async () => { const response = await server .put(buildURL(url)) .send({ item: dbItem }) .set(createAuthHeaders('put', url)) - .expect(STATUS_CODES.badRequest) + .expect(STATUS_CODES.conflict) expect(response.body).toEqual({ data: { id: dbItem.id }, - error: "Items can't be added to a published collection", + error: + "The collection that contains this item has been already published. The item can't be inserted.", ok: false, }) }) @@ -540,11 +515,12 @@ describe('Item router', () => { .put(buildURL(url)) .send({ item: { ...dbItem, collection_id: null } }) .set(createAuthHeaders('put', url)) - .expect(STATUS_CODES.badRequest) + .expect(STATUS_CODES.conflict) expect(response.body).toEqual({ data: { id: dbItem.id }, - error: "Items can't be removed from a pubished collection", + error: + "The collection that contains this item has been already published. The item can't be deleted.", ok: false, }) }) @@ -560,16 +536,14 @@ describe('Item router', () => { .put(buildURL(url)) .send({ item: { ...dbItem, rarity: ItemRarity.EPIC } }) .set(createAuthHeaders('put', url)) - .expect(STATUS_CODES.badRequest) + .expect(STATUS_CODES.conflict) expect(response.body).toEqual({ data: { id: dbItem.id, - current: dbItem.rarity, - other: ItemRarity.EPIC, }, error: - "An item rarity from a published collection can't be changed", + "The collection that contains this item has been already published. The item can't be updated with a new rarity.", ok: false, }) }) @@ -579,7 +553,6 @@ describe('Item router', () => { describe('and all the conditions for success are given', () => { beforeEach(() => { mockOwnableCanUpsert(Item, dbItem.id, wallet.address, true) - mockPeer.fetchWearables.mockResolvedValueOnce([] as Wearable[]) mockCollection.findOne.mockResolvedValueOnce({ collection_id: dbItem.collection_id, eth_address: wallet.address, @@ -706,7 +679,6 @@ describe('Item router', () => { "The collection that contains this item has been already published. The item can't be deleted.", data: { id: dbItem.id, - blockchain_item_id: dbItem.blockchain_item_id, contract_address: collectionAttributesMock.contract_address, }, ok: false, diff --git a/src/Item/Item.router.ts b/src/Item/Item.router.ts index 4cf55a76..507b358f 100644 --- a/src/Item/Item.router.ts +++ b/src/Item/Item.router.ts @@ -14,7 +14,7 @@ import { withLowercasedParams, withSchemaValidation, } from '../middleware' -import { Ownable, OwnableModel } from '../Ownable' +import { OwnableModel } from '../Ownable' import { S3Item, getFileUploader, ACL, S3Content } from '../S3' import { RequestParameters } from '../RequestParameters' import { @@ -29,15 +29,19 @@ import { ItemAttributes } from './Item.types' import { upsertItemSchema } from './Item.schema' import { FullItem } from './Item.types' import { hasAccess } from './access' -import { getDecentralandItemURN, toDBItem } from './utils' +import { getDecentralandItemURN } from './utils' import { ItemService } from './Item.service' import { CollectionForItemLockedError, DCLItemAlreadyPublishedError, InconsistentItemError, + ItemCantBeMovedFromCollectionError, NonExistentItemError, ThirdPartyItemAlreadyPublishedError, + UnauthorizedToChangeToCollection, + UnauthorizedToUpsertError, } from './Item.errors' +import { NonExistentCollectionException } from '../Collection/Collection.exceptions' export class ItemRouter extends Router { // To be removed once we move everything to the item service @@ -316,7 +320,7 @@ export class ItemRouter extends Router { return Bridge.consolidateItems(dbItems, remoteItems, catalystItems) } - upsertItem = async (req: AuthRequest) => { + upsertItem = async (req: AuthRequest): Promise => { const id = server.extractFromReq(req, 'id') const itemJSON: FullItem = server.extractFromReq(req, 'item') @@ -332,137 +336,53 @@ export class ItemRouter extends Router { } const eth_address = req.auth.ethAddress.toLowerCase() - - if (itemJSON.is_published || itemJSON.is_approved) { - throw new HTTPError( - 'Can not change is_published or is_approved property', - { id, eth_address }, - STATUS_CODES.unauthorized - ) - } - - const canUpsert = await new Ownable(Item).canUpsert(id, eth_address) - - if (!canUpsert) { - throw new HTTPError( - 'Unauthorized user', - { id, eth_address }, - STATUS_CODES.unauthorized + try { + const upsertedItem = await this.itemService.upsertItem( + itemJSON, + eth_address ) - } - - const dbItem = await Item.findOne(id) - - if (dbItem) { - const areBothCollectionIdsDefined = - itemJSON.collection_id && dbItem.collection_id - - const isItemCollectionBeingChanged = - areBothCollectionIdsDefined && - itemJSON.collection_id !== dbItem.collection_id - - if (isItemCollectionBeingChanged) { + return upsertedItem + } catch (error) { + if (error instanceof UnauthorizedToUpsertError) { throw new HTTPError( - "Item can't change between collections", - { id }, + error.message, + { id: error.id, eth_address: error.eth_address }, STATUS_CODES.unauthorized ) - } - } - - const findCollection = async (id?: string | null) => - id ? await Collection.findOne(id) : undefined - - const dbCollection = await findCollection(itemJSON.collection_id) - - if (itemJSON.collection_id && !dbCollection) { - throw new HTTPError( - 'Collection not found', - { collectionId: itemJSON.collection_id }, - STATUS_CODES.notFound - ) - } - - if (dbCollection) { - const isCollectionOwnerDifferent = - dbCollection.eth_address.toLowerCase() !== eth_address - - if (isCollectionOwnerDifferent) { + } else if (error instanceof ItemCantBeMovedFromCollectionError) { throw new HTTPError( - 'Unauthorized user', - { id, eth_address, collection_id: itemJSON.collection_id }, + error.message, + { id: error.id }, STATUS_CODES.unauthorized ) - } - } - - const isDbCollectionPublished = - dbCollection && - (await this.collectionService.isPublished(dbCollection.contract_address!)) - - if (isDbCollectionPublished) { - if (!dbItem) { + } else if (error instanceof UnauthorizedToChangeToCollection) { throw new HTTPError( - "Items can't be added to a published collection", - { id }, - STATUS_CODES.badRequest + error.message, + { + id: error.id, + eth_address: error.eth_address, + collection_id: error.collection_id, + }, + STATUS_CODES.unauthorized ) - } - - const areBothRaritiesDefined = itemJSON.rarity && dbItem.rarity - - const isRarityBeingChanged = - areBothRaritiesDefined && itemJSON.rarity !== dbItem.rarity - - if (isRarityBeingChanged) { + } else if (error instanceof NonExistentCollectionException) { throw new HTTPError( - "An item rarity from a published collection can't be changed", - { id, current: dbItem.rarity, other: itemJSON.rarity }, - STATUS_CODES.badRequest + error.message, + { collectionId: error.id }, + STATUS_CODES.notFound ) - } - } - - if ( - dbCollection && - !isDbCollectionPublished && - this.collectionService.isLockActive(dbCollection.lock) - ) { - throw new HTTPError( - "Locked collection items can't be updated", - { id }, - STATUS_CODES.locked - ) - } - - const dbItemCollection = await findCollection(dbItem?.collection_id) - - const isDbItemCollectionPublished = - dbItemCollection && - (await this.collectionService.isPublished( - dbItemCollection.contract_address! - )) - - if (isDbItemCollectionPublished && dbItem) { - const isItemBeingRemovedFromCollection = - !itemJSON.collection_id && dbItem.collection_id - - if (isItemBeingRemovedFromCollection) { + } else if (error instanceof DCLItemAlreadyPublishedError) { throw new HTTPError( - "Items can't be removed from a pubished collection", - { id }, - STATUS_CODES.badRequest + error.message, + { id: error.id }, + STATUS_CODES.conflict ) + } else if (error instanceof CollectionForItemLockedError) { + throw new HTTPError(error.message, { id }, STATUS_CODES.locked) } - } - const attributes = toDBItem({ - ...itemJSON, - eth_address, - }) - - const item: ItemAttributes = await new Item(attributes).upsert() - return Bridge.toFullItem(item) + throw error + } } deleteItem = async (req: AuthRequest): Promise => { @@ -489,7 +409,6 @@ export class ItemRouter extends Router { error.message, { id: error.id, - blockchain_item_id: error.blockchainItemId, contract_address: error.contractAddress, }, STATUS_CODES.conflict diff --git a/src/Item/Item.service.ts b/src/Item/Item.service.ts index 30ede029..dfd4c66a 100644 --- a/src/Item/Item.service.ts +++ b/src/Item/Item.service.ts @@ -1,6 +1,8 @@ import { CollectionService } from '../Collection/Collection.service' import { isTPCollection } from '../Collection/utils' +import { Bridge } from '../ethereum/api/Bridge' import { thirdPartyAPI } from '../ethereum/api/thirdParty' +import { Ownable } from '../Ownable' import { CollectionForItemLockedError, ItemAction, @@ -8,10 +10,13 @@ import { InconsistentItemError, DCLItemAlreadyPublishedError, ThirdPartyItemAlreadyPublishedError, + ItemCantBeMovedFromCollectionError, + UnauthorizedToUpsertError, + UnauthorizedToChangeToCollection, } from './Item.errors' import { Item } from './Item.model' -import { ItemAttributes } from './Item.types' -import { buildTPItemURN, isTPItem } from './utils' +import { FullItem, ItemAttributes } from './Item.types' +import { buildTPItemURN, isTPItem, toDBItem } from './utils' export class ItemService { private collectionService = new CollectionService() @@ -43,7 +48,6 @@ export class ItemService { ) { throw new DCLItemAlreadyPublishedError( dbItem.id, - dbItem.blockchain_item_id!, dbCollection.contract_address!, ItemAction.DELETE ) @@ -106,4 +110,113 @@ export class ItemService { await this.deleteDCLItem(dbItem) } } + + private async checkIfItemIsMovedToAnotherCollection( + itemToUpsert: FullItem, + dbItem: ItemAttributes + ): Promise { + const areBothCollectionIdsDefined = + itemToUpsert.collection_id && dbItem.collection_id + + const isItemCollectionBeingChanged = + areBothCollectionIdsDefined && + itemToUpsert.collection_id !== dbItem.collection_id + + if (isItemCollectionBeingChanged) { + throw new ItemCantBeMovedFromCollectionError(itemToUpsert.id) + } + } + + private async upsertDCLItem( + item: FullItem, + eth_address: string + ): Promise { + const canUpsert = await new Ownable(Item).canUpsert(item.id, eth_address) + if (!canUpsert) { + throw new UnauthorizedToUpsertError(item.id, eth_address) + } + + const dbItem = await Item.findOne(item.id) + if (dbItem) { + await this.checkIfItemIsMovedToAnotherCollection(item, dbItem) + } + + const collectionId = item.collection_id || dbItem?.collection_id + const dbCollection = collectionId + ? await this.collectionService.getDBCollection(collectionId) + : undefined + + if (dbCollection) { + const isCollectionOwnerDifferent = + dbCollection.eth_address.toLowerCase() !== eth_address + + if (isCollectionOwnerDifferent) { + throw new UnauthorizedToChangeToCollection( + item.id, + eth_address, + item.collection_id! + ) + } + + const isDbCollectionPublished = + dbCollection && + (await this.collectionService.isPublished( + dbCollection.contract_address! + )) + + if (isDbCollectionPublished) { + // Prohibits adding new items to a published collection + if (!dbItem) { + throw new DCLItemAlreadyPublishedError( + item.id, + dbCollection.contract_address!, + ItemAction.INSERT + ) + } + + // Prohibits removing an item from a published collection + const isItemBeingRemovedFromCollection = + !item.collection_id && dbItem.collection_id + + if (isItemBeingRemovedFromCollection) { + throw new DCLItemAlreadyPublishedError( + item.id, + dbCollection.contract_address!, + ItemAction.DELETE + ) + } + + // Prohibits changing the rarity of a published item. + const areBothRaritiesDefined = item.rarity && dbItem.rarity + + const isRarityBeingChanged = + areBothRaritiesDefined && item.rarity !== dbItem.rarity + + if (isRarityBeingChanged) { + throw new DCLItemAlreadyPublishedError( + item.id, + dbCollection.contract_address!, + ItemAction.RARITY_UPDATE + ) + } + } else if (this.collectionService.isLockActive(dbCollection.lock)) { + throw new CollectionForItemLockedError(item.id, ItemAction.UPSERT) + } + } + + const attributes = toDBItem({ + ...item, + eth_address, + }) + + const upsertedItem: ItemAttributes = await new Item(attributes).upsert() + return Bridge.toFullItem(upsertedItem) + } + + public async upsertItem( + item: FullItem, + eth_address: string + ): Promise { + return this.upsertDCLItem(item, eth_address) + } }