Skip to content

Commit

Permalink
fix for #409
Browse files Browse the repository at this point in the history
added a new suite of tests for asserting area behaviors at the data model level.
  • Loading branch information
CocoisBuggy committed Nov 8, 2024
1 parent df91b92 commit be379f6
Show file tree
Hide file tree
Showing 4 changed files with 227 additions and 25 deletions.
45 changes: 32 additions & 13 deletions src/model/MutableAreaDataSource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void> {
// 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<AreaType | null> {
const session = await this.areaModel.startSession()
let ret: AreaType | null = null
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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))
}
12 changes: 2 additions & 10 deletions src/model/__tests__/AreaUtils.ts
Original file line number Diff line number Diff line change
@@ -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')
})
191 changes: 191 additions & 0 deletions src/model/__tests__/MutableAreaDataSource.test.ts
Original file line number Diff line number Diff line change
@@ -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<ChangeRecordMetadataType>)
}))

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<Omit<AreaType, 'metadata'> & { metadata: Partial<AreaType['metadata']>}>)
}))

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<Omit<AreaType, 'metadata'> & { metadata: Partial<AreaType['metadata']>}>)
}))

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<ChangeRecordMetadataType>)
}))

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 })
}))
})
4 changes: 2 additions & 2 deletions src/model/__tests__/updateAreas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down

0 comments on commit be379f6

Please sign in to comment.