diff --git a/src/__tests__/bulkImport.test.ts b/src/__tests__/bulkImport.test.ts index cfcd0ffd..b67aca3e 100644 --- a/src/__tests__/bulkImport.test.ts +++ b/src/__tests__/bulkImport.test.ts @@ -4,11 +4,11 @@ import express from "express"; import {InMemoryDB} from "../utils/inMemoryDB.js"; import {queryAPI, setUpServer} from "../utils/testUtils.js"; import {muuidToString} from "../utils/helpers.js"; -import MutableAreaDataSource from "../model/MutableAreaDataSource.js"; import exampleImportData from './import-example.json' assert {type: 'json'}; import {AreaType} from "../db/AreaTypes.js"; import {BulkImportResultType} from "../db/BulkImportTypes.js"; import MutableClimbDataSource from "../model/MutableClimbDataSource.js"; +import BulkImportDataSource from "../model/BulkImportDataSource.js"; describe('bulkImportAreas', () => { const query = ` @@ -16,9 +16,15 @@ describe('bulkImportAreas', () => { bulkImportAreas(input: $input) { addedAreas { uuid + metadata { + area_id + } } updatedAreas { uuid + metadata { + area_id + } } addedOrUpdatedClimbs { id @@ -34,7 +40,7 @@ describe('bulkImportAreas', () => { let inMemoryDB: InMemoryDB let testArea: AreaType - let areas: MutableAreaDataSource + let bulkImport: BulkImportDataSource let climbs: MutableClimbDataSource beforeAll(async () => { @@ -44,14 +50,14 @@ describe('bulkImportAreas', () => { user = muuid.mode('relaxed').v4() userUuid = muuidToString(user) - areas = MutableAreaDataSource.getInstance() + bulkImport = BulkImportDataSource.getInstance() climbs = MutableClimbDataSource.getInstance() }) beforeEach(async () => { await inMemoryDB.clear() - await areas.addCountry('usa') - testArea = await areas.addArea(user, "Test Area", null, "us") + await bulkImport.addCountry('usa') + testArea = await bulkImport.addArea(user, "Test Area", null, "us") }) afterAll(async () => { @@ -106,25 +112,25 @@ describe('bulkImportAreas', () => { areas: [ ...exampleImportData.areas, { - uuid: testArea.metadata.area_id.toUUID().toString(), + uuid: testArea.metadata.area_id, areaName: "Updated Test Area", } ] } } }); - expect(res.status).toBe(200) + expect(res.body.errors).toBeFalsy() - const result = res.body.data as BulkImportResultType + const result = res.body.data.bulkImportAreas as BulkImportResultType expect(result.addedAreas.length).toBe(4) - const committedAreas = await Promise.all(result.addedAreas.map((area) => areas.findOneAreaByUUID(area.metadata.area_id))); + const committedAreas = await Promise.all(result.addedAreas.map((area) => bulkImport.findOneAreaByUUID(muuid.from(area.metadata.area_id)))); expect(committedAreas.length).toBe(4); const committedClimbs = await Promise.all(result.addedOrUpdatedClimbs.map((climb) => climbs.findOneClimbByMUUID(climb._id))); expect(committedClimbs.length).toBe(2); - const updatedAreas = await Promise.all(result.updatedAreas.map((area) => areas.findOneAreaByUUID(area.metadata.area_id))); + const updatedAreas = await Promise.all(result.updatedAreas.map((area) => bulkImport.findOneAreaByUUID(muuid.from(area.metadata.area_id)))); expect(updatedAreas.length).toBe(1); expect(updatedAreas[0].area_name).toBe("Updated Test Area"); }) diff --git a/src/graphql/schema/AreaEdit.gql b/src/graphql/schema/AreaEdit.gql index 91718ba1..b13d807b 100644 --- a/src/graphql/schema/AreaEdit.gql +++ b/src/graphql/schema/AreaEdit.gql @@ -29,7 +29,7 @@ type Mutation { You can start at any point in the tree given a valid parent area with its id. If starting at the root level, the countryCode must be provided. """ - bulkImportAreas(input: BulkImportInput): [BulkImportResult] + bulkImportAreas(input: BulkImportInput): BulkImportResult } input DestinationFlagInput { @@ -71,7 +71,7 @@ input BulkImportAreaInput { areaName: String "An optional description of the area. If provided with an id, the description will be updated" description: String - "Only relevant for the top most areas of a country. Get a list of valid countries by querying the countries field" + "Only relevant for the top most areas of a country. Get a list of valid countries by using the countries query" countryCode: String "The grade context of the area. Every area that has climbs must have a valid grade context" gradeContext: String diff --git a/src/model/BulkImportDataSource.ts b/src/model/BulkImportDataSource.ts index d869ed33..6ed62f64 100644 --- a/src/model/BulkImportDataSource.ts +++ b/src/model/BulkImportDataSource.ts @@ -1,5 +1,5 @@ import MutableAreaDataSource from './MutableAreaDataSource.js' -import mongoose from 'mongoose' +import mongoose, { ClientSession } from 'mongoose' import { withTransaction } from '../utils/helpers.js' import muuid, { MUUID } from 'uuid-mongodb' import { AreaType } from '../db/AreaTypes.js' @@ -48,7 +48,9 @@ export default class BulkImportDataSource extends MutableAreaDataSource { logger.error('bulk import failed', e) throw e } finally { - await session.endSession() + if (!session.hasEnded) { + await session.endSession() + } } } @@ -71,7 +73,7 @@ export default class BulkImportDataSource extends MutableAreaDataSource { if (areaNode.uuid !== undefined && areaNode.uuid !== null) { area = await this.updateAreaWith({ user, - areaUuid: areaNode.uuid, + areaUuid: muuid.from(areaNode.uuid), document: { areaName: areaNode.areaName, description: areaNode.description, @@ -118,16 +120,17 @@ export default class BulkImportDataSource extends MutableAreaDataSource { } } if (areaNode.climbs !== undefined) { - result.addedOrUpdatedClimbs.push(...(await climbs?.addOrUpdateClimbsWith({ + const addedOrUpdatedClimbs = await Promise.all(await climbs?.addOrUpdateClimbsWith({ userId: user, parentId: area.metadata.area_id, changes: [...areaNode.climbs.map(this.toClimbChangeInputType) ?? []], session - }).then((climbIds) => climbIds.map(async (id) => - await climbs?.findOneClimbByMUUID(muuid.from(id))) + }).then((climbIds) => climbIds + .map((id) => climbs?.climbModel.findById(muuid.from(id)).session(session as ClientSession))) ?? []) + result.addedOrUpdatedClimbs.push(...addedOrUpdatedClimbs .filter((climb) => climb !== null) - .map((climb) => climb as unknown as ClimbType)) - )) + .map((climb) => climb as unknown as ClimbType) + ) } return result } diff --git a/src/model/MutableAreaDataSource.ts b/src/model/MutableAreaDataSource.ts index 7beaa139..eefc149d 100644 --- a/src/model/MutableAreaDataSource.ts +++ b/src/model/MutableAreaDataSource.ts @@ -158,20 +158,26 @@ export default class MutableAreaDataSource extends AreaDataSource { throw new Error(`Adding area "${areaName}" failed. Must provide parent Id or country code`) } - let uuid: MUUID + let parentId: MUUID if (parentUuid != null) { - uuid = parentUuid + parentId = parentUuid } else if (countryCode != null) { - uuid = countryCode2Uuid(countryCode) + parentId = countryCode2Uuid(countryCode) } else { throw new Error(`Adding area "${areaName}" failed. Unable to determine parent id or country code`) } const session = sessionCtx ?? await this.areaModel.startSession() - if (session.inTransaction()) { - return await this._addArea(session, user, areaName, uuid, experimentalAuthor, isLeaf, isBoulder) - } else { - return await withTransaction(session, async () => await this._addArea(session, user, areaName, uuid, experimentalAuthor, isLeaf, isBoulder)) + try { + if (session.inTransaction()) { + return await this._addArea(session, user, areaName, parentId, experimentalAuthor, isLeaf, isBoulder) + } else { + return await withTransaction(session, async () => await this._addArea(session, user, areaName, parentId, experimentalAuthor, isLeaf, isBoulder)) + } + } finally { + if (sessionCtx == null) { + await session.endSession() + } } } @@ -338,6 +344,7 @@ export default class MutableAreaDataSource extends AreaDataSource { * @param user * @param areaUuid Area uuid to be updated * @param document New fields + * @param sessionCtx optional existing session to use for the transactions * @returns Newly updated area */ async updateArea (user: MUUID, areaUuid: MUUID, document: AreaEditableFieldsType, sessionCtx?: ClientSession): Promise { @@ -349,7 +356,7 @@ export default class MutableAreaDataSource extends AreaDataSource { const area = await this.areaModel.findOne(filter).session(session) if (area == null) { - throw new Error('Area update error. Reason: Area not found.') + throw new Error(`Area update error. Reason: Area with id ${areaUuid.toString()} not found.`) } const { @@ -365,23 +372,23 @@ export default class MutableAreaDataSource extends AreaDataSource { } = document // See https://github.com/OpenBeta/openbeta-graphql/issues/244 - let experimentaAuthorId: MUUID | null = null + let experimentalAuthorId: MUUID | null = null if (experimentalAuthor != null) { - experimentaAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url) + experimentalAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url) } const opType = OperationType.updateArea const change = await changelogDataSource.create(session, user, opType) const _change: ChangeRecordMetadataType = { - user: experimentaAuthorId ?? user, + user: experimentalAuthorId ?? user, historyId: change._id, prevHistoryId: area._change?.historyId._id, operation: opType, seq: 0 } area.set({ _change }) - area.updatedBy = experimentaAuthorId ?? user + area.updatedBy = experimentalAuthorId ?? user if (area.pathTokens.length === 1) { if (areaName != null || shortCode != null) throw new Error(`[${area.area_name}]: Area update error. Reason: Updating country name or short code is not allowed.`) @@ -435,10 +442,16 @@ export default class MutableAreaDataSource extends AreaDataSource { } const session = sessionCtx ?? await this.areaModel.startSession() - if (session.inTransaction()) { - return await _updateArea(session, user, areaUuid, document) - } else { - return await withTransaction(session, async () => await _updateArea(session, user, areaUuid, document)) + try { + if (session.inTransaction()) { + return await _updateArea(session, user, areaUuid, document) + } else { + return await withTransaction(session, async () => await _updateArea(session, user, areaUuid, document)) + } + } finally { + if (sessionCtx == null) { + await session.endSession() + } } } diff --git a/src/server.ts b/src/server.ts index 4335d6f9..20f661aa 100644 --- a/src/server.ts +++ b/src/server.ts @@ -20,6 +20,7 @@ import type { DataSources } from 'apollo-server-core/dist/graphqlOptions' import UserDataSource from './model/UserDataSource.js' import express from 'express' import * as http from 'http' +import BulkImportDataSource from './model/BulkImportDataSource' export async function createServer (): Promise<{ app: express.Application, server: ApolloServer }> { const schema = applyMiddleware( @@ -29,6 +30,7 @@ export async function createServer (): Promise<{ app: express.Application, serve const dataSources: () => DataSources = () => ({ climbs: MutableClimbDataSource.getInstance(), areas: MutableAreaDataSource.getInstance(), + bulkImport: BulkImportDataSource.getInstance(), organizations: MutableOrgDS.getInstance(), ticks: TickDataSource.getInstance(), history: ChangeLogDataSource.getInstance(),