diff --git a/teammapper-backend/src/map/controllers/maps.controller.ts b/teammapper-backend/src/map/controllers/maps.controller.ts index 8ffde3c1..840b1b88 100644 --- a/teammapper-backend/src/map/controllers/maps.controller.ts +++ b/teammapper-backend/src/map/controllers/maps.controller.ts @@ -6,6 +6,7 @@ import { NotFoundException, Param, Post, + Logger } from '@nestjs/common' import { MapsService } from '../services/maps.service' import { @@ -19,6 +20,7 @@ import { EntityNotFoundError } from 'typeorm' @Controller('api/maps') export default class MapsController { + private readonly logger = new Logger(MapsController.name) constructor(private mapsService: MapsService) {} @Get(':id') @@ -51,21 +53,28 @@ export default class MapsController { @Post() async create( @Body() body: IMmpClientMapCreateRequest - ): Promise { + ): Promise { const newMap = await this.mapsService.createEmptyMap(body.rootNode) - return { - map: await this.mapsService.exportMapToClient(newMap.id), - adminId: newMap.adminId, - modificationSecret: newMap.modificationSecret, + const exportedMap = await this.mapsService.exportMapToClient(newMap.id) + + if (exportedMap) { + return { + map: exportedMap, + adminId: newMap.adminId, + modificationSecret: newMap.modificationSecret, + } } } @Post(':id/duplicate') async duplicate( @Param('id') mapId: string, - ): Promise { + ): Promise { const oldMap = await this.mapsService.findMap(mapId).catch((e: Error) => { - if (e.name === 'MalformedUUIDError') throw new NotFoundException() + if (e.name === 'MalformedUUIDError') { + this.logger.warn(`:id/duplicate(): Wrong/no UUID provided for findMap() with mapId ${mapId}`) + return; + } }) if (!oldMap) throw new NotFoundException() @@ -75,11 +84,15 @@ export default class MapsController { const oldNodes = await this.mapsService.findNodes(oldMap.id) await this.mapsService.addNodes(newMap.id, oldNodes) - - return { - map: await this.mapsService.exportMapToClient(newMap.id), - adminId: newMap.adminId, - modificationSecret: newMap.modificationSecret + + const exportedMap = await this.mapsService.exportMapToClient(newMap.id); + + if (exportedMap) { + return { + map: exportedMap, + adminId: newMap.adminId, + modificationSecret: newMap.modificationSecret + } } } } diff --git a/teammapper-backend/src/map/controllers/maps.gateway.ts b/teammapper-backend/src/map/controllers/maps.gateway.ts index d2c5db60..b7864885 100644 --- a/teammapper-backend/src/map/controllers/maps.gateway.ts +++ b/teammapper-backend/src/map/controllers/maps.gateway.ts @@ -1,4 +1,4 @@ -import { Inject, UseGuards } from '@nestjs/common' +import { Inject, UseGuards, Logger } from '@nestjs/common' import { CACHE_MANAGER } from '@nestjs/cache-manager' import { Cache } from 'cache-manager' import { randomBytes } from 'crypto' @@ -37,6 +37,8 @@ export class MapsGateway implements OnGatewayDisconnect { @WebSocketServer() server: Server + private readonly logger = new Logger(MapsService.name) + constructor( private mapsService: MapsService, @Inject(CACHE_MANAGER) private cacheManager: Cache, @@ -57,10 +59,12 @@ export class MapsGateway implements OnGatewayDisconnect { async onJoin( @ConnectedSocket() client: Socket, @MessageBody() request: IMmpClientJoinRequest - ): Promise { + ): Promise { const map = await this.mapsService.findMap(request.mapId) - if (!map) - return Promise.reject() + if (!map) { + this.logger.warn(`onJoin(): Could not find map ${request.mapId} when client ${client.id} tried to join`); + return; + } client.join(request.mapId) this.cacheManager.set(client.id, request.mapId, 10000) diff --git a/teammapper-backend/src/map/services/maps.service.spec.ts b/teammapper-backend/src/map/services/maps.service.spec.ts index 85ae3b36..8dc77762 100644 --- a/teammapper-backend/src/map/services/maps.service.spec.ts +++ b/teammapper-backend/src/map/services/maps.service.spec.ts @@ -5,7 +5,7 @@ import { Logger } from '@nestjs/common' import { TypeOrmModule } from '@nestjs/typeorm' import { MmpMap } from '../entities/mmpMap.entity' import { MmpNode } from '../entities/mmpNode.entity' -import { EntityNotFoundError, Repository } from 'typeorm' +import { Repository } from 'typeorm' import { ConfigModule } from '@nestjs/config' import AppModule from '../../app.module' import { @@ -164,12 +164,10 @@ describe('MapsController', () => { }) describe('exportMapToClient', () => { - it('throws error when no map is available', async () => { - expect( - mapsService.exportMapToClient( - '78a2ae85-1815-46da-a2bc-a41de6bdd5ab' - ) - ).rejects.toThrow(EntityNotFoundError) + it('returns undefined when no map is available', async () => { + expect(await mapsService.exportMapToClient( + '78a2ae85-1815-46da-a2bc-a41de6bdd5ab' + )).toEqual(undefined) }) }) diff --git a/teammapper-backend/src/map/services/maps.service.ts b/teammapper-backend/src/map/services/maps.service.ts index 37f440f3..3ba86e33 100644 --- a/teammapper-backend/src/map/services/maps.service.ts +++ b/teammapper-backend/src/map/services/maps.service.ts @@ -1,6 +1,6 @@ import { Injectable, Logger } from '@nestjs/common' import { InjectRepository } from '@nestjs/typeorm' -import { EntityNotFoundError, Repository } from 'typeorm' +import { Repository } from 'typeorm' import { MmpMap } from '../entities/mmpMap.entity' import { MmpNode } from '../entities/mmpNode.entity' import { @@ -42,32 +42,50 @@ export class MapsService { async updateLastAccessed(uuid: string, lastAccessed = new Date()) { const map = await this.findMap(uuid) - if (!map) return Promise.reject(new EntityNotFoundError("MmpMap", uuid)) + if (!map) { + this.logger.warn(`updateLastAccessed(): Map was not found`) + return; + } this.mapsRepository.update(uuid, { lastAccessed }) } - async exportMapToClient(uuid: string): Promise { + async exportMapToClient(uuid: string): Promise { const map = await this.findMap(uuid) - if (!map) return Promise.reject(new EntityNotFoundError("MmpMap", uuid)) + if (!map) { + this.logger.warn(`exportMapToClient(): Map was not found`) + return; + } const nodes = await this.findNodes(map?.id) const days = configService.deleteAfterDays() - - return mapMmpMapToClient( - map, - nodes, - await this.getDeletedAt(map, days), - days - ) + const deletedAt = await this.getDeletedAt(map, days); + + if (deletedAt) { + return mapMmpMapToClient( + map, + nodes, + deletedAt, + days + ) + } } - async addNode(mapId: string, node: MmpNode): Promise { + async addNode(mapId: string, node: MmpNode): Promise { // detached nodes are not allowed to have a parent - if (node.detached && node.nodeParentId) return Promise.reject() + if (node.detached && node.nodeParentId) { + this.logger.warn(`addNode(): Detached node ${node.id} is not allowed to have a parent.`); + return; + } // root nodes are not allowed to have a parent - if (node.root && node.nodeParentId) return Promise.reject() - if (!mapId || !node) return Promise.reject() + if (node.root && node.nodeParentId) { + this.logger.warn(`addNode(): Root node ${node.id} is not allowed to have a parent.`); + return; + } + if (!mapId || !node) { + this.logger.warn(`addNode(): Required arguments mapId or node not supplied`); + return; + } const existingNode = await this.nodesRepository.findOne({ where: { id: node.id, nodeMapId: mapId }, @@ -82,7 +100,7 @@ export class MapsService { try { return this.nodesRepository.save(newNode) } catch(error) { - this.logger.error(`${error.constructor.name} - Failed to add node ${newNode.id}: ${error}`) + this.logger.warn(`${error.constructor.name} addNode(): Failed to add node ${newNode.id}: ${error}`) return Promise.reject(error) } } @@ -90,7 +108,7 @@ export class MapsService { async addNodesFromClient( mapId: string, clientNodes: IMmpClientNode[] - ): Promise { + ): Promise { const mmpNodes = clientNodes.map(x => mapClientNodeToMmpNode(x, mapId)) return await this.addNodes(mapId, mmpNodes) } @@ -98,19 +116,25 @@ export class MapsService { async addNodes( mapId: string, nodes: Partial[] - ): Promise { - if (!mapId || nodes.length === 0) return Promise.reject() + ): Promise { + if (!mapId || nodes.length === 0) { + this.logger.warn(`Required arguments mapId or nodes not supplied to addNodes()`); + return []; + } const reducer = async (previousPromise: Promise, node: MmpNode): Promise => { const accCreatedNodes = await previousPromise; if (await this.validatesNodeParentForNode(mapId, node)) { try { const newNode = await this.addNode(mapId, node); - return accCreatedNodes.concat([newNode]); + if (newNode) { + return accCreatedNodes.concat([newNode]); + } } catch (error) { this.logger.warn(`Failed to add node ${node.id} to map ${mapId}: ${error}`); - return accCreatedNodes; } + + return accCreatedNodes; } this.logger.warn(`Parent with id ${node.nodeParentId} does not exist for node ${node.id} and map ${mapId}`); @@ -140,12 +164,15 @@ export class MapsService { async updateNode( mapId: string, clientNode: IMmpClientNode - ): Promise { + ): Promise { const existingNode = await this.nodesRepository.findOne({ where: { nodeMapId: mapId, id: clientNode.id }, }) - if (!existingNode) return Promise.reject() + if (!existingNode) { + this.logger.warn(`updateNode(): Existing node on server for given client node ${clientNode.id} has not been found.`); + return; + } try { return this.nodesRepository.save({ @@ -154,7 +181,7 @@ export class MapsService { lastModified: new Date(), }) } catch(error) { - this.logger.error(`${error.constructor.name} - Failed to update node ${existingNode.id}: ${error}`) + this.logger.warn(`${error.constructor.name} updateNode(): Failed to update node ${existingNode.id}: ${error}`) return Promise.reject(error) } } @@ -186,7 +213,7 @@ export class MapsService { try { await this.nodesRepository.save(newRootNode) } catch(error) { - this.logger.error(`${error.constructor.name} - Failed to create root node ${newRootNode.id}: ${error}`) + this.logger.warn(`${error.constructor.name} createEmptyMap(): Failed to create root node ${newRootNode.id}: ${error}`) return Promise.reject(error) } } @@ -226,7 +253,7 @@ export class MapsService { try { await this.nodesRepository.save(serverNode); } catch(error) { - this.logger.error(`${error.constructor.name} - Failed to update node ${serverNode.id} during diff update: ${error}`) + this.logger.warn(`${error.constructor.name} diffUpdatedCallback(): Failed to update node ${serverNode.id}: ${error}`) return Promise.reject(error) } } @@ -274,8 +301,11 @@ export class MapsService { return await this.mapsRepository.findOne({ where: { id: mapId } }) } - async getDeletedAt(map: MmpMap, afterDays: number): Promise { - if (!map) return Promise.reject() + async getDeletedAt(map: MmpMap, afterDays: number): Promise { + if (!map) { + this.logger.warn(`Required argument map was not supplied to getDeletedAt()`); + return; + } // get newest node of this map: const newestNodeQuery = this.nodesRepository