diff --git a/src/model/MutableAreaDataSource.ts b/src/model/MutableAreaDataSource.ts index 41e68329..8b12cb2b 100644 --- a/src/model/MutableAreaDataSource.ts +++ b/src/model/MutableAreaDataSource.ts @@ -53,6 +53,27 @@ export interface UpdateAreaOptions { export default class MutableAreaDataSource extends AreaDataSource { experimentalUserDataSource = createExperimentalUserDataSource() + private areaNameCompare (name: string): string { + return name.trim().toLocaleLowerCase().split(' ').filter(i => i !== '').join(' ') + } + + private async validateUniqueAreaName (areaName: string, parent: AreaType | null): Promise { + // area names must be unique in a document area structure context, so if the name has changed we need to check + // that the name is unique for this context + let neighbours: string[] + + if (parent !== null) { + neighbours = (await this.areaModel.find({ _id: parent.children })).map(i => i.area_name) + } else { + neighbours = (await this.areaModel.find({ pathTokens: { $size: 1 } })).map(i => i.area_name) + } + + neighbours = neighbours.map(i => this.areaNameCompare(i)) + if (neighbours.includes(this.areaNameCompare(areaName))) { + throw new UserInputError(`[${areaName}]: This name already exists for some other area in this parent`) + } + } + async setDestinationFlag (user: MUUID, uuid: MUUID, flag: boolean): Promise { const session = await this.areaModel.startSession() let ret: AreaType | null = null @@ -119,6 +140,8 @@ export default class MutableAreaDataSource extends AreaDataSource { logger.warn(`Missing lnglat for ${countryName}`) } + await this.validateUniqueAreaName(countryName, null) + const rs = await this.areaModel.insertMany(doc) if (rs.length === 1) { return await rs[0].toObject() @@ -194,6 +217,8 @@ export default class MutableAreaDataSource extends AreaDataSource { parent.metadata.isBoulder = false } + await this.validateUniqueAreaName(areaName, parent) + // See https://github.com/OpenBeta/openbeta-graphql/issues/244 let experimentaAuthorId: MUUID | null = null if (experimentalAuthor != null) { @@ -377,6 +402,12 @@ export default class MutableAreaDataSource extends AreaDataSource { experimentalAuthorId = await this.experimentalUserDataSource.updateUser(session, experimentalAuthor.displayName, experimentalAuthor.url) } + // area names must be unique in a document area structure context, so if the name has changed we need to check + // that the name is unique for this context + if (areaName !== undefined && this.areaNameCompare(areaName) !== this.areaNameCompare(area.area_name)) { + await this.validateUniqueAreaName(areaName, await this.areaModel.findOne({ children: area._id }).session(session)) + } + const opType = OperationType.updateArea const change = await changelogDataSource.create(session, user, opType) @@ -611,7 +642,7 @@ export default class MutableAreaDataSource extends AreaDataSource { export const newAreaHelper = (areaName: string, parentAncestors: string, parentPathTokens: string[], parentGradeContext: GradeContexts): AreaType => { const _id = new mongoose.Types.ObjectId() - const uuid = genMUIDFromPaths(parentPathTokens, areaName) + const uuid = muuid.v4() const pathTokens = produce(parentPathTokens, draft => { draft.push(areaName) @@ -663,15 +694,3 @@ export const countryCode2Uuid = (code: string): MUUID => { const alpha3 = code.length === 2 ? isoCountries.toAlpha3(code) : code return muuid.from(uuidv5(alpha3.toUpperCase(), NIL)) } - -/** - * Generate a stable UUID from a list of paths. Example: `Canada|Squamish => 8f623793-c2b2-59e0-9e64-d167097e3a3d` - * @param parentPathTokens Ancestor paths - * @param thisPath Current area - * @returns MUUID - */ -export const genMUIDFromPaths = (parentPathTokens: string[], thisPath: string): MUUID => { - const keys = parentPathTokens.slice() // clone array - keys.push(thisPath) - return muuid.from(uuidv5(keys.join('|').toUpperCase(), NIL)) -} diff --git a/src/model/__tests__/AreaUtils.ts b/src/model/__tests__/AreaUtils.ts index 55d00ee5..0f30849b 100644 --- a/src/model/__tests__/AreaUtils.ts +++ b/src/model/__tests__/AreaUtils.ts @@ -1,12 +1,4 @@ -// import muid from 'uuid-mongodb' -import { genMUIDFromPaths } from '../MutableAreaDataSource.js' - describe('Test area utilities', () => { - it('generates UUID from area tokens', () => { - const paths = ['USA', 'Red Rocks'] - const uid = genMUIDFromPaths(paths, 'Calico Basin') - - expect(uid.toUUID().toString()).toEqual('9fbc30ab-82d7-5d74-85b1-9bec5ee00388') - expect(paths).toHaveLength(2) // make sure we're not changing the input! - }) + test.todo('The name comparison code unit') + test.todo('The name-uniqueness system with other side-effects stripped out') }) diff --git a/src/model/__tests__/MutableAreaDataSource.test.ts b/src/model/__tests__/MutableAreaDataSource.test.ts new file mode 100644 index 00000000..1250036c --- /dev/null +++ b/src/model/__tests__/MutableAreaDataSource.test.ts @@ -0,0 +1,191 @@ +import { getAreaModel, createIndexes } from "../../db" +import inMemoryDB from "../../utils/inMemoryDB" +import MutableAreaDataSource from "../MutableAreaDataSource" +import muid, { MUUID } from 'uuid-mongodb' +import { AreaType, OperationType } from "../../db/AreaTypes" +import { ChangeRecordMetadataType } from "../../db/ChangeLogType" +import { UserInputError } from "apollo-server-core" +import mongoose from "mongoose" +import exp from "constants" + + +describe("Test area mutations", () => { + let areas: MutableAreaDataSource + let rootCountry: AreaType + let areaCounter = 0 + const testUser = muid.v4() + + async function addArea(name?: string, extra?: Partial<{ leaf: boolean, boulder: boolean, parent: MUUID | AreaType}>) { + function isArea(x: any): x is AreaType { + return typeof x.metadata?.area_id !== 'undefined' + } + + areaCounter += 1 + if (name === undefined || name === 'test') { + name = process.uptime().toString() + '-' + areaCounter.toString() + } + + let parent: MUUID | undefined = undefined + if (extra?.parent) { + if (isArea(extra.parent)) { + parent = extra.parent.metadata?.area_id + } else { + parent = extra.parent + } + } + + return areas.addArea( + testUser, + name, + parent ?? rootCountry.metadata.area_id, + undefined, + undefined, + extra?.leaf, + extra?.boulder + ) + } + + beforeAll(async () => { + await inMemoryDB.connect() + await getAreaModel().collection.drop() + await createIndexes() + + areas = MutableAreaDataSource.getInstance() + // We need a root country, and it is beyond the scope of these tests + rootCountry = await areas.addCountry("USA") + }) + + afterAll(inMemoryDB.close) + + describe("Add area param cases", () => { + test("Add a simple area with no specifications using a parent UUID", () => areas + .addArea(testUser, 'Texas2', rootCountry.metadata.area_id) + .then(area => { + expect(area?._change).toMatchObject({ + user: testUser, + operation: OperationType.addArea, + } satisfies Partial) + })) + + test("Add an area with an unknown UUID parent should fail", + async () => await expect(() => areas.addArea(testUser, 'Texas', muid.v4())).rejects.toThrow()) + + test("Add a simple area with no specifications using a country code", () => areas.addArea(testUser, 'Texas part 2', null, 'USA') + .then(texas => areas.addArea(testUser, 'Texas Child', texas.metadata.area_id))) + + test("Add a simple area, then specify a new child one level deep", () => addArea('California') + .then(async parent => { + let child = await addArea('Child', { parent }) + expect(child).toMatchObject({ area_name: 'Child' }) + let parentCheck = await areas.findOneAreaByUUID(parent.metadata.area_id) + expect(parentCheck?.children ?? []).toContainEqual(child._id) + })) + + test("Add a leaf area", () => addArea('Somewhere').then(parent => addArea('Child', { leaf: true, parent })) + .then(async leaf => { + expect(leaf).toMatchObject({ metadata: { leaf: true }}) + let area = await areas.areaModel.findById(leaf._id) + expect(area).toMatchObject({ metadata: { leaf: true }}) + })) + + test("Add a leaf area that is a boulder", () => addArea('Maine') + .then(parent => addArea('Child', {leaf: true, boulder: true, parent} )) + .then(area => { + expect(area).toMatchObject({ + metadata: { + leaf: true, + isBoulder: true, + }, + } satisfies Partial & { metadata: Partial}>) + })) + + test("Add a NON-leaf area that is a boulder", () => addArea('Wisconcin') + .then(texas => addArea('Child', { leaf: false, boulder: true })) + .then(area => { + expect(area).toMatchObject({ + metadata: { + // Even though we specified false to leaf on the input, we expect it to be true + // after write because a boulder cannot contain sub-areas + leaf: true, + isBoulder: true, + }, + } satisfies Partial & { metadata: Partial}>) + })) + + test("Adding a child to a leaf area should cause it to become a normal area", () => addArea() + .then(parent => Promise.all(new Array(5).map(() => addArea('test', { leaf: true, parent } )))) + .then(([leaf]) => leaf) + .then(leaf => addArea('test', { parent: leaf })) + .then(leaf => expect(leaf).toMatchObject({ metadata: { leaf: false }}))) + + test("area names should be unique in their parent context", () => addArea('test').then(async parent => { + await addArea('Big ol boulder', { parent }) + await expect(() => addArea('Big ol boulder', { parent })).rejects.toThrowError(UserInputError) + })) + }) + + test("Delete Area", () => addArea("test").then(area => areas.deleteArea(testUser, area.metadata.area_id)).then(async deleted => { + expect(deleted).toBeDefined() + // TODO: this test fails based on the data returned, which appears to omit the _deleting field. + let d = await areas.areaModel.findById(deleted?._id) + + expect(d).toBeDefined() + expect(d).not.toBeNull() + expect(d?._deleting).toBeDefined() + })) + + test("Delete Area that is already deleted should throw", () => addArea("test") + .then(area => areas.deleteArea(testUser, area.metadata.area_id)) + .then(async area => { + expect(area).not.toBeNull() + await expect(() => areas.deleteArea(testUser, area!.metadata.area_id)).rejects.toThrow() + })) + + + + describe("Area update cases", () => { + test("Updating an area should superficially pass", () => addArea('test').then(area => areas.updateArea(testUser, area.metadata.area_id, { areaName: `New Name! ${process.uptime()}`}))) + test("Updating an area should produce a change entry in the changelog", () => addArea('test') + .then(area => areas.updateArea(testUser, area.metadata.area_id, { areaName: process.uptime().toString() })) + .then(area => { + expect(area?._change).toMatchObject({ + user: testUser, + operation: OperationType.updateArea, + } satisfies Partial) + })) + + test("Area name uniqueness in its current parent context", () => addArea('test').then(async parent => { + let [area, newArea, divorcedArea] = await Promise.all([ + addArea('original', { parent }), + addArea('wannabe', { parent }), + addArea(undefined, { parent: rootCountry }), + ]) + + await Promise.all([ + // Case where an area gets changed to what it already is, which should not throw an error + areas.updateArea(testUser, area.metadata.area_id, { areaName: area.area_name }), + // name-uniqueness should not be global, so this shouldn't throw + areas.updateArea(testUser, divorcedArea.metadata.area_id, { areaName: area.area_name }), + // if we update one of the areas to have a name for which another area already exists, we should expect this to throw. + expect(() => areas.updateArea(testUser, newArea.metadata.area_id, { areaName: area.area_name })).rejects.toThrowError(UserInputError), + ]) + })) + }) + + test("Area name uniqueness should not create a UUID shadow via deletion", () => addArea('test').then(async parent => { + let name = 'Big ol boulder' + let big = await addArea(name, { boulder: true, parent }) + await areas.deleteArea(testUser, big.metadata.area_id) + await addArea(name, { boulder: true, parent }) + })) + + test("Area name uniqueness should not create a UUID shadow via edit of name", () => addArea('test').then(async parent => { + let nameShadow = 'Big ol boulder 2' + let big = await addArea(nameShadow, { boulder: true, parent }) + + // We change the name of the original owner of the nameshadow, and then try to add a + // name claming the original name in this area structure context + await areas.updateArea(testUser, big.metadata.area_id, { areaName: "Still big ol bolder"}) + await addArea(nameShadow, { boulder: true, parent }) + })) +}) \ No newline at end of file diff --git a/src/model/__tests__/updateAreas.ts b/src/model/__tests__/updateAreas.ts index 347bc52d..4d5a894b 100644 --- a/src/model/__tests__/updateAreas.ts +++ b/src/model/__tests__/updateAreas.ts @@ -224,14 +224,14 @@ describe('Areas', () => { // eslint-disable-next-line await new Promise(res => setTimeout(res, 2000)) - await expect(areas.addCountry('ita')).rejects.toThrowError('E11000 duplicate key error') + await expect(areas.addCountry('ita')).rejects.toThrowError('This name already exists for some other area in this parent') }) it('should not create duplicate sub-areas', async () => { const fr = await areas.addCountry('fra') await areas.addArea(testUser, 'Verdon Gorge', fr.metadata.area_id) await expect(areas.addArea(testUser, 'Verdon Gorge', fr.metadata.area_id)) - .rejects.toThrowError('E11000 duplicate key error') + .rejects.toThrowError('This name already exists for some other area in this parent') }) it('should fail when adding without a parent country', async () => {