diff --git a/CLAUDE.md b/CLAUDE.md index 770668c3..dffd79f5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -165,6 +165,17 @@ Project Lead ([export/tdei.vue](pages/workspace/[id]/export/tdei.vue)). A Lead without one of those TDEI roles cannot export; a non-lead with `poc` can. * Validate Changeset — ✅ enforced (`isValidator`, [review/Toolbar.vue](components/review/Toolbar.vue)) +* Merge Changes From Another Workspace: ⚠️ enforced in the UI only (`isLead`, + on the menu entry in [Toolbar.vue](components/dashboard/Toolbar.vue) and on + both [merge/index.vue](pages/workspace/[id]/merge/index.vue) and + [merge/conflicts.vue](pages/workspace/[id]/merge/conflicts.vue)). **No + server-side merge endpoint exists**, so the merge runs entirely in the browser + and nothing checks the `lead` role on the server. The one backend control on + the path is `POST /api/v1/workspaces`, which rejects a project group the + caller does not belong to. Merge targets are restricted to workspaces built from the same TDEI dataset + **and** in the same project group, and the destination picker is constrained + to the workspace's own project group, so a merge cannot move data across a + tenant boundary. * Move Workspace from Project Group to Project Group — ❌ **not implemented.** No `move`/`transfer`/`changeProjectGroup` code exists anywhere in the frontend. * Edit POSM Element — ✅ available, but **not role-gated** (see note below) @@ -190,8 +201,9 @@ would have to be enforced by the backend / changeset-validation flow. ## Test status -The e2e suite currently passes aside from **6 intentionally-skipped tests**: 6 -`test.fixme` dashboard flows blocked by maplibre/external-editor rendering. Some +The e2e suite currently passes aside from **7 intentionally-skipped tests**: 6 +`test.fixme` dashboard flows blocked by maplibre/external-editor rendering, and +the merge conflicts map-and-tag-table flow, which needs WebGL. Some heavy authed pages (e.g. `settings/members`) can flake under high parallelism (the lazy-compile issue above) — run serially (`--workers=1`) for deterministic results. diff --git a/components/dashboard/Toolbar.vue b/components/dashboard/Toolbar.vue index 4538ca2b..e768f4d4 100644 --- a/components/dashboard/Toolbar.vue +++ b/components/dashboard/Toolbar.vue @@ -139,6 +139,15 @@ Export + + + Merge + + Settings @@ -178,6 +187,7 @@ const editHash = computed(() => { }); const isOsw = computed(() => props.workspace.type === 'osw'); +const isLead = computed(() => props.workspace.role === 'lead'); const editorLabel = computed(() => { switch (props.workspace.type) { case 'osw': @@ -202,6 +212,7 @@ const editRouteRapid3 = computed(() => ({ const reviewRoute = computed(() => workspacePath('review')); const projectsRoute = computed(() => workspacePath('projects')); const exportRoute = computed(() => workspacePath('export')); +const mergeRoute = computed(() => workspacePath('merge')); const settingsRoute = computed(() => workspacePath('settings')); function workspacePath(page: string): string { diff --git a/components/merge/ConflictDetail.vue b/components/merge/ConflictDetail.vue new file mode 100644 index 00000000..8d98990f --- /dev/null +++ b/components/merge/ConflictDetail.vue @@ -0,0 +1,177 @@ + + + + + diff --git a/components/merge/ConflictList.vue b/components/merge/ConflictList.vue new file mode 100644 index 00000000..2575e131 --- /dev/null +++ b/components/merge/ConflictList.vue @@ -0,0 +1,71 @@ + + + + + diff --git a/components/merge/ConflictMap.vue b/components/merge/ConflictMap.vue new file mode 100644 index 00000000..055b3361 --- /dev/null +++ b/components/merge/ConflictMap.vue @@ -0,0 +1,325 @@ + + + + + diff --git a/components/merge/ConflictSidebar.vue b/components/merge/ConflictSidebar.vue new file mode 100644 index 00000000..bacf1cb5 --- /dev/null +++ b/components/merge/ConflictSidebar.vue @@ -0,0 +1,82 @@ + + + + + diff --git a/components/merge/ConflictStatusAlerts.vue b/components/merge/ConflictStatusAlerts.vue new file mode 100644 index 00000000..7fb5cfb8 --- /dev/null +++ b/components/merge/ConflictStatusAlerts.vue @@ -0,0 +1,34 @@ + + + diff --git a/components/merge/ConflictTagTable.vue b/components/merge/ConflictTagTable.vue new file mode 100644 index 00000000..9e856d9c --- /dev/null +++ b/components/merge/ConflictTagTable.vue @@ -0,0 +1,202 @@ + + + + + diff --git a/components/review/Map.vue b/components/review/Map.vue index 80e899bd..724b3c10 100644 --- a/components/review/Map.vue +++ b/components/review/Map.vue @@ -5,49 +5,18 @@ /> - - + + diff --git a/pages/workspace/[id]/merge/index.vue b/pages/workspace/[id]/merge/index.vue new file mode 100644 index 00000000..10a6f05c --- /dev/null +++ b/pages/workspace/[id]/merge/index.vue @@ -0,0 +1,332 @@ +// Test outline +// @test e2e: loading this page as a workspace owner shows a form listing the workspaces eligible to merge with this +one (playwright snapshot this) +// @test e2e: loading this page as a non-owner shows the "only workspace owners can merge" warning and leaves the Merge +button disabled (assert() the button is disabled) +// @test e2e: a workspace with no eligible merge targets shows the "no existing workspaces will merge" panel with a +link back to the dashboard (playwright snapshot this) +// @test e2e: the project group picker offers only the workspace's own project group, so a merge cannot relocate data +into another tenant (assert() the options list has exactly one entry) +// @test e2e: entering a title already used in the project group shows the "not available" hint and leaves Merge +disabled (assert() this) +// @test e2e: the form warns that the merged workspace will not be linked to a TDEI dataset, and for a GTFS Pathways +workspace that it then cannot be exported to TDEI (assert() the warning text for each type) +// @test e2e: merging two workspaces with no conflicts commits the merge and navigates to the new workspace's editor, +centred on the merged data (assert() the URL) +// @test e2e: merging two workspaces with conflicts navigates to the conflicts page instead of committing (assert() +the URL) +// @test e2e: a failing merge shows an error toast and re-enables the Merge button rather than failing silently +(assert() the toast text) +// @test e2e: validate that all the API calls used on this page match the Swagger spec +(https://new-api.workspaces-stage.sidewalks.washington.edu/openapi.json) + + + + diff --git a/services/export/tdei.ts b/services/export/tdei.ts index 87675fb0..9c40e7ff 100644 --- a/services/export/tdei.ts +++ b/services/export/tdei.ts @@ -133,16 +133,28 @@ export class TdeiExporter { this._context.status = status.validate; this._validate(workspace); + this._context.status = status.bbox; + const bbox = await this._osmClient.getWorkspaceBbox(workspace.id); + + // No bounding box means the workspace has never held a node. The OSM reads + // reduce that to an empty result rather than failing, which suits an + // archive download but not an upload: an empty dataset published to TDEI + // cannot be undone from here. The API computes the box over every node + // version, deleted ones included, so a workspace whose data was all + // deleted still has one and is not refused here. + // + if (!bbox) { + throw new TdeiExporterValidationError( + `Workspace "${workspace.title}" holds no data, so there is nothing to export. ` + + 'Add data to it, then export again.' + ); + } + if (!metadata.dataset_area) { - this._context.status = status.bbox; - const bbox = await this._osmClient.getWorkspaceBbox(workspace.id); - - if (bbox) { - const { min_lat, min_lon, max_lat, max_lon } = bbox - metadata.dataset_area = geojson.featureCollection([ - geojson.bboxToPolygon(min_lat, min_lon, max_lat, max_lon) - ]) - } + const { min_lat, min_lon, max_lat, max_lon } = bbox; + metadata.dataset_area = geojson.featureCollection([ + geojson.bboxToPolygon(min_lat, min_lon, max_lat, max_lon) + ]); } let changesetArchive: Blob | undefined; diff --git a/services/index.ts b/services/index.ts index 06a02fa1..ac28cedc 100644 --- a/services/index.ts +++ b/services/index.ts @@ -8,6 +8,7 @@ import { ChangesetManager, } from '~/services/changesets'; import { OsmApiClient } from '~/services/osm'; +import { WorkspaceMergerFactory } from '~/services/merge'; import { PathwaysEditorManager } from '~/services/pathways'; import { ProjectWizardClient } from '~/services/project-wizard'; import { ReviewManager } from '~/services/review'; @@ -63,6 +64,11 @@ export const reviewManager = new ReviewManager( workspacesClient, ); +export const workspaceMergerFactory = new WorkspaceMergerFactory( + workspacesClient, + osmClient, +); + export const rapidManager = new RapidManager(rapidUrl, osmWebUrl, tdeiAuth); export const rapid3Manager = rapid3Url ? new Rapid3Manager(rapid3Url, osmWebUrl, tdeiAuth) : null; export const pathwaysManager = new PathwaysEditorManager(pathwaysUrl, osmWebUrl, tdeiAuth); diff --git a/services/merge/index.ts b/services/merge/index.ts new file mode 100644 index 00000000..57dfc8e5 --- /dev/null +++ b/services/merge/index.ts @@ -0,0 +1,16 @@ +export { applyResolutions, threeWayMerge } from '~/services/merge/three-way'; +export { findDanglingReferences } from '~/services/merge/references'; +export { + findImportMismatch, + findUnusableTags, + MAX_TAG_LENGTH +} from '~/services/merge/validation'; +export { ThreeWayWorkspaceMerger, WorkspaceMergerFactory } from '~/services/merge/workspace'; + +export type { + ConflictChoice, + ConflictResolution, + MergeConflict, + MergeConflictKind, + MergeResult +} from '~/services/merge/types'; diff --git a/services/merge/references.ts b/services/merge/references.ts new file mode 100644 index 00000000..60430219 --- /dev/null +++ b/services/merge/references.ts @@ -0,0 +1,102 @@ +import { copyOsmElement, OsmElementLookup } from '~/util/osm'; + +import type { MergeResult } from '~/services/merge/types'; +import type { OsmElement, OsmElementType } from '~/types/osm'; + +export function referenceKey(type: OsmElementType, id: number): string { + return `${type}/${id}`; +} + +/** + * Puts back elements a set still references. + * + * A side that deletes a way normally deletes its vertices too. When the other + * side edited that way, the vertices are dropped as an uncontested deletion + * while the way itself becomes a conflict, so keeping the way leaves it + * pointing at nothing. The last agreed version of those vertices is the + * ancestor's, which is what a kept way was drawn over. + * + * `excluded` names elements the user resolved as deleted. Those stay out, and + * the reference that still needs them is left for `findDanglingReferences` to + * report rather than silently reversing the decision. + * + * Restored elements can themselves reference more, so this repeats until + * nothing further is found. Used for both a resolved set and, through + * `repairReferences`, a merge that produced no conflicts at all. + */ +export function restoreMissingReferences( + output: OsmElementLookup, + ancestor: OsmElementLookup, + excluded: ReadonlySet = new Set() +) { + for (;;) { + let restored = false; + + for (const element of [...output]) { + for (const [type, id] of referencesOf(element)) { + if (output.has(type, id) || excluded.has(referenceKey(type, id))) { + continue; + } + + const original = ancestor.get(type, id); + + if (original) { + output.insert(copyOsmElement(original)); + restored = true; + } + } + } + + if (!restored) { + return; + } + } +} + +function referencesOf(element: OsmElement): [OsmElementType, number][] { + if (element.type === 'way') { + return element.nodes.map(ref => ['node', ref]); + } + + if (element.type === 'relation') { + return element.members.map(member => [member.type, member.ref]); + } + + return []; +} + +/** + * Reports ways and relations left referencing elements the merge does not + * contain. + * + * A resolution can reintroduce an element whose references the other side + * deleted. The osmChange builder refuses those, but only once the destination + * workspace already exists, so the failure arrives late and reads as an + * internal error. Checking first lets the caller name what to change. + */ +export function findDanglingReferences(elements: OsmElementLookup): string[] { + const problems: string[] = []; + + for (const element of elements) { + for (const [type, id] of referencesOf(element)) { + if (!elements.has(type, id)) { + problems.push(`${element.type} ${element.id} references ${type} ${id}`); + } + } + } + + return problems; +} + +/** Copies the merged set and puts back anything it still references. */ +export function repairReferences(result: MergeResult): OsmElementLookup { + const repaired = new OsmElementLookup(); + + for (const element of result.elements) { + repaired.insert(copyOsmElement(element)); + } + + restoreMissingReferences(repaired, result.ancestor); + + return repaired; +} diff --git a/services/merge/three-way.ts b/services/merge/three-way.ts new file mode 100644 index 00000000..8692e6f6 --- /dev/null +++ b/services/merge/three-way.ts @@ -0,0 +1,275 @@ +import { referenceKey, restoreMissingReferences } from '~/services/merge/references'; +import { copyOsmElement, OsmElementLookup } from '~/util/osm'; +import { + checkConflict, + checkGeometryConflict, + mergeOsmElements, + mergeTagsThreeWay +} from '~/util/osm-merge'; + +import type { + ConflictResolution, + MergeConflict, + MergeConflictKind +} from '~/services/merge/types'; +import type { OsmElement } from '~/types/osm'; + +/** + * Each workspace is its own database with its own ID sequence, so two + * workspaces routinely assign the same ID to unrelated elements created after + * the import. Those are not a conflict, they are two different elements, and + * keeping both means giving one of them a new ID and repointing everything in + * that workspace that referenced it. + */ +function remapCollidingIds( + elementsB: OsmElement[], + lookupC: OsmElementLookup, + createdInA: OsmElementLookup +): OsmElement[] { + const remapped = new Map(); + let nextSyntheticId = -1; + + for (const element of elementsB) { + if (lookupC.has(element.type, element.id)) { + continue; // Descended from the import, so the ID is shared by design. + } + + if (createdInA.has(element.type, element.id)) { + remapped.set(referenceKey(element.type, element.id), nextSyntheticId--); + } + } + + if (remapped.size === 0) { + return elementsB; + } + + for (const element of elementsB) { + if (element.type === 'way') { + element.nodes = element.nodes.map( + ref => remapped.get(referenceKey('node', ref)) ?? ref + ); + } else if (element.type === 'relation') { + element.members = element.members.map(member => ({ + ...member, + ref: remapped.get(referenceKey(member.type, member.ref)) ?? member.ref + })); + } + + const newId = remapped.get(referenceKey(element.type, element.id)); + + if (newId !== undefined) { + element.id = newId; + } + } + + return elementsB; +} + +function describeConflict( + number: number, + kind: MergeConflictKind, + a: OsmElement, + b: OsmElement, + base?: OsmElement +): MergeConflict { + const { tags, conflicts } = mergeTagsThreeWay(a.tags, b.tags, base); + + return { + number, + kind, + type: a.type, + // Both sides carry the same ID: the synthesised side is a copy of the + // ancestor, which was fetched by the other side's ID. + elementId: a.id, + a, + b, + base, + geometryConflict: checkGeometryConflict(a, b, base), + tagConflicts: conflicts, + mergedTags: tags + }; +} + +/** + * Merges two workspaces that share a common ancestor. + * + * Elements the merge settles on its own go into `merged`. Elements it cannot + * are reported in `conflicts` and deliberately left out of `merged`: both sides + * carry the same ID, so storing them together would silently discard one. + * `applyResolutions` puts the user's choice back in. + */ +export function threeWayMerge( + elementsA: OsmElement[], + elementsB: OsmElement[], + lookupC: OsmElementLookup +): { + merged: OsmElementLookup; + conflicts: MergeConflict[]; +} { + const merged = new OsmElementLookup(); + const remainingA = new OsmElementLookup(); + const conflicts: MergeConflict[] = []; + + // Work on copies throughout. The inputs belong to the caller, and the + // ancestor lookup is read repeatedly, so mutating either corrupts later + // comparisons: + // + const copiesA = elementsA.map(copyOsmElement); + const createdInA = new OsmElementLookup(); + + for (const elementA of copiesA) { + // Absent from the import means someone created it afterwards. There is no + // common ancestor to merge against, so it carries straight through: + // + if (!lookupC.has(elementA.type, elementA.id)) { + createdInA.insert(elementA); + merged.insert(elementA); + continue; + } + + remainingA.insert(elementA); + } + + const copiesB = remapCollidingIds( + elementsB.map(copyOsmElement), + lookupC, + createdInA + ); + + let conflictNumber = 1; + + for (const elementB of copiesB) { + const base = lookupC.get(elementB.type, elementB.id); + + if (!base) { + merged.insert(elementB); + continue; + } + + const elementA = remainingA.get(elementB.type, elementB.id); + + // Untouched in B, so whatever A did to it stands, including deleting it: + // + if (elementB.version === 1) { + if (elementA) { + merged.insert(elementA); + remainingA.drop(elementA); + } + continue; + } + + if (elementA) { + remainingA.drop(elementA); + + if (elementA.version === 1) { + merged.insert(elementB); // Untouched in A. + continue; + } + + if (!checkConflict(elementA, elementB, base)) { + merged.insert(mergeOsmElements(elementA, elementB, base)); + continue; + } + + conflicts.push( + describeConflict(conflictNumber++, 'both-modified', elementA, elementB, base) + ); + } else { + conflicts.push( + describeConflict( + conflictNumber++, + 'deleted-in-a', + copyOsmElement(base), + elementB, + base + ) + ); + } + } + + // Whatever is left existed in the import and is gone from B, so B deleted it. + // An element A also left alone is simply dropped; one A modified is a + // conflict between that edit and the deletion: + // + for (const elementA of remainingA) { + if (elementA.version === 1) { + continue; + } + + const base = lookupC.get(elementA.type, elementA.id); + + if (base) { + conflicts.push( + describeConflict( + conflictNumber++, + 'deleted-in-b', + elementA, + copyOsmElement(base), + base + ) + ); + } + } + + return { merged, conflicts }; +} + +/** + * Folds the user's conflict decisions back into the merged element set. + * + * Tags come from the resolution, which starts from `mergedTags`, so tags + * neither side disputed are preserved even when the user only picks a + * geometry. Given an `ancestor`, elements a kept way or relation still + * references are restored from it, except those the user resolved as deleted, + * which stay out so the contradiction is reported rather than reversed. + */ +export function applyResolutions( + elements: OsmElementLookup, + conflicts: MergeConflict[], + resolutions: ConflictResolution[], + ancestor?: OsmElementLookup +): OsmElementLookup { + const output = new OsmElementLookup(); + const deleted = new Set(); + + for (const element of elements) { + output.insert(copyOsmElement(element)); + } + + conflicts.forEach((conflict, index) => { + const resolution = resolutions[index]; + + if (!resolution?.resolved || resolution.chosenSide === null) { + throw new Error(`Conflict ${conflict.number} has not been resolved.`); + } + + if (resolution.chosenSide === 'delete') { + output.remove(conflict.type, conflict.elementId); + deleted.add(referenceKey(conflict.type, conflict.elementId)); + return; + } + + const chosenElement = resolution.chosenSide === 'b' ? conflict.b : conflict.a; + + // Only a geometry conflict makes the two sides' shapes a choice. Without + // one, the merge can settle the geometry itself, and taking the chosen + // side wholesale would throw away an edit the other side made alone: + // + const element = copyOsmElement( + conflict.geometryConflict + ? chosenElement + : mergeOsmElements(conflict.a, conflict.b, conflict.base) + ); + + element.id = conflict.elementId; + element.tags = { ...resolution.tags }; + + output.insert(element); + }); + + if (ancestor) { + restoreMissingReferences(output, ancestor, deleted); + } + + return output; +} diff --git a/services/merge/types.ts b/services/merge/types.ts new file mode 100644 index 00000000..96cbb820 --- /dev/null +++ b/services/merge/types.ts @@ -0,0 +1,66 @@ +import type { OsmElement, OsmElementType, OsmTags } from '~/types/osm'; +import type { WorkspaceType } from '~/types/workspaces'; +import type { OsmElementLookup } from '~/util/osm'; + +/** + * How the two workspaces diverged. + * + * `deleted-in-a` and `deleted-in-b` mean one side removed the element while the + * other edited it. The side that deleted is represented by the common ancestor, + * so the UI must offer "drop the element" as a third outcome rather than + * presenting it as an ordinary A-versus-B choice. + */ +export type MergeConflictKind = 'both-modified' | 'deleted-in-a' | 'deleted-in-b'; + +export interface MergeConflict { + number: number; + kind: MergeConflictKind; + type: OsmElementType; + elementId: number; + a: OsmElement; + b: OsmElement; + base?: OsmElement; + geometryConflict: boolean; + tagConflicts: string[]; + + /** + * Tags settled by the three-way merge, with conflicting keys left at A's + * value. Seeds the resolution so that tags neither side disputed survive. + */ + mergedTags: OsmTags; +} + +export type ConflictChoice = 'a' | 'b' | 'delete'; + +export interface ConflictResolution { + resolved: boolean; + chosenSide: ConflictChoice | null; + tags: OsmTags; +} + +export interface MergeResult { + type: WorkspaceType; + title: string; + workspaceIdA: number; + workspaceIdB: number; + tdeiDatasetId?: string; + tdeiProjectGroupId: string; + + /** + * Dataset details copied from workspace A. Both sources are built from the + * same TDEI dataset, so the merged workspace should present the same way on + * the dashboard, the dashboard map and the TDEI export page. + */ + description?: string; + tdeiServiceId?: string; + tdeiMetadata?: string; + elements: OsmElementLookup; + conflicts: MergeConflict[]; + + /** + * The import both workspaces descend from. Kept so that resolving a conflict + * can restore an element the other side's deletion removed but the resolved + * element still references. + */ + ancestor: OsmElementLookup; +} diff --git a/services/merge/upload.ts b/services/merge/upload.ts new file mode 100644 index 00000000..66e07795 --- /dev/null +++ b/services/merge/upload.ts @@ -0,0 +1,75 @@ +import { OsmChangeBuilder } from '~/util/osmChange'; + +import type { OsmElement, OsmRelation, OsmWay } from '~/types/osm'; + +export function buildOsc(changesetId: number, elements: Iterable): string { + const oscBuilder = new OsmChangeBuilder(changesetId); + + // The target workspace is empty, so every element is created. They have to + // be emitted in dependency order for the builder to resolve `nd` and + // `member` references to the placeholder IDs it assigns: + // + const ways: OsmWay[] = []; + const relations: OsmRelation[] = []; + + for (const element of elements) { + if (element.type === 'node') { + oscBuilder.create(element); + } else if (element.type === 'way') { + ways.push(element); + } else { + relations.push(element); + } + } + + for (const way of ways) { + oscBuilder.create(way); + } + + for (const relation of sortRelationsByDependency(relations)) { + oscBuilder.create(relation); + } + + return oscBuilder.serializeXml(); +} + +/** + * Orders relations so that one referencing another is emitted after it. The API + * rejects a forward reference to a placeholder it has not seen yet. + */ +function sortRelationsByDependency(relations: OsmRelation[]): OsmRelation[] { + const byId = new Map(relations.map(relation => [relation.id, relation])); + const ordered: OsmRelation[] = []; + const visited = new Set(); + + function visit(relation: OsmRelation, seen: Set) { + // A membership cycle cannot be ordered. Returning leaves one member + // unresolved, which the builder reports by name, rather than looping here: + // + if (visited.has(relation.id) || seen.has(relation.id)) { + return; + } + + seen.add(relation.id); + + for (const member of relation.members) { + if (member.type === 'relation') { + const child = byId.get(member.ref); + + if (child) { + visit(child, seen); + } + } + } + + seen.delete(relation.id); + visited.add(relation.id); + ordered.push(relation); + } + + for (const relation of relations) { + visit(relation, new Set()); + } + + return ordered; +} diff --git a/services/merge/validation.ts b/services/merge/validation.ts new file mode 100644 index 00000000..81488824 --- /dev/null +++ b/services/merge/validation.ts @@ -0,0 +1,140 @@ +import { geometryEqual } from '~/util/osm'; + +import type { OsmElementLookup } from '~/util/osm'; +import type { OsmTags } from '~/types/osm'; + +/** + * The OSM API caps a tag key, a tag value and a relation member role at 255 + * characters each, and rejects the whole upload with a 400 when any is longer. + * Counted in code points, because the server counts characters: a surrogate + * pair is one character to it and two units to `String.length`. + */ +export const MAX_TAG_LENGTH = 255; + +/** + * Characters XML 1.0 cannot represent, even escaped. The serializer writes them + * through verbatim, producing a document the server cannot parse. + * + * Lone surrogates are here for a different reason: they serialize, but fetch's + * UTF-8 encoder rewrites one to U+FFFD, so the tag would upload quietly + * corrupted instead of failing. `\p{Surrogate}` under the `u` flag matches only + * unpaired ones, so an ordinary astral character such as an emoji, which is a + * valid surrogate pair, is left alone. + */ +// eslint-disable-next-line no-control-regex +const INVALID_XML = /[\u0000-\u0008\u000B\u000C\u000E-\u001F\uFFFE\uFFFF]|\p{Surrogate}/u; + +function describe(element: { type: string; id: number }): string { + return `${element.type} ${element.id}`; +} + +/** + * Reports tags and relation member roles the OSM API will not accept, so a + * merge fails while it is still only a set of resolutions rather than after the + * destination workspace has been created and has to be rolled back. The + * osmChange builder cannot catch these: an over-long or control-character value + * serializes without complaint, and the server's rejection names neither the + * element nor the tag. + */ +export function findUnusableTags(elements: OsmElementLookup): string[] { + const problems: string[] = []; + + for (const element of elements) { + const label = describe(element); + + for (const [key, value] of Object.entries(element.tags ?? { })) { + if (value === undefined) { + continue; + } + + if ([...key].length > MAX_TAG_LENGTH) { + problems.push(`${label}: the tag name "${key.slice(0, 40)}..." is too long`); + } + + if ([...value].length > MAX_TAG_LENGTH) { + problems.push( + `${label}: the value of "${key}" is longer than ${MAX_TAG_LENGTH} characters` + ); + } + + if (INVALID_XML.test(key) || INVALID_XML.test(value)) { + problems.push(`${label}: the tag "${key}" contains a character XML cannot carry`); + } + } + + // A member role is arbitrary data that reaches the serializer the same way + // a tag value does, and the API caps it the same way: + // + if (element.type === 'relation') { + for (const member of element.members) { + if ([...member.role].length > MAX_TAG_LENGTH) { + problems.push( + `${label}: the role of ${member.type} ${member.ref} is longer ` + + `than ${MAX_TAG_LENGTH} characters` + ); + } + + if (INVALID_XML.test(member.role)) { + problems.push( + `${label}: the role of ${member.type} ${member.ref} contains a ` + + 'character XML cannot carry' + ); + } + } + } + } + + return problems; +} + +/** + * Checks that both workspaces were imported from the same dataset the same + * way, and describes the first disagreement found. + * + * Each workspace is a separate database with its own id sequences, so element + * 42 in one is unrelated to element 42 in the other unless the two imports + * assigned identical ids. The merge reads its common ancestor from workspace A + * alone and matches workspace B against it by id, so if the two imports + * disagree, every B element still reads as untouched and A's copy silently + * wins, losing B's work with no error at all. + * + * Geometry and tags are compared, but not metadata: two runs of the same + * import assign their own changeset ids, user ids and timestamps, while a + * given element is in the same place and carries the same tags in both unless + * the ids have shifted or the importer itself changed. + */ +export function findImportMismatch( + ancestorA: OsmElementLookup, + ancestorB: OsmElementLookup +): string | undefined { + if (ancestorA.size !== ancestorB.size) { + return `their imports created ${ancestorA.size} and ${ancestorB.size} elements`; + } + + for (const element of ancestorA) { + const counterpart = ancestorB.get(element.type, element.id); + + if (!counterpart) { + return `${describe(element)} exists in one import and not the other`; + } + + if (!geometryEqual(element, counterpart)) { + return `${describe(element)} is in a different place in each import`; + } + + if (!tagsEqual(element.tags, counterpart.tags)) { + return `${describe(element)} carries different tags in each import`; + } + } + + return undefined; +} + +function tagsEqual(a: OsmTags | undefined, b: OsmTags | undefined): boolean { + const left = a ?? { }; + const right = b ?? { }; + const keys = Object.keys(left); + + return keys.length === Object.keys(right).length + && keys.every(key => Object.hasOwn(right, key) && right[key] === left[key]); +} diff --git a/services/merge/workspace.ts b/services/merge/workspace.ts new file mode 100644 index 00000000..b7ee4e3a --- /dev/null +++ b/services/merge/workspace.ts @@ -0,0 +1,296 @@ +import { findDanglingReferences, repairReferences } from '~/services/merge/references'; +import { threeWayMerge } from '~/services/merge/three-way'; +import { buildOsc } from '~/services/merge/upload'; +import { findImportMismatch, findUnusableTags } from '~/services/merge/validation'; +import { compareWorkspaceCreatedAtDesc } from '~/services/workspaces'; +import { OsmElementLookup } from '~/util/osm'; + +import type { MergeResult } from '~/services/merge/types'; +import type { OsmApiClient } from '~/services/osm'; +import type { WorkspacesClient } from '~/services/workspaces'; +import type { Workspace, WorkspaceId } from '~/types/workspaces'; + +export class ThreeWayWorkspaceMerger { + readonly #workspacesClient: WorkspacesClient; + readonly #osmClient: OsmApiClient; + readonly #workspace: Workspace; + + constructor( + workspacesClient: WorkspacesClient, + osmClient: OsmApiClient, + workspace: Workspace + ) { + this.#workspacesClient = workspacesClient; + this.#osmClient = osmClient; + this.#workspace = workspace; + } + + async getMergeTargets(): Promise { + return (await this.#workspacesClient.getMyWorkspaces()) + .filter(w => this.#isEligibleWorkspace(w)) + .sort(compareWorkspaceCreatedAtDesc); + } + + async merge( + targetId: WorkspaceId, + projectGroupId: string, + title: string + ): Promise { + // The target is filtered out of `getMergeTargets`, but the workspace the + // user started from is never checked, and it fails later as a bogus import + // mismatch rather than saying its import has not finished: + // + if (!importHasSettled(this.#workspace)) { + throw new Error( + `This workspace's import is ${this.#workspace.importStatus}, so it does not hold ` + + 'all of its data yet. Wait for the import to finish, then merge.' + ); + } + + const [elementsA, elementsB, lookupC, targetLookupC] = await Promise.all([ + this.#osmClient.getWorkspaceData(this.#workspace.id), + this.#osmClient.getWorkspaceData(targetId), + this.#getInitialLookup(this.#workspace.id), + this.#getInitialLookup(targetId) + ]); + + // Matching TDEI dataset and project group is not enough: the two imports + // ran separately, into separate databases with their own id sequences. + // + const mismatch = findImportMismatch(lookupC, targetLookupC); + + if (mismatch) { + throw new Error( + 'These workspaces were not imported the same way, so their element IDs cannot be ' + + `matched up: ${mismatch}. Merging them would discard the other workspace's work.` + ); + } + + const { merged, conflicts } = threeWayMerge(elementsA, elementsB, lookupC); + + return { + type: this.#workspace.type, + title, + workspaceIdA: this.#workspace.id, + workspaceIdB: targetId, + tdeiDatasetId: this.#workspace.tdeiRecordId, + tdeiProjectGroupId: projectGroupId, + description: this.#workspace.description, + tdeiServiceId: this.#workspace.tdeiServiceId, + tdeiMetadata: this.#workspace.tdeiMetadata, + elements: merged, + conflicts, + ancestor: lookupC + }; + } + + /** + * Reads the import that both workspaces descend from: the workspace's first + * changeset. + * + * This takes the import to be a single changeset, which is what the importer + * writes: the deployment raises the changeset element cap + * (`WS_OSM_MAX_CHANGESET_ELEMENTS`) far enough that a dataset never has to be + * split. An import split across several changesets would leave everything + * after the first looking newly created on both sides, and the merge would + * keep two copies of it. Nothing here detects that. + */ + async #getInitialLookup(workspaceId: WorkspaceId): Promise { + // The API returns the newest changesets and caps the page, so asking for + // the oldest is what makes this the import rather than an ordinary edit on + // a workspace with a long history. The reduce below only normalises order + // within the page that comes back: if a server ignored `order` entirely, + // the earliest of its newest page would not be the import either. + // + const changesets = await this.#osmClient.listChangesets(workspaceId, { + order: 'oldest' + }); + + if (changesets.length === 0) { + throw new Error( + `Workspace ${workspaceId} has no changesets, so there is no common ancestor ` + + 'to merge against.' + ); + } + + const initial = changesets.reduce((earliest, changeset) => + changeset.created_at.getTime() < earliest.created_at.getTime() + ? changeset + : earliest + ); + + const osc = await this.#osmClient.getOsmChange(workspaceId, initial.id); + + // An empty ancestor makes every element look newly created, so no conflict + // can be detected and the merge silently copies both workspaces whole. + // Refusing is better than producing that: + // + const lookup = new OsmElementLookup(osc.create); + + if (lookup.size === 0) { + throw new Error( + `Changeset ${initial.id} creates no elements, so it cannot be the import ` + + `workspace ${workspaceId} descends from. There is no common ancestor to ` + + 'merge against.' + ); + } + + return lookup; + } + + #isEligibleWorkspace(workspace: Workspace): boolean { + // Without a dataset ID there is no shared import, and two workspaces that + // both lack one are unrelated rather than matching: + // + if (!this.#workspace.tdeiRecordId) { + return false; + } + + return workspace.id !== this.#workspace.id + && workspace.type === this.#workspace.type + && workspace.tdeiRecordId === this.#workspace.tdeiRecordId + && workspace.tdeiProjectGroupId === this.#workspace.tdeiProjectGroupId + && importHasSettled(workspace); + } +} + +export class WorkspaceMergerFactory { + readonly #workspacesClient: WorkspacesClient; + readonly #osmClient: OsmApiClient; + + constructor(workspacesClient: WorkspacesClient, osmClient: OsmApiClient) { + this.#workspacesClient = workspacesClient; + this.#osmClient = osmClient; + } + + getThreeWayMerger(workspace: Workspace): ThreeWayWorkspaceMerger { + return new ThreeWayWorkspaceMerger( + this.#workspacesClient, + this.#osmClient, + workspace + ); + } + + async commit(result: MergeResult, elements?: OsmElementLookup): Promise { + // A resolved set arrives already repaired by `applyResolutions`, which + // knows which elements the user chose to delete and leaves those out. An + // unresolved one has had no repair at all: a merge with no conflicts never + // visits the conflicts page, and one side deleting an element the other + // side's untouched way still uses is an uncontested deletion, not a + // conflict. Without this the commit would fail telling the user to change + // a resolution that does not exist. + // + const merged = elements ?? repairReferences(result); + + // Checked before anything is created. The osmChange builder rejects a + // dangling reference too, but only after the destination workspace exists, + // which turns a fixable resolution into a failed commit and a rollback: + // + const dangling = findDanglingReferences(merged); + + if (dangling.length > 0) { + throw new Error( + `The merge is incomplete: ${summarize(dangling)}` + + (elements + ? '. Change the resolution that removes those elements, or keep the side that uses them.' + : '. The two workspaces cannot be merged without the missing elements.') + ); + } + + // Same reasoning as the dangling check: the server rejects these, but only + // once the destination workspace exists, and its error names neither the + // element nor the tag: + // + const unusable = findUnusableTags(merged); + + if (unusable.length > 0) { + throw new Error( + `The merge cannot be uploaded: ${summarize(unusable)}` + + '. Shorten or remove those tag values, then commit again.' + ); + } + + // createBlankWorkspace also provisions the workspace in the OSM API. A + // workspace created without that step has no database to write to. + // + // `tdeiRecordId` is deliberately absent. The create endpoint reads it, with + // a project group id, as "import this TDEI dataset": it marks the workspace + // as importing and queues a job that would write the whole source dataset + // into the one this merge is about to upload to. The dataset's other details + // carry no such meaning and are safe to send. + // + const mergedWorkspaceId = await this.#workspacesClient.createBlankWorkspace({ + title: result.title, + type: result.type, + tdeiProjectGroupId: result.tdeiProjectGroupId, + description: result.description, + tdeiServiceId: result.tdeiServiceId, + tdeiMetadata: result.tdeiMetadata + }); + + let openChangesetId: number; + + try { + openChangesetId = await this.#osmClient.createChangeset(mergedWorkspaceId, { + comment: `Merge of workspaces ${result.workspaceIdA} and ${result.workspaceIdB}` + }); + + const osc = buildOsc(openChangesetId, merged); + + await this.#osmClient.uploadChangeset(mergedWorkspaceId, openChangesetId, osc); + } catch (error) { + // Leaving the empty workspace behind would put an unusable entry on the + // dashboard, and every retry would add another: + // + await this.#deleteQuietly(mergedWorkspaceId); + throw error; + } + + // Closing happens outside the rollback on purpose. The upload has already + // succeeded here, so the merged data is in the new workspace: deleting it + // over a failed close would discard the whole merge. A changeset left + // open closes itself, after an idle timeout on the server. + // + try { + await this.#osmClient.closeChangeset(mergedWorkspaceId, openChangesetId); + } catch (error) { + console.warn(`Merged changeset ${openChangesetId} could not be closed.`, error); + } + + return mergedWorkspaceId; + } + + async #deleteQuietly(workspaceId: WorkspaceId): Promise { + try { + await this.#workspacesClient.deleteWorkspace(workspaceId); + } catch (error) { + // The commit failure is the one worth reporting, so this one is logged + // rather than thrown: it would otherwise replace the original error on + // its way to the user, and an orphaned workspace would be invisible. + // + console.warn(`Rollback of merged workspace ${workspaceId} failed.`, error); + } + } +} + +/** + * Whether a workspace's data is all there, so it can be merged. + * + * Imports run as a background job, so a workspace can be listed while it is + * still filling. Merging one compares a partial dataset and reports it as an + * import mismatch, which blames the imports rather than saying one has not + * finished. `in-progress` is excluded as unfinished and `failed` as + * incomplete. Everything else passes, including an absent status: the backend + * types the column as a plain nullable string with no constraint, and a + * workspace that never had a TDEI import reports `NA` and is already excluded + * by the dataset ID check. + */ +function importHasSettled(workspace: Workspace): boolean { + return workspace.importStatus !== 'in-progress' && workspace.importStatus !== 'failed'; +} + +/** Caps a problem list at three named items so the message stays readable. */ +function summarize(problems: string[]): string { + return problems.slice(0, 3).join('; ') + + (problems.length > 3 ? `, and ${problems.length - 3} more` : ''); +} diff --git a/services/osm.ts b/services/osm.ts index 888f170c..cb9488b2 100644 --- a/services/osm.ts +++ b/services/osm.ts @@ -20,6 +20,7 @@ import type { OsmElement, OsmNode, OsmNote, + OsmTags, OsmWay, } from '~/types/osm'; import type { WorkspaceId } from '~/types/workspaces'; @@ -343,8 +344,22 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { return (await response.json()).elements; } - async listChangesets(workspaceId: WorkspaceId): Promise { - const response = await this._get(`changesets.json`, { + async listChangesets( + workspaceId: WorkspaceId, + params: { order?: 'oldest' | 'newest'; limit?: number } = { } + ): Promise { + const query = new URLSearchParams(); + + if (params.order) { + query.set('order', params.order); + } + + if (params.limit !== undefined) { + query.set('limit', String(params.limit)); + } + + const search = query.size > 0 ? `?${query}` : ''; + const response = await this._get(`changesets.json${search}`, { headers: { 'X-Workspace': String(workspaceId) }, }); @@ -414,12 +429,20 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { return osmChange; } - async createChangeset(workspaceId: WorkspaceId): Promise { + async createChangeset(workspaceId: WorkspaceId, tags: OsmTags = { }): Promise { const doc = xml.parse(''); const changesetNode = doc.firstChild!.firstChild!; - changesetNode.appendChild(xml.makeNode(doc, 'tag', { k: 'workspace', v: workspaceId })); - changesetNode.appendChild(xml.makeNode(doc, 'tag', { k: 'comment', v: 'Import workspace' })); - changesetNode.appendChild(xml.makeNode(doc, 'tag', { k: 'created_by', v: 'TDEI Workspaces' })); + const changesetTags: OsmTags = { + workspace: String(workspaceId), + created_by: 'TDEI Workspaces', + ...tags + }; + + // A for...in loop here could pollute this by walking the prototype chain: + // + for (const [k, v] of Object.entries(changesetTags)) { + changesetNode.appendChild(xml.makeNode(doc, 'tag', { k, v })); + } const body = xml.serialize(doc); const response = await this._put('changeset/create', body, { @@ -434,12 +457,20 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { changesetId: number, changesetXml: string ) { - await this._post(`changeset/${changesetId}/upload`, changesetXml, { + const response = await this._post(`changeset/${changesetId}/upload`, changesetXml, { headers: { 'Content-Type': 'application/xml', 'X-Workspace': String(workspaceId), }, }); + + return await response.text(); + } + + async closeChangeset(workspaceId: WorkspaceId, changesetId: number): Promise { + await this._put(`changeset/${changesetId}/close`, undefined, { + headers: { 'X-Workspace': String(workspaceId) } + }); } async getChangesetComments( @@ -503,8 +534,17 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { return notesGeoJsonToEntities(await response.json()); } - async getWorkspaceData(workspaceId: WorkspaceId): Promise { + async getWorkspaceData(workspaceId: WorkspaceId): Promise { const bboxParam = await this.getExportBbox(workspaceId); + + if (bboxParam === undefined) { + // The API reports no bounding box for a workspace that has never held a + // node. The export paths archive an empty workspace legitimately, so this + // reads as no elements rather than throwing: + // + return []; + } + const response = await this._get(`map.json?bbox=${bboxParam}`, { headers: { 'Accept': 'application/json', @@ -512,11 +552,31 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { } }); - return (await response.json()).elements; + const elements: OsmElement[] = (await response.json()).elements ?? []; + + // The API sends timestamps as strings. Every other method on this client + // normalizes them here rather than leaving it to callers: + // + for (const element of elements) { + element.timestamp = new Date(element.timestamp); + } + + return elements; } async exportWorkspaceXml(workspaceId: WorkspaceId): Promise { const bboxParam = await this.getExportBbox(workspaceId); + + if (bboxParam === undefined) { + // Same case as `getWorkspaceData`: no bounding box means the workspace + // has never held a node, and an empty document is its export: + // + return new Blob( + ['\n\n'], + { type: 'application/xml' } + ); + } + const response = await this._get(`map?bbox=${bboxParam}`, { headers: { 'Accept': 'application/xml', @@ -533,6 +593,12 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { body?: HttpBody, config?: FetchConfig, ): Promise { + // `sendProtectedRequest` may refresh the session before or between + // attempts, and a failed refresh throws the TDEI API's response. Only the + // error this client's own request threw is the OSM server's answer: + // + let osmFailure: unknown; + try { const requestOptions: FetchConfig = { credentials: 'include', @@ -545,11 +611,14 @@ export class OsmApiClient extends BaseHttpClient implements ICancelableClient { method, body, withBearerToken(requestOptions, accessToken), - ) + ).catch((error: unknown) => { + osmFailure = error; + throw error; + }) ); } catch (e: unknown) { - if (e instanceof BaseHttpClientError) { + if (e === osmFailure && e instanceof BaseHttpClientError) { throw new OsmApiClientError(e.response); } diff --git a/services/workspaces.ts b/services/workspaces.ts index c2032c2f..32408112 100644 --- a/services/workspaces.ts +++ b/services/workspaces.ts @@ -5,6 +5,7 @@ import { type FetchConfig, type HttpBody, } from '~/services/http'; +import { OsmApiClientError } from '~/services/osm'; import { buildPathwaysCsvArchive } from '~/services/pathways'; import { compareStringAsc } from '~/util/compare'; @@ -145,7 +146,13 @@ export class WorkspacesClient extends BaseHttpClient implements ICancelableClien } // The v1 API returns coordinates already in decimal degrees. - return await response.json(); + const bbox: BoundingBox | null = await response.json(); + + if (!bbox || !isCompleteBbox(bbox)) { + return undefined; + } + + return bbox; } finally { this._baseUrl = originalBaseUrl; @@ -163,15 +170,43 @@ export class WorkspacesClient extends BaseHttpClient implements ICancelableClien workspace.createdByName = this.#tdeiClient.auth.displayName; const workspaceResponse = await this._post('workspaces', workspace); - const workspaceId = (await workspaceResponse.json()).workspaceId; + const body: unknown = await workspaceResponse.json(); + const workspaceId = (body as { workspaceId?: unknown } | null)?.workspaceId; + + // Checked because a failed commit rolls back by deleting this ID, and a + // rollback must never be aimed at something the response did not name: + // + if (typeof workspaceId !== 'number' || !Number.isInteger(workspaceId)) { + throw new Error('Workspace creation response did not include a valid integer workspace ID.'); + } return workspaceId; } - /// Create a new workspace (Blank) and provision it in the OSM API. + /** Creates the workspace row and provisions its OSM database. */ async createBlankWorkspace(workspace: WorkspaceCreation): Promise { const workspaceId = await this.createWorkspace(workspace); - await this.#osmClient.createWorkspace(workspaceId); + + try { + await this.#osmClient.createWorkspace(workspaceId); + } + catch (error) { + // The row exists now but has no OSM database behind it, so leaving it + // would put an unusable workspace on the dashboard and every retry would + // add another. The provisioning failure is the one to report, but a + // failed rollback is logged rather than lost, since it leaves that + // workspace behind: + // + try { + await this.deleteWorkspace(workspaceId); + } + catch (rollbackError) { + console.warn(`Rollback of workspace ${workspaceId} failed.`, rollbackError); + } + + throw error; + } + return workspaceId; } @@ -241,11 +276,31 @@ export class WorkspacesClient extends BaseHttpClient implements ICancelableClien ); } + /** + * Deletes the workspace's OSM database, then its row. + * + * The row goes second because deleting it removes every member's role, and + * where OSM requests pass through the API, it checks for the lead role + * before it drops the database, so a drop sent after the row delete would be + * refused. + * + * A refused drop (401 or 403) stops here and leaves the workspace intact. Any + * other failed response is taken to mean the database is already gone: the + * OSM API answers with an error, not a no-op, for a workspace that never had + * one, such as an import that failed before creating it, and stopping would + * leave that row impossible to delete. + */ async deleteWorkspace(id: WorkspaceId): Promise { - await Promise.all([ - this._delete(`workspaces/${id}`), - this.#osmClient.deleteWorkspace(id), - ]); + try { + await this.#osmClient.deleteWorkspace(id); + } + catch (error) { + if (!(error instanceof OsmApiClientError) || [401, 403].includes(error.response.status)) { + throw error; + } + } + + await this._delete(`workspaces/${id}`); } async getLongFormQuestSettings(id: WorkspaceId): Promise { @@ -370,3 +425,8 @@ export class WorkspacesClient extends BaseHttpClient implements ICancelableClien } } } + +function isCompleteBbox(bbox: BoundingBox): boolean { + return [bbox.min_lon, bbox.min_lat, bbox.max_lon, bbox.max_lat] + .every(Number.isFinite); +} diff --git a/test/e2e/merge-conflicts.spec.ts b/test/e2e/merge-conflicts.spec.ts new file mode 100644 index 00000000..4c6d704d --- /dev/null +++ b/test/e2e/merge-conflicts.spec.ts @@ -0,0 +1,382 @@ +import { test, expect, seedAuthenticatedSession } from './fixtures'; +import { recordContract } from './contract'; +import { mergeWorkspaceA, mergeWorkspaceB, projectGroups } from '../mocks/fixtures'; + +import type { Page } from '@playwright/test'; + +// Generated from the @test outline in pages/workspace/[id]/merge/conflicts.vue. +// +// This page has no route of its own into a populated state: the MergeResult is +// held in shared state by `useMergeResult`, set by the merge page. So every test +// that needs conflicts drives through /workspace/1/merge and clicks Merge, which +// means the OSM stubs below have to produce a real conflict. +// +// The merge reads, per workspace: GET workspaces/{id}/bbox then +// GET osm/.../map.json?bbox=..., plus GET osm/.../changesets.json and the +// ancestor's osmChange for workspace A. The ancestor must be non-empty or the +// merge refuses (an empty ancestor would make every element look new and would +// silently duplicate both workspaces). +// +// CONVENTION: tests are written to the @test comments (intended behavior). Where +// the code diverges, the test is left to FAIL (red) and the divergence is noted. + +const MERGE_URL = '/workspace/1/merge'; +const CONFLICTS_URL = '/workspace/1/merge/conflicts'; + +const BBOX = { min_lon: -122.4, min_lat: 47.6, max_lon: -122.3, max_lat: 47.7 }; + +// Version 2 marks an element edited since the import, which is version 1. The +// three-way merge treats a version-1 element as untouched on that side. +function editedNode(id: number, lat: number, tags: Record = {}) { + return { + id, + type: 'node', + lat, + lon: -122.35, + version: 2, + changeset: 1, + timestamp: '2026-01-01T00:00:00Z', + user: 'tester', + uid: 7, + tags + }; +} + +// The import both workspaces descend from: workspace A's first changeset. +const ANCESTOR_OSC = ` + + + +`; + +async function stubConflictingMerge(page: Page) { + await page.route('**/api.test/workspaces/1', route => + route.fulfill({ json: mergeWorkspaceA }) + ); + + await page.route('**/api.test/workspaces/mine', route => + route.fulfill({ json: [mergeWorkspaceA, mergeWorkspaceB] }) + ); + await page.route('**/tdei-user/project-group-roles/**', route => + route.fulfill({ json: projectGroups }) + ); + await page.route('**/api.test/workspaces/check', route => + route.fulfill({ json: { available: true } }) + ); + await page.route('**/workspaces/*/bbox', route => route.fulfill({ json: BBOX })); + + // Both sides moved node 1 away from the ancestor's 47.60, and to different + // places: one geometry conflict. Only A added `surface`, and neither side + // touched `highway`, so there is no tag conflict and both tags must survive + // either choice. + await page.route('**/osm/**/map.json**', (route) => { + const inB = route.request().headers()['x-workspace'] === '2'; + const node = inB + ? editedNode(1, 47.70, { highway: 'footway' }) + : editedNode(1, 47.65, { highway: 'footway', surface: 'asphalt' }); + + return route.fulfill({ json: { elements: [node] } }); + }); + + await page.route('**/osm/**/changesets.json**', route => + route.fulfill({ + json: { + changesets: [ + { id: 1, created_at: '2026-01-01T00:00:00Z', closed_at: '2026-01-01T00:01:00Z' } + ] + } + }) + ); + + await page.route('**/osm/**/changeset/1/download**', route => + route.fulfill({ body: ANCESTOR_OSC, contentType: 'application/xml' }) + ); +} + +async function driveMerge(page: Page) { + await page.goto(MERGE_URL); + await page.getByLabel(/Merge workspace/).selectOption({ index: 0 }); + await page.getByRole('button', { name: /Merge/ }).click(); + await expect(page).toHaveURL(/\/workspace\/1\/merge\/conflicts$/); +} + +// Stubs a successful commit into workspace 99. `upload` resolves with the +// uploaded osmChange once it is sent. +async function stubCommit(page: Page): Promise<{ upload: Promise }> { + let resolveUpload: (body: string) => void; + const upload = new Promise((resolve) => { + resolveUpload = resolve; + }); + + await page.route('**/api.test/workspaces', route => + route.fulfill({ status: 201, json: { workspaceId: 99 } }) + ); + await page.route('**/osm/**/workspaces/99', route => route.fulfill({ status: 200, body: '' })); + await page.route('**/osm/**/changeset/create**', route => + route.fulfill({ status: 200, body: '777' }) + ); + await page.route('**/osm/**/changeset/777/upload**', (route) => { + resolveUpload(route.request().postData() ?? ''); + return route.fulfill({ status: 200, body: '' }); + }); + await page.route('**/osm/**/changeset/777/close**', route => + route.fulfill({ status: 200, body: '' }) + ); + + return { upload }; +} + +async function startMergeWithConflict(page: Page) { + await stubConflictingMerge(page); + await driveMerge(page); +} + +test.describe('workspace merge conflicts page', () => { + test.beforeEach(async ({ page }) => { + await seedAuthenticatedSession(page); + }); + + // @test e2e: loading this page directly, with no merge in progress, redirects back to the + // merge page (assert() the URL and that the merge page renders) + test('opening it with no merge in progress redirects to the merge page', async ({ page }) => { + await stubConflictingMerge(page); + await page.goto(CONFLICTS_URL); + + // The redirect fires on mount, and this is the only test that hard-loads + // this route, so against the dev server it pays the lazy first compile and + // needs a longer wait than the 10s default. + await expect(page).toHaveURL(/\/workspace\/1\/merge$/, { timeout: 30_000 }); + + // The URL changes before the merge page renders, so it alone would not + // show that the page arrived. + await expect(page.getByRole('heading', { name: 'Merge Workspace' })).toBeVisible(); + }); + + // @test e2e: merging two workspaces with conflicts navigates to the conflicts page instead of + // committing (assert() the URL) + // This is the merge page's outline item; it is the precondition for the rest. + test('a merge with conflicts lands on this page', async ({ page }) => { + // The title check is a POST but reads only, so match the calls a commit makes: + const writes: string[] = []; + page.on('request', (request) => { + const url = request.url(); + + if (/\/workspaces$|\/osm\/.*\/workspaces\/\d+$|\/changeset\//.test(url) && request.method() !== 'GET') { + writes.push(`${request.method()} ${url}`); + } + }); + + await startMergeWithConflict(page); + + await expect(page).toHaveURL(/\/workspace\/1\/merge\/conflicts$/); + await expect(page.getByRole('button', { name: 'Commit Merge' })).toBeVisible(); + + // Nothing may be created or uploaded until the conflicts are resolved: + expect(writes).toEqual([]); + }); + + // @test e2e: leaving this page without its buttons (browser back, a header link) drops the + // merge, so coming back does not offer a merge computed from outdated data + // (assert() the redirect on return) + test('leaving by the back button drops the merge', async ({ page }) => { + await startMergeWithConflict(page); + + // The URL changes before the page finishes loading, and a page that never + // rendered is never unmounted, so going back any sooner tests nothing: + await expect(page.getByRole('button', { name: 'Commit Merge' })).toBeVisible(); + + await page.goBack(); + await expect(page).toHaveURL(/\/workspace\/1\/merge$/); + await expect(page.getByRole('heading', { name: 'Merge Workspace' })).toBeVisible(); + + await page.goForward(); + + await expect(page).toHaveURL(/\/workspace\/1\/merge$/); + await expect(page.getByRole('button', { name: 'Commit Merge' })).toHaveCount(0); + }); + + // @test e2e: loading this page with a merge in progress lists every conflict in the sidebar + // with geometry/tags badges (playwright snapshot this) + test('the sidebar lists each conflict with its badges', async ({ page }) => { + await startMergeWithConflict(page); + + // The detail toolbar repeats the element name, so scope to the sidebar. + const sidebar = page.locator('.conflicts-sidebar'); + + await expect(sidebar.getByText('node 1')).toBeVisible(); + await expect(sidebar.getByText('geometry', { exact: true })).toBeVisible(); + await expect(page.getByText(/1 of 1 conflicts require resolution/)).toBeVisible(); + await expect(sidebar).toMatchAriaSnapshot(); + }); + + // @test e2e: clicking a conflict in the sidebar shows its two versions on the map and its tag + // differences in the table below (playwright snapshot this) + // BLOCKED: asserting the two versions needs the maplibre canvas, which requires WebGL in the + // headless browser, and the fixture yields a single conflict so there is no second item to + // click. Give the merge two conflicting elements and either run with a GPU-enabled context or + // assert only the tag table, before implementing. + test.fixme('clicking a conflict shows both versions and its tag differences', async () => {}); + + // @test e2e: "Commit Merge" stays disabled until every conflict is resolved (assert() the + // disabled state) + test('Commit Merge is disabled until every conflict is resolved', async ({ page }) => { + await startMergeWithConflict(page); + + const commit = page.getByRole('button', { name: 'Commit Merge' }); + await expect(commit).toBeDisabled(); + + await page.getByRole('button', { name: 'Keep A' }).click(); + + await expect(page.getByText('All conflicts resolved.')).toBeVisible(); + await expect(commit).toBeEnabled(); + }); + + // @test e2e: "Keep A" and "Keep B" resolve a conflict in one click and keep the tags the other + // side did not dispute (assert() the resolved count and the committed osmChange) + for (const [side, lat] of [['A', '47.65'], ['B', '47.7']]) { + test(`Keep ${side} resolves the conflict in one click and keeps the undisputed tags`, async ({ page }) => { + await startMergeWithConflict(page); + const { upload } = await stubCommit(page); + + await expect(page.getByText('1 of 1 conflicts require resolution.')).toBeVisible(); + + await page.getByRole('button', { name: `Keep ${side}` }).click(); + + await expect(page.getByText('All conflicts resolved.')).toBeVisible(); + + // The tag table shows the undisputed tags as the merge computed them, + // whatever Keep did, so the upload is what shows they were kept: + await page.getByRole('button', { name: 'Commit Merge' }).click(); + const body = await upload; + + expect(body).toContain(`lat="${lat}"`); + expect(body).toContain(''); + expect(body).toContain(''); + }); + } + + // @test e2e: a non-owner sees the commit controls disabled with an explanation (assert() the + // disabled state) + test('a non-owner can review but not commit', async ({ page }) => { + await stubConflictingMerge(page); + + // Only a lead can start the merge, since the merge page disables its Merge + // button otherwise, and this page is only reachable through that merge. So the + // first read, the merge page's, reports `lead`; every read after it reports + // `contributor`, and this page's own read is the one the commit gate uses. + let reads = 0; + await page.route('**/api.test/workspaces/1', route => + route.fulfill({ + json: { ...mergeWorkspaceA, role: reads++ === 0 ? 'lead' : 'contributor' } + }) + ); + + await driveMerge(page); + + await expect(page.getByText('Only workspace owners can commit a merge.')).toBeVisible(); + + await page.getByRole('button', { name: 'Keep A' }).click(); + + // Assert the click landed first: Commit is also disabled while anything is + // unresolved, so the disabled state alone would pass if Keep A did nothing. + await expect(page.getByText('All conflicts resolved.')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Commit Merge' })).toBeDisabled(); + }); + + // @test e2e: committing creates the merged workspace and navigates to its editor, without the + // page saying no merge is in progress on the way out (assert() the URL and that + // the notice never appears) + test('committing creates the merged workspace and opens its editor', async ({ page }) => { + await startMergeWithConflict(page); + + // The notice can be on screen for only a few hundred milliseconds while the + // page leaves, too briefly for a polling assertion, so record whether it + // was ever added to the page: + await page.evaluate(() => { + const record = window as unknown as { sawNoMergeNotice: boolean }; + record.sawNoMergeNotice = false; + + new MutationObserver(() => { + if (document.body.textContent?.includes('No merge is in progress')) { + record.sawNoMergeNotice = true; + } + }).observe(document.body, { childList: true, subtree: true, characterData: true }); + }); + + await stubCommit(page); + + await page.getByRole('button', { name: 'Keep A' }).click(); + await page.getByRole('button', { name: 'Commit Merge' }).click(); + + // `datatype` decides which editor loads, and without it the Pathways + // editor loads, so the path alone would not show the right editor opened. + await expect(page).toHaveURL(/\/workspace\/99\/edit\?datatype=osw/); + + // The URL changes before the out-in transition removes this page, and the + // notice could appear during that transition, so read the record only + // once the page is gone: + await expect(page.locator('.merge-conflicts-page')).toHaveCount(0); + expect(await page.evaluate(() => + (window as unknown as { sawNoMergeNotice: boolean }).sawNoMergeNotice + )).toBe(false); + }); + + // @test e2e: a failing commit shows an error toast and re-enables the button rather than + // failing silently (assert() the toast text) + test('a failing commit surfaces an error instead of failing silently', async ({ page }) => { + await startMergeWithConflict(page); + + // Creating the merged workspace is the first call the commit makes, so this + // is the only failure the flow can report. + await page.route('**/api.test/workspaces', route => + route.fulfill({ status: 500, json: { detail: 'could not create workspace' } }) + ); + + await page.getByRole('button', { name: 'Keep A' }).click(); + await page.getByRole('button', { name: 'Commit Merge' }).click(); + + await expect(page.locator('.Toastify__toast--error')) + .toContainText('could not create workspace'); + await expect(page.getByRole('button', { name: 'Commit Merge' })).toBeEnabled(); + }); + + // @test e2e: a delete-versus-edit conflict offers "Delete Element" instead of a plain A/B + // choice, and choosing it removes the element from the merge (assert() the + // button set) + test('a delete-versus-edit conflict offers Delete Element', async ({ page }) => { + await stubConflictingMerge(page); + + // Workspace A deleted node 1; workspace B edited it. + await page.route('**/osm/**/map.json**', (route) => { + const workspace = route.request().headers()['x-workspace']; + const elements = workspace === '2' ? [editedNode(1, 47.60, { barrier: 'kerb' })] : []; + + return route.fulfill({ json: { elements } }); + }); + + await driveMerge(page); + + await expect(page.getByRole('button', { name: 'Delete Element' })).toBeVisible(); + await expect(page.getByRole('button', { name: 'Keep A' })).toHaveCount(0); + await expect(page.getByRole('button', { name: 'Keep B' })).toBeVisible(); + + await page.getByRole('button', { name: 'Delete Element' }).click(); + + await expect(page.getByText(/left out of the merged workspace/)).toBeVisible(); + await expect(page.getByText('All conflicts resolved.')).toBeVisible(); + }); + + // @test e2e: validate that all the API calls used on this page match the Swagger spec + test('the API calls match the Swagger spec', async ({ page }) => { + // Reaching this page runs the merge page's title-availability check, which + // is not in the vendored spec: it matches GET /workspaces/{workspace_id} + // and reports a bogus method violation. The create specs ignore it too. + const contract = recordContract(page, { ignoredPaths: ['workspaces/check'] }); + + await startMergeWithConflict(page); + await expect(page.getByText(/1 of 1 conflicts require resolution/)).toBeVisible(); + + expect(contract.violations()).toEqual([]); + }); +}); diff --git a/test/e2e/merge-conflicts.spec.ts-snapshots/workspace-merge-conflicts-page-the-sidebar-lists-each-conflict-with-its-badges-1.aria.yml b/test/e2e/merge-conflicts.spec.ts-snapshots/workspace-merge-conflicts-page-the-sidebar-lists-each-conflict-with-its-badges-1.aria.yml new file mode 100644 index 00000000..8f18cb26 --- /dev/null +++ b/test/e2e/merge-conflicts.spec.ts-snapshots/workspace-merge-conflicts-page-the-sidebar-lists-each-conflict-with-its-badges-1.aria.yml @@ -0,0 +1,4 @@ +- complementary: + - button " node 1 geometry" + - button "Cancel" + - button "Commit Merge " [disabled] \ No newline at end of file diff --git a/test/e2e/merge-index.spec.ts b/test/e2e/merge-index.spec.ts new file mode 100644 index 00000000..a4157c54 --- /dev/null +++ b/test/e2e/merge-index.spec.ts @@ -0,0 +1,277 @@ +import { test, expect, seedAuthenticatedSession } from './fixtures'; +import { recordContract } from './contract'; +import { mergeWorkspaceA, mergeWorkspaceB, projectGroups } from '../mocks/fixtures'; + +import type { Page } from '@playwright/test'; + +// Generated from the @test outline in pages/workspace/[id]/merge/index.vue. +// +// The page does three top-level awaits before it renders, and all three MUST be +// stubbed or it 500s on the failed fetch: +// 1. workspacesClient.getWorkspace(id) -> GET workspaces/{id} +// 2. merger.getMergeTargets() -> GET workspaces/mine +// 3. tdeiUserClient.getMyProjectGroups(...) -> GET tdei-user/project-group-roles/{subject} +// +// `mergeWorkspaceA`/`B` are the fixture pair that is eligible for each other; +// see the note on them in test/mocks/fixtures.ts. +// +// CONVENTION: tests are written to the @test comments (intended behavior). Where +// the code diverges, the test is left to FAIL (red) and the divergence is noted. + +const MERGE_URL = '/workspace/1/merge'; + +async function stubMergePage(page: Page, overrides: { workspace?: object; mine?: object[] } = {}) { + await page.route('**/api.test/workspaces/1', route => + route.fulfill({ json: { ...mergeWorkspaceA, ...overrides.workspace } }) + ); + + await page.route('**/api.test/workspaces/mine', route => + route.fulfill({ json: overrides.mine ?? [mergeWorkspaceA, mergeWorkspaceB] }) + ); + + await page.route('**/tdei-user/project-group-roles/**', route => + route.fulfill({ json: projectGroups }) + ); + + await page.route('**/api.test/workspaces/check', route => + route.fulfill({ json: { available: true } }) + ); +} + +test.describe('workspace merge page', () => { + test.beforeEach(async ({ page }) => { + await seedAuthenticatedSession(page); + }); + + // @test e2e: loading this page as a workspace owner shows a form listing the workspaces + // eligible to merge with this one (playwright snapshot this) + test('an owner sees the merge form listing eligible targets', async ({ page }) => { + await stubMergePage(page); + await page.goto(MERGE_URL); + + await expect(page.getByRole('heading', { name: 'Merge Workspace' })).toBeVisible(); + await expect(page.getByText('Only workspace owners can merge')).toHaveCount(0); + + const target = page.getByLabel(/Merge workspace/); + await expect(target).toBeVisible(); + await expect(target.locator('option')).toHaveText(['Seattle Sidewalks (survey)']); + await expect(page.locator('.merge-page')).toMatchAriaSnapshot(); + }); + + // @test e2e: loading this page as a non-owner shows the "only workspace owners can merge" + // warning and leaves the Merge button disabled (assert() the button is disabled) + test('a non-owner sees the warning and a disabled Merge button', async ({ page }) => { + await stubMergePage(page, { workspace: { role: 'contributor' } }); + await page.goto(MERGE_URL); + + await expect(page.getByText('Only workspace owners can merge workspaces.')).toBeVisible(); + await expect(page.getByRole('button', { name: /Merge/ })).toBeDisabled(); + }); + + // @test e2e: a workspace with no eligible merge targets shows the "no existing workspaces + // will merge" panel with a link back to the dashboard (playwright snapshot this) + test('no eligible targets shows the explanation and a way back', async ({ page }) => { + await stubMergePage(page, { mine: [mergeWorkspaceA] }); + await page.goto(MERGE_URL); + + await expect(page.getByText(/No existing workspaces will merge with/)).toBeVisible(); + // The control is an `` that bootstrap-vue-next also stamps with + // `role="button"`, so ARIA reports it as a button. Assert the destination + // rather than the presence: "a way back" means it actually goes back. + const back = page.getByRole('button', { name: 'Return to Dashboard' }); + + await expect(back).toBeVisible(); + await expect(back).toHaveAttribute('href', '/dashboard'); + await expect(page.locator('.merge-page')).toMatchAriaSnapshot(); + }); + + // Not an outline item: pins the guard in ThreeWayWorkspaceMerger#isEligibleWorkspace. + // Two workspaces that both lack a dataset id have no shared import, so they are + // unrelated rather than matching. + test('a workspace with no dataset id offers no targets', async ({ page }) => { + await stubMergePage(page, { + workspace: { tdeiRecordId: null }, + mine: [ + { ...mergeWorkspaceA, tdeiRecordId: null }, + { ...mergeWorkspaceB, tdeiRecordId: null } + ] + }); + await page.goto(MERGE_URL); + + await expect(page.getByText(/No existing workspaces will merge with/)).toBeVisible(); + }); + + // @test e2e: the project group picker offers only the workspace's own project group, so a + // merge cannot relocate data into another tenant (assert() the options list has + // exactly one entry) + test('the project group picker is constrained to the workspace own project group', async ({ page }) => { + await stubMergePage(page); + // Registered last so it wins: route precedence is most-recently-registered-first. + await page.route('**/tdei-user/project-group-roles/**', route => + route.fulfill({ + json: [ + ...projectGroups, + { + tdei_project_group_id: '99999999-9999-4999-8999-999999999999', + project_group_name: 'Unrelated Tenant', + roles: ['poc'] + } + ] + }) + ); + await page.goto(MERGE_URL); + + await page.getByLabel('Project Group').click(); + + await expect(page.locator('.pg-dropdown li')).toHaveText(['Puget Sound']); + }); + + // @test e2e: entering a title already used in the project group shows the "not available" + // hint and leaves Merge disabled (assert() this) + test('a duplicate title blocks the merge', async ({ page }) => { + await stubMergePage(page); + await page.route('**/api.test/workspaces/check', route => + route.fulfill({ json: { available: false } }) + ); + await page.goto(MERGE_URL); + + await page.getByLabel('Merged Workspace Title').fill('Taken name'); + + await expect(page.getByText(/already exists in this project group/)).toBeVisible(); + await expect(page.getByRole('button', { name: /Merge/ })).toBeDisabled(); + }); + + // @test e2e: a failing merge shows an error toast and re-enables the Merge button rather + // than failing silently (assert() the toast text) + test('a failing merge surfaces an error instead of failing silently', async ({ page }) => { + await stubMergePage(page); + + // merge() reads both workspaces and their common ancestor in parallel and + // reports whichever request rejects first, so failing every read with the + // same body is what makes the toast text deterministic. + const unavailable = { status: 500, json: { detail: 'workspace data unavailable' } }; + + await page.route('**/workspaces/*/bbox', route => route.fulfill(unavailable)); + await page.route('**/osm/**', route => route.fulfill(unavailable)); + + await page.goto(MERGE_URL); + await page.getByLabel(/Merge workspace/).selectOption({ index: 0 }); + + const mergeButton = page.getByRole('button', { name: /Merge/ }); + await mergeButton.click(); + + await expect(page.locator('.Toastify__toast--error')) + .toContainText('workspace data unavailable'); + await expect(mergeButton).toBeEnabled(); + }); + + // @test e2e: the form warns that the merged workspace will not be linked to a TDEI dataset, + // and for a GTFS Pathways workspace that it then cannot be exported to TDEI + // (assert() the warning text for each type) + test('the form warns that the merged workspace loses its TDEI dataset link', async ({ page }) => { + await stubMergePage(page); + await page.goto(MERGE_URL); + + await expect(page.getByText('will not be linked to a TDEI dataset, so exporting it publishes a new dataset')) + .toBeVisible(); + await expect(page.getByText('cannot be exported to TDEI')).toHaveCount(0); + }); + + test('the warning tells a GTFS Pathways owner the merge cannot be exported', async ({ page }) => { + await stubMergePage(page, { + workspace: { type: 'pathways' }, + mine: [{ ...mergeWorkspaceA, type: 'pathways' }, { ...mergeWorkspaceB, type: 'pathways' }] + }); + await page.goto(MERGE_URL); + + await expect(page.getByText('cannot be exported to TDEI. Export this data before merging')) + .toBeVisible(); + await expect(page.getByText('publishes a new dataset')).toHaveCount(0); + }); + + // @test e2e: merging two workspaces with no conflicts commits the merge and navigates to the + // new workspace's editor, centred on the merged data (assert() the URL) + test('a merge with no conflicts commits and opens the new workspace', async ({ page }) => { + await stubMergePage(page); + + // Both workspaces still hold the import untouched (version 1), so the + // three-way merge settles everything and reports no conflicts. + const imported = { + id: 1, + type: 'node', + lat: 47.6, + lon: -122.35, + version: 1, + changeset: 1, + timestamp: '2026-01-01T00:00:00Z', + user: 'tester', + uid: 7, + tags: {} + }; + + await page.route('**/workspaces/*/bbox', route => + route.fulfill({ json: { min_lon: -122.4, min_lat: 47.6, max_lon: -122.3, max_lat: 47.7 } }) + ); + await page.route('**/osm/**/map.json**', route => + route.fulfill({ json: { elements: [imported] } }) + ); + await page.route('**/osm/**/changesets.json**', route => + route.fulfill({ + json: { + changesets: [ + { id: 1, created_at: '2026-01-01T00:00:00Z', closed_at: '2026-01-01T00:01:00Z' } + ] + } + }) + ); + await page.route('**/osm/**/changeset/1/download**', route => + route.fulfill({ + contentType: 'application/xml', + body: ` + + ` + }) + ); + + await page.route('**/api.test/workspaces', route => + route.fulfill({ status: 201, json: { workspaceId: 99 } }) + ); + await page.route('**/osm/**/workspaces/99', route => route.fulfill({ status: 200, body: '' })); + await page.route('**/osm/**/changeset/create**', route => + route.fulfill({ status: 200, body: '777' }) + ); + await page.route('**/osm/**/changeset/777/upload**', route => + route.fulfill({ status: 200, body: '' }) + ); + await page.route('**/osm/**/changeset/777/close**', route => + route.fulfill({ status: 200, body: '' }) + ); + + await page.goto(MERGE_URL); + await page.getByLabel(/Merge workspace/).selectOption({ index: 0 }); + await page.getByRole('button', { name: /Merge/ }).click(); + + // `datatype` decides which editor loads, and without it the Pathways + // editor loads, so the path alone would not show the right editor opened. + await expect(page).toHaveURL(/\/workspace\/99\/edit\?datatype=osw/); + + // Without a position the editor opens wherever it was last left, not on + // the merged data. The only node is at 47.6, -122.35: + await expect(page).toHaveURL(/#map=16\/47\.6\/-122\.35$/); + }); + + // @test e2e: validate that all the API calls used on this page match the Swagger spec + test('the API calls match the Swagger spec', async ({ page }) => { + // The title-availability check is not in the vendored spec, so it matches + // GET /workspaces/{workspace_id} and reports a bogus method violation. The + // create specs ignore it for the same reason. + const contract = recordContract(page, { ignoredPaths: ['workspaces/check'] }); + + await stubMergePage(page); + await page.goto(MERGE_URL); + await expect(page.getByRole('heading', { name: 'Merge Workspace' })).toBeVisible(); + + expect(contract.violations()).toEqual([]); + }); +}); diff --git a/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-an-owner-sees-the-merge-form-listing-eligible-targets-1.aria.yml b/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-an-owner-sees-the-merge-form-listing-eligible-targets-1.aria.yml new file mode 100644 index 00000000..aa0bab96 --- /dev/null +++ b/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-an-owner-sees-the-merge-form-listing-eligible-targets-1.aria.yml @@ -0,0 +1,16 @@ +- heading "Merge Workspace" [level=1] +- text: Merge workspace +- strong: "\"Seattle Sidewalks\"" +- text: "with:" +- combobox "Merge workspace \"Seattle Sidewalks\" with:": + - option "Seattle Sidewalks (survey)" +- text: Merged Workspace Title +- textbox "Merged Workspace Title": "Merged: Seattle Sidewalks" +- text: Project Group +- textbox "Project Group": + - /placeholder: Search project groups... + - text: Puget Sound +- paragraph: The merged workspace stays in this workspace's project group. +- alert: The merged workspace will not be linked to a TDEI dataset, so exporting it publishes a new dataset rather than a new version of this one. +- button "Cancel" +- button "Merge " [disabled] \ No newline at end of file diff --git a/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-no-eligible-targets-shows-the-explanation-and-a-way-back-1.aria.yml b/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-no-eligible-targets-shows-the-explanation-and-a-way-back-1.aria.yml new file mode 100644 index 00000000..e96bcdfa --- /dev/null +++ b/test/e2e/merge-index.spec.ts-snapshots/workspace-merge-page-no-eligible-targets-shows-the-explanation-and-a-way-back-1.aria.yml @@ -0,0 +1,8 @@ +- heading "Merge Workspace" [level=1] +- text:  +- paragraph: + - text: No existing workspaces will merge with + - strong: Seattle Sidewalks + - text: . +- paragraph: Eligible workspaces must be created from the same TDEI dataset and belong to the same project group. +- button "Return to Dashboard" \ No newline at end of file diff --git a/test/mocks/fixtures.ts b/test/mocks/fixtures.ts index 9c660f80..c1b17acd 100644 --- a/test/mocks/fixtures.ts +++ b/test/mocks/fixtures.ts @@ -53,6 +53,30 @@ export const myWorkspaces = [ // A single WorkspaceResponse for `GET /api/v1/workspaces/{id}` (settings/edit/etc). export const aWorkspace = myWorkspaces[0]!; +// A pair of workspaces the merge flow accepts for each other: a workspace is an +// eligible merge target only when it shares both `tdeiRecordId` and +// `tdeiProjectGroupId` with the one being merged. The workspaces above carry +// `tdeiRecordId: null`, which is deliberately eligible for nothing. +// +// The dataset id is a UUID because the spec types `tdeiRecordId` as +// `format: uuid`, and the contract validator checks formats. +const MERGE_DATASET_ID = '33333333-3333-4333-8333-333333333333'; + +export const mergeWorkspaceA = { + ...myWorkspaces[0]!, + tdeiRecordId: MERGE_DATASET_ID +}; + +export const mergeWorkspaceB = { + ...myWorkspaces[1]!, + type: 'osw', + title: 'Seattle Sidewalks (survey)', + tdeiRecordId: MERGE_DATASET_ID, + // The base fixture is mid-import, which makes it ineligible as a merge + // target. A merge pair is two finished imports. + importStatus: 'completed' +}; + // Shape mirrors `GET project-group-roles/{subject}` (the TDEI user API — not in // the new-API OpenAPI spec). The client maps `project_group_name` → `name`. export const projectGroups = [ diff --git a/test/mocks/merge.ts b/test/mocks/merge.ts new file mode 100644 index 00000000..8ddcadaf --- /dev/null +++ b/test/mocks/merge.ts @@ -0,0 +1,64 @@ +import { vi } from 'vitest'; + +import { ThreeWayWorkspaceMerger } from '~/services/merge'; + +import type { OsmApiClient } from '~/services/osm'; +import type { WorkspacesClient } from '~/services/workspaces'; +import type { OsmElement } from '~/types/osm'; +import type { Workspace } from '~/types/workspaces'; + +// Shared workspace fixtures and merge runners for the merge suites. + +export const DATASET_ID = 'dataset-1'; +export const PROJECT_GROUP_ID = '11111111-1111-4111-8111-111111111111'; + +export function workspace(overrides: Partial = { }): Workspace { + return { + id: 1, + type: 'osw', + title: 'Workspace A', + tdeiRecordId: DATASET_ID, + tdeiProjectGroupId: PROJECT_GROUP_ID, + createdAt: new Date('2026-01-01T00:00:00Z'), + createdBy: 'tester', + createdByName: 'Tester', + externalAppAccess: 0, + ...overrides + }; +} + +export interface MergeScenario { + ancestor: OsmElement[]; + a: OsmElement[]; + b: OsmElement[]; +} + +export function makeOsmClient(scenario: MergeScenario) { + return { + getWorkspaceData: vi.fn(async (id: number) => + id === 1 ? scenario.a : scenario.b + ), + listChangesets: vi.fn(async () => [ + { id: 9, created_at: new Date('2026-02-01T00:00:00Z') }, + { id: 4, created_at: new Date('2026-01-01T00:00:00Z') } + ]), + getOsmChange: vi.fn(async () => ({ + create: scenario.ancestor, + modify: [], + delete: [] + })) + }; +} + +export async function runMerge(scenario: MergeScenario) { + const osmClient = makeOsmClient(scenario); + const merger = new ThreeWayWorkspaceMerger( + { } as unknown as WorkspacesClient, + osmClient as unknown as OsmApiClient, + workspace() + ); + + const result = await merger.merge(2, PROJECT_GROUP_ID, 'Merged'); + + return { result, osmClient }; +} diff --git a/test/mocks/osm-elements.ts b/test/mocks/osm-elements.ts new file mode 100644 index 00000000..bf6cd153 --- /dev/null +++ b/test/mocks/osm-elements.ts @@ -0,0 +1,32 @@ +import type { OsmNode, OsmRelation, OsmRelationRef, OsmWay } from '~/types/osm'; + +// Shared OSM element factories for the merge and osmChange suites. Every field +// has a fixed default so a test only states what it is actually about. + +const BASE = { + version: 1, + changeset: 1, + timestamp: new Date('2026-01-01T00:00:00Z'), + user: 'tester', + uid: 7 +}; + +export function osmNode(id: number, overrides: Partial = { }): OsmNode { + return { ...BASE, id, type: 'node', lat: 47.6, lon: -122.3, ...overrides }; +} + +export function osmWay( + id: number, + nodes: number[], + overrides: Partial = { } +): OsmWay { + return { ...BASE, id, type: 'way', nodes, ...overrides }; +} + +export function osmRelation( + id: number, + members: OsmRelationRef[], + overrides: Partial = { } +): OsmRelation { + return { ...BASE, id, type: 'relation', members, ...overrides }; +} diff --git a/test/unit/components/ConflictTagTable.test.ts b/test/unit/components/ConflictTagTable.test.ts new file mode 100644 index 00000000..81913d4b --- /dev/null +++ b/test/unit/components/ConflictTagTable.test.ts @@ -0,0 +1,130 @@ +import { mount } from '@vue/test-utils'; +import { describe, expect, it } from 'vitest'; +import { markRaw, reactive } from 'vue'; + +import ConflictTagTable from '~/components/merge/ConflictTagTable.vue'; +import { MAX_TAG_LENGTH } from '~/services/merge'; + +import { osmNode } from '../../mocks/osm-elements'; + +import type { ResolutionState } from '~/composables/useConflictResolutions'; +import type { MergeConflict } from '~/services/merge'; +import type { OsmTags } from '~/types/osm'; + +// A tag key is arbitrary data and OSM reserves none, so a key that happens to +// name an Object.prototype member must still read as absent when a side does +// not carry it. Reading it off the prototype instead surfaces a function, which +// renders as source, is emitted as the chosen value, and reaches the upload. +const PROTOTYPE_KEYS = ['toString', 'constructor', 'valueOf', 'hasOwnProperty', '__proto__']; + +function conflictOver(key: string, aTags: OsmTags, bTags: OsmTags): MergeConflict { + return { + number: 1, + kind: 'both-modified', + type: 'node', + elementId: 1, + a: osmNode(1, { version: 2, tags: aTags }), + b: osmNode(1, { version: 2, tags: bTags }), + base: osmNode(1, { tags: { [key]: 'original' } }), + geometryConflict: false, + tagConflicts: [key], + mergedTags: { } + }; +} + +// Mirrors how the page supplies it: the resolution lives inside a reactive +// array. That matters because Vue's get trap shadows `hasOwnProperty`, so a +// plain object here would not exercise the path the page actually takes. +function resolutionFor(tags: OsmTags = { }): ResolutionState { + const resolutions = reactive([ + { resolved: false, chosenSide: 'a', tags: markRaw(tags), decidedTags: [] } + ]); + + return resolutions[0]!; +} + +function mountTable(conflict: MergeConflict, resolution: ResolutionState) { + return mount(ConflictTagTable, { + props: { conflict, resolution }, + global: { stubs: { AppIcon: true } } + }); +} + +describe('ConflictTagTable prototype-named tag keys', () => { + it.each(PROTOTYPE_KEYS)( + 'renders a %s tag the side does not carry as absent, not as an inherited member', + (key) => { + // A deleted the tag; B changed it. The row is a genuine dispute. + const wrapper = mountTable( + conflictOver(key, { }, { [key]: 'from-b' }), + resolutionFor() + ); + + expect(wrapper.html()).not.toContain('native code'); + expect(wrapper.html()).toContain('(none)'); + } + ); + + it.each(PROTOTYPE_KEYS)('emits a string, never a function, when %s is picked', async (key) => { + const wrapper = mountTable( + conflictOver(key, { }, { [key]: 'from-b' }), + resolutionFor() + ); + + // The A cell is the side that deleted the tag, so picking it means "drop". + await wrapper.findAll('.pick-cell')[0]!.trigger('click'); + + const emitted = wrapper.emitted('set-tag'); + + expect(emitted).toHaveLength(1); + expect(emitted![0]![1]).toBe(''); + }); + + it('does not mark a side chosen because both reads fell through to the prototype', () => { + // Neither side carries `toString`, and nothing has been decided, so no cell + // should be highlighted. A prototype read makes both comparands equal. + const conflict = conflictOver('toString', { }, { }); + const wrapper = mountTable(conflict, resolutionFor()); + + expect(wrapper.find('.table-danger').exists()).toBe(false); + expect(wrapper.find('.table-primary').exists()).toBe(false); + }); + + it('renders a real hasOwnProperty tag value rather than Vue reactivity plumbing', () => { + // Vue's reactive get trap returns its own `hasOwnProperty` implementation + // for that key, shadowing a real own property, so the resolution's tag map + // is kept out of the reactive graph. + const wrapper = mountTable( + conflictOver('hasOwnProperty', { hasOwnProperty: 'left' }, { hasOwnProperty: 'right' }), + resolutionFor({ hasOwnProperty: 'chosen' }) + ); + + expect(wrapper.html()).not.toContain('function hasOwnProperty'); + expect(wrapper.html()).not.toContain('track(obj'); + expect(wrapper.find('input').attributes('value')).toBe('chosen'); + }); + + it('still shows a prototype-named key that is a real tag on both sides', () => { + const wrapper = mountTable( + conflictOver('toString', { toString: 'left' }, { toString: 'right' }), + resolutionFor() + ); + + expect(wrapper.html()).toContain('left'); + expect(wrapper.html()).toContain('right'); + expect(wrapper.html()).not.toContain('native code'); + }); +}); + +describe('ConflictTagTable custom value input', () => { + it('caps a typed value at the length the merge validator enforces', () => { + // A value the input accepts but the validator rejects fails the whole + // upload after the destination workspace has already been created. + const wrapper = mountTable( + conflictOver('surface', { surface: 'left' }, { surface: 'right' }), + resolutionFor() + ); + + expect(wrapper.find('input').attributes('maxlength')).toBe(String(MAX_TAG_LENGTH)); + }); +}); diff --git a/test/unit/composables/useConflictResolutions.test.ts b/test/unit/composables/useConflictResolutions.test.ts new file mode 100644 index 00000000..a0a79ddf --- /dev/null +++ b/test/unit/composables/useConflictResolutions.test.ts @@ -0,0 +1,277 @@ +import { describe, expect, it } from 'vitest'; + +import { useConflictResolutions } from '~/composables/useConflictResolutions'; + +import { osmNode } from '../../mocks/osm-elements'; + +import type { MergeConflict } from '~/services/merge'; + +function conflict(overrides: Partial = { }): MergeConflict { + return { + number: 1, + kind: 'both-modified', + type: 'node', + elementId: 1, + a: osmNode(1, { lat: 10, version: 2, tags: { highway: 'footway', surface: 'asphalt' } }), + b: osmNode(1, { lat: 20, version: 2, tags: { highway: 'footway', surface: 'concrete' } }), + geometryConflict: true, + tagConflicts: ['surface'], + mergedTags: { highway: 'footway', surface: 'asphalt' }, + ...overrides + }; +} + +// Vue's reactive proxy special-cases `hasOwnProperty`, and plain object +// literals inherit the rest, so a tag literally named one of these reads back +// as a function unless the maps are kept raw and read as own properties. Tag +// keys are arbitrary data: OSM reserves none. +// +// `__proto__` is the one that breaks differently. Reading it is not the +// problem, writing it is: assigning to `__proto__` on a plain object literal +// runs the inherited setter, which ignores a string, so the tag disappears +// instead of being stored. +const PROTOTYPE_KEYS = [ + 'hasOwnProperty', + 'toString', + 'constructor', + 'valueOf', + 'isPrototypeOf', + '__proto__' +]; + +function conflictOverKey(key: string): MergeConflict { + return conflict({ + a: osmNode(1, { lat: 10, version: 2, tags: { [key]: 'from-a' } }), + b: osmNode(1, { lat: 10, version: 2, tags: { [key]: 'from-b' } }), + base: osmNode(1, { tags: { [key]: 'original' } }), + geometryConflict: false, + tagConflicts: [key], + mergedTags: { [key]: 'from-a' } + }); +} + +describe('useConflictResolutions with prototype-named tag keys', () => { + it.each(PROTOTYPE_KEYS)('keepSide carries the real value for a tag named %s', (key) => { + const { keepSide, resolutions } = useConflictResolutions([conflictOverKey(key)]); + + keepSide(0, 'b'); + + expect(resolutions[0]!.tags[key]).toBe('from-b'); + }); + + it.each(PROTOTYPE_KEYS)('setTag stores a real value for a tag named %s', (key) => { + const { setTag, resolutions } = useConflictResolutions([conflictOverKey(key)]); + + setTag(0, key, 'typed-in'); + + expect(resolutions[0]!.tags[key]).toBe('typed-in'); + }); + + it('drops such a key when the kept side deleted it', () => { + const { keepSide, resolutions } = useConflictResolutions([ + conflict({ + a: osmNode(1, { lat: 10, version: 2, tags: { } }), + b: osmNode(1, { lat: 10, version: 2, tags: { hasOwnProperty: 'from-b' } }), + base: osmNode(1, { tags: { hasOwnProperty: 'original' } }), + geometryConflict: false, + tagConflicts: ['hasOwnProperty'], + mergedTags: { } + }) + ]); + + keepSide(0, 'a'); + + expect(Object.keys(resolutions[0]!.tags)).toEqual([]); + }); +}); + +describe('useConflictResolutions', () => { + it('starts every conflict unresolved, seeded with the merged tags', () => { + const { resolutions, unresolvedCount } = useConflictResolutions([conflict()]); + + expect(unresolvedCount.value).toBe(1); + expect(resolutions[0]!.tags).toEqual({ highway: 'footway', surface: 'asphalt' }); + }); + + it('needs only the tags settled when the geometry is not in dispute', () => { + // The merge settles the geometry itself when only one side moved, so the + // user is asked to pick a side only for the tags. + const { resolutions } = useConflictResolutions([ + conflict({ + geometryConflict: false, + a: osmNode(1, { lat: 10, version: 2, tags: { surface: 'asphalt' } }), + b: osmNode(1, { lat: 10, version: 2, tags: { surface: 'concrete' } }) + }) + ]); + + expect(resolutions[0]!.chosenSide).toBe('a'); + }); + + it('resolves a whole conflict when a side is kept', () => { + const { keepSide, isResolved, resolutions } = useConflictResolutions([conflict()]); + + keepSide(0, 'b'); + + expect(isResolved(0)).toBe(true); + expect(resolutions[0]!.chosenSide).toBe('b'); + expect(resolutions[0]!.tags).toEqual({ + highway: 'footway', + surface: 'concrete' + }); + }); + + it('keeps tags the other side added when one side is kept', () => { + // "Keep A" settles the disputed keys in A's favor. It does not throw away + // work B did on keys nobody disputed. + const { keepSide, resolutions } = useConflictResolutions([ + conflict({ + a: osmNode(1, { lat: 10, version: 2, tags: { surface: 'asphalt' } }), + b: osmNode(1, { lat: 20, version: 2, tags: { surface: 'concrete', lit: 'yes' } }), + mergedTags: { surface: 'asphalt', lit: 'yes' } + }) + ]); + + keepSide(0, 'a'); + + expect(resolutions[0]!.tags).toEqual({ surface: 'asphalt', lit: 'yes' }); + }); + + it('drops a disputed tag the kept side deleted', () => { + const { keepSide, resolutions } = useConflictResolutions([ + conflict({ + a: osmNode(1, { lat: 10, version: 2, tags: { } }), + b: osmNode(1, { lat: 20, version: 2, tags: { surface: 'concrete' } }), + tagConflicts: ['surface'], + mergedTags: { surface: 'concrete' } + }) + ]); + + keepSide(0, 'a'); + + expect(resolutions[0]!.tags).not.toHaveProperty('surface'); + }); + + it('does not take a disputed tag off the prototype when the kept side lacks it', () => { + // A tag key is arbitrary data. `toString` and friends must read as absent + // rather than resolving to the inherited member. + const { keepSide, resolutions } = useConflictResolutions([ + conflict({ + a: osmNode(1, { lat: 10, version: 2, tags: { } }), + b: osmNode(1, { lat: 20, version: 2, tags: { toString: 'yes' } }), + tagConflicts: ['toString'], + mergedTags: { toString: 'yes' } + }) + ]); + + keepSide(0, 'a'); + + expect(Object.keys(resolutions[0]!.tags)).toEqual([]); + }); + + it('stays unresolved until every disputed tag has been decided', () => { + const { chooseSide, setTag, isResolved } = useConflictResolutions([ + conflict({ tagConflicts: ['surface', 'width'] }) + ]); + + chooseSide(0, 'a'); + expect(isResolved(0)).toBe(false); + + setTag(0, 'surface', 'asphalt'); + expect(isResolved(0)).toBe(false); + + setTag(0, 'width', '2'); + expect(isResolved(0)).toBe(true); + }); + + it('stays unresolved until a geometry side is chosen', () => { + const { setTag, isResolved, chooseSide } = useConflictResolutions([conflict()]); + + setTag(0, 'surface', 'asphalt'); + expect(isResolved(0)).toBe(false); + + chooseSide(0, 'b'); + expect(isResolved(0)).toBe(true); + }); + + it('treats an empty value as dropping the tag, and counts it as decided', () => { + const { setTag, chooseSide, resolutions, isResolved } = useConflictResolutions([ + conflict() + ]); + + chooseSide(0, 'a'); + setTag(0, 'surface', ''); + + expect(resolutions[0]!.tags).not.toHaveProperty('surface'); + expect(isResolved(0)).toBe(true); + }); + + it('resolves a delete-versus-edit conflict by dropping the element', () => { + const { dropElement, isResolved, resolutions } = useConflictResolutions([ + conflict({ kind: 'deleted-in-a', tagConflicts: ['surface'] }) + ]); + + dropElement(0); + + expect(isResolved(0)).toBe(true); + expect(resolutions[0]!.chosenSide).toBe('delete'); + }); + + it('tracks progress across several conflicts', () => { + const { keepSide, unresolvedCount } = useConflictResolutions([ + conflict({ number: 1 }), + conflict({ number: 2 }) + ]); + + expect(unresolvedCount.value).toBe(2); + + keepSide(0, 'a'); + expect(unresolvedCount.value).toBe(1); + + keepSide(1, 'b'); + expect(unresolvedCount.value).toBe(0); + }); + + it('reports which tags the user has decided', () => { + const { setTag, resolutions } = useConflictResolutions([conflict()]); + + expect(resolutions[0]!.decidedTags).toEqual([]); + setTag(0, 'surface', 'asphalt'); + expect(resolutions[0]!.decidedTags).toEqual(['surface']); + }); +}); + +describe('conflicts that need a decision before they count as resolved', () => { + // A delete-versus-edit conflict has no geometry dispute: the deleted side is + // a copy of the ancestor, so `geometryConflict` is false and `chosenSide` is + // seeded rather than left null. That seed must not be mistaken for a + // decision. Whether a deletion stands is the one call the user has to make, + // and committing it silently would revert the other side's delete. + it('starts a delete-versus-edit conflict unresolved, with no tags in dispute', () => { + const { unresolvedCount, isResolved } = useConflictResolutions([ + conflict({ kind: 'deleted-in-a', geometryConflict: false, tagConflicts: [] }) + ]); + + expect(isResolved(0)).toBe(false); + expect(unresolvedCount.value).toBe(1); + }); + + it('resolves it once the user keeps a side', () => { + const { unresolvedCount, keepSide } = useConflictResolutions([ + conflict({ kind: 'deleted-in-a', geometryConflict: false, tagConflicts: [] }) + ]); + + keepSide(0, 'b'); + + expect(unresolvedCount.value).toBe(0); + }); + + it('resolves it once the user honors the deletion', () => { + const { unresolvedCount, dropElement } = useConflictResolutions([ + conflict({ kind: 'deleted-in-a', geometryConflict: false, tagConflicts: [] }) + ]); + + dropElement(0); + + expect(unresolvedCount.value).toBe(0); + }); +}); diff --git a/test/unit/services/export-tdei.test.ts b/test/unit/services/export-tdei.test.ts new file mode 100644 index 00000000..83c43c76 --- /dev/null +++ b/test/unit/services/export-tdei.test.ts @@ -0,0 +1,76 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { TdeiExporter, TdeiExporterContext } from '~/services/export/tdei'; +import { aWorkspace } from '../../mocks/fixtures'; + +import type { OsmApiClient } from '~/services/osm'; +import type { TdeiClient } from '~/services/tdei'; +import type { Workspace } from '~/types/workspaces'; + +// The fixture is shaped like the API response, which sends null for fields +// the client type declares optional: +const workspace = { + ...aWorkspace, + type: 'osw', + tdeiServiceId: 'service-1' +} as unknown as Workspace; + +function makeExporter(bbox: unknown) { + const osmClient = { + getWorkspaceBbox: vi.fn().mockResolvedValue(bbox), + // Stops the export at the first step past the checks, so a test can tell + // whether it got that far without stubbing the whole upload: + exportWorkspaceXml: vi.fn().mockRejectedValue(new Error('reached the export')) + }; + const tdeiClient = { + convertDataset: vi.fn(), + uploadOswDataset: vi.fn() + }; + const exporter = new TdeiExporter( + tdeiClient as unknown as TdeiClient, + osmClient as unknown as OsmApiClient, + new TdeiExporterContext() + ); + + return { exporter, osmClient, tdeiClient }; +} + +describe('TdeiExporter.upload', () => { + it('refuses a workspace with no bounding box before exporting anything', async () => { + const { exporter, osmClient, tdeiClient } = makeExporter(undefined); + + await exporter.upload(workspace, { } as never); + + expect(exporter.context.error).toContain('holds no data, so there is nothing to export'); + expect(osmClient.exportWorkspaceXml).not.toHaveBeenCalled(); + expect(tdeiClient.convertDataset).not.toHaveBeenCalled(); + expect(tdeiClient.uploadOswDataset).not.toHaveBeenCalled(); + }); + + it('reuses the bounding box it checked as the dataset area', async () => { + const { exporter, osmClient } = makeExporter({ + min_lon: -122.4, + min_lat: 47.6, + max_lon: -122.2, + max_lat: 47.8 + }); + const metadata: Record = { }; + + await exporter.upload(workspace, metadata as never); + + expect(osmClient.getWorkspaceBbox).toHaveBeenCalledTimes(1); + expect(osmClient.exportWorkspaceXml).toHaveBeenCalledWith(workspace.id); + expect(exporter.context.error).not.toContain('holds no data'); + expect(metadata.dataset_area).toMatchObject({ type: 'FeatureCollection' }); + }); + + it('keeps a dataset area the user already supplied', async () => { + const { exporter } = makeExporter({ min_lon: 1, min_lat: 2, max_lon: 3, max_lat: 4 }); + const area = { type: 'FeatureCollection', features: [] }; + const metadata: Record = { dataset_area: area }; + + await exporter.upload(workspace, metadata as never); + + expect(metadata.dataset_area).toBe(area); + }); +}); diff --git a/test/unit/services/merge-commit.test.ts b/test/unit/services/merge-commit.test.ts new file mode 100644 index 00000000..e67a87d3 --- /dev/null +++ b/test/unit/services/merge-commit.test.ts @@ -0,0 +1,309 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { WorkspaceMergerFactory } from '~/services/merge'; +import { OsmElementLookup } from '~/util/osm'; + +import { DATASET_ID, PROJECT_GROUP_ID } from '../../mocks/merge'; +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +import type { MergeResult } from '~/services/merge'; +import type { OsmApiClient } from '~/services/osm'; +import type { WorkspacesClient } from '~/services/workspaces'; + +interface CommitOverrides { + upload?: () => Promise; + elements?: OsmElementLookup; + ancestor?: OsmElementLookup; +} + +function makeCommitStubs(overrides: CommitOverrides = { }) { + const workspacesClient = { + createBlankWorkspace: vi.fn(async () => 42), + deleteWorkspace: vi.fn(async () => undefined) + }; + + const osmClient = { + createChangeset: vi.fn(async () => 777), + uploadChangeset: vi.fn( + async (_workspaceId: number, _changesetId: number, _osc: string): Promise => + overrides.upload ? await overrides.upload() : '' + ), + closeChangeset: vi.fn(async () => undefined) + }; + + const factory = new WorkspaceMergerFactory( + workspacesClient as unknown as WorkspacesClient, + osmClient as unknown as OsmApiClient + ); + + const result: MergeResult = { + type: 'osw', + title: 'Merged', + workspaceIdA: 1, + workspaceIdB: 2, + tdeiDatasetId: DATASET_ID, + tdeiProjectGroupId: PROJECT_GROUP_ID, + elements: overrides.elements + ?? new OsmElementLookup([osmNode(1, { version: 3 })]), + conflicts: [], + ancestor: overrides.ancestor ?? new OsmElementLookup() + }; + + return { factory, workspacesClient, osmClient, result }; +} + +describe('WorkspaceMergerFactory.commit', () => { + it('provisions the workspace in the OSM database, not just the workspace row', async () => { + // createWorkspace only inserts the row. Without the OSM tenant schema the + // very next call, createChangeset, has no database to write to. + const { factory, workspacesClient, result } = makeCommitStubs(); + + await factory.commit(result); + + expect(workspacesClient.createBlankWorkspace).toHaveBeenCalledWith( + expect.objectContaining({ + title: 'Merged', + type: 'osw', + tdeiProjectGroupId: PROJECT_GROUP_ID + }) + ); + }); + + it('closes the changeset it opened', async () => { + const { factory, osmClient, result } = makeCommitStubs(); + + await factory.commit(result); + + expect(osmClient.closeChangeset).toHaveBeenCalledWith(42, 777); + }); + + it('uploads the elements as an osmChange with negative create IDs', async () => { + const { factory, osmClient, result } = makeCommitStubs(); + + await factory.commit(result); + + const [, , osc] = osmClient.uploadChangeset.mock.calls[0]!; + + expect(osc).toMatch(//); + expect(osc).toMatch(/id="-1"/); + expect(osc).not.toMatch(/]*id="1"/); + }); + + it('deletes the new workspace when the upload fails, so a retry leaves no orphan', async () => { + const { factory, workspacesClient, result } = makeCommitStubs({ + upload: async () => { throw new Error('upload rejected'); } + }); + + await expect(factory.commit(result)).rejects.toThrow('upload rejected'); + expect(workspacesClient.deleteWorkspace).toHaveBeenCalledWith(42); + }); + + it('reports the original failure even when the cleanup also fails', async () => { + const { factory, workspacesClient, result } = makeCommitStubs({ + upload: async () => { throw new Error('upload rejected'); } + }); + + workspacesClient.deleteWorkspace.mockRejectedValue(new Error('delete failed')); + + await expect(factory.commit(result)).rejects.toThrow('upload rejected'); + }); + + it('rejects a dangling reference before it creates the workspace', async () => { + // Reported after the create, this would leave an empty workspace behind on + // every attempt at a resolution the builder cannot serialize. + const { factory, workspacesClient, result } = makeCommitStubs(); + const broken = new OsmElementLookup([osmNode(1), osmWay(10, [1, 2])]); + + await expect(factory.commit(result, broken)).rejects + .toThrow(/merge is incomplete: way 10 references node 2/); + + expect(workspacesClient.createBlankWorkspace).not.toHaveBeenCalled(); + }); + + it('commits a caller-supplied element set when conflicts were resolved', async () => { + const { factory, osmClient, result } = makeCommitStubs(); + const resolved = new OsmElementLookup([osmNode(1, { version: 3, tags: { a: 'b' } })]); + + await factory.commit(result, resolved); + + const [, , osc] = osmClient.uploadChangeset.mock.calls[0]!; + + expect(osc).toMatch(/k="a"/); + }); + + it('carries the dataset details over to the merged workspace', async () => { + const { factory, workspacesClient, result } = makeCommitStubs(); + + result.description = 'Merged sidewalks'; + result.tdeiServiceId = 'service-9'; + result.tdeiMetadata = '{"area":1}'; + + await factory.commit(result); + + expect(workspacesClient.createBlankWorkspace).toHaveBeenCalledWith( + expect.objectContaining({ + description: 'Merged sidewalks', + tdeiServiceId: 'service-9', + tdeiMetadata: '{"area":1}' + }) + ); + }); + + // The create endpoint reads a payload carrying both a dataset id and a + // project group id as "import this TDEI dataset": it marks the workspace as + // importing and queues a job that writes the whole source dataset into it. + // Sending the id here would have every merge upload its result into a + // workspace that a background import is simultaneously filling. + it('does not send a dataset id, which the create endpoint reads as an import request', async () => { + const { factory, workspacesClient, result } = makeCommitStubs(); + + await factory.commit(result); + + expect(workspacesClient.createBlankWorkspace).toHaveBeenCalledWith( + expect.not.objectContaining({ tdeiRecordId: expect.anything() }) + ); + expect(workspacesClient.createBlankWorkspace).toHaveBeenCalledWith( + expect.objectContaining({ tdeiProjectGroupId: PROJECT_GROUP_ID }) + ); + }); + + it('rejects a tag the API cannot store before it creates the workspace', async () => { + // Reported after the create, this would leave an empty workspace behind, + // and the server's own error names neither the element nor the tag. + const { factory, workspacesClient, result } = makeCommitStubs({ + elements: new OsmElementLookup([osmNode(1, { tags: { note: 'x'.repeat(256) } })]) + }); + + await expect(factory.commit(result)).rejects + .toThrow(/cannot be uploaded: node 1: the value of "note" is longer than 255/); + + expect(workspacesClient.createBlankWorkspace).not.toHaveBeenCalled(); + }); + + it('names the first three problems and counts the rest', async () => { + const tags = { note: 'x'.repeat(256) }; + const { factory, result } = makeCommitStubs({ + elements: new OsmElementLookup( + [1, 2, 3, 4, 5].map(id => osmNode(id, { tags })) + ) + }); + + await expect(factory.commit(result)).rejects.toThrow(/, and 2 more/); + }); + + it('keeps the merged workspace when only the close fails', async () => { + // The upload has already succeeded, so the merged data is in the new + // workspace. Deleting it over a failed close would discard the merge. + const { factory, workspacesClient, osmClient, result } = makeCommitStubs(); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + osmClient.closeChangeset.mockRejectedValue(new Error('close failed')); + + // Restored in `finally`: a failed assertion here would otherwise leave the + // spy in place for every later test in this file. + try { + await expect(factory.commit(result)).resolves.toBe(42); + expect(workspacesClient.deleteWorkspace).not.toHaveBeenCalled(); + } + finally { + warn.mockRestore(); + } + }); +}); + +describe('commit repairs an unresolved element set', () => { + // One side deleted elements the other side's untouched way still uses. That + // is an uncontested deletion, not a conflict, so the conflicts page is never + // reached and nothing else would repair it. + function danglingMerge() { + return makeCommitStubs({ + // The way survived; the vertices it uses did not. + elements: new OsmElementLookup([osmWay(50, [5, 6])]), + ancestor: new OsmElementLookup([osmNode(5), osmNode(6), osmWay(50, [5, 6])]) + }); + } + + it('restores references on a merge that produced no conflicts', async () => { + const { factory, osmClient, result } = danglingMerge(); + + await expect(factory.commit(result)).resolves.toBe(42); + + const [, , osc] = osmClient.uploadChangeset.mock.calls[0]!; + const doc = new DOMParser().parseFromString(osc, 'application/xml'); + + // Both vertices came back, so the way's refs resolve: + expect(doc.getElementsByTagName('node')).toHaveLength(2); + }); + + it('commits the caller-supplied resolved set rather than the merged one', async () => { + const { factory, osmClient, result } = danglingMerge(); + + // Only node 5 kept: the user deleted node 6 and the way was dropped with it. + await factory.commit(result, new OsmElementLookup([osmNode(5)])); + + const [, , osc] = osmClient.uploadChangeset.mock.calls[0]!; + const doc = new DOMParser().parseFromString(osc, 'application/xml'); + + expect(doc.getElementsByTagName('node')).toHaveLength(1); + expect(doc.getElementsByTagName('way')).toHaveLength(0); + }); + + it('tells the user which resolution to change when a resolved set dangles', async () => { + // The two branches of the message differ: a resolved set names an action + // the user can take, an unresolved one has no resolution to point at. + const { factory, result } = danglingMerge(); + + await expect(factory.commit(result, new OsmElementLookup([osmWay(50, [5, 6]), osmNode(5)]))) + .rejects.toThrow(/Change the resolution that removes those elements/); + }); + + it('reports an unresolved set the ancestor cannot repair as unmergeable', async () => { + const { factory, workspacesClient, result } = makeCommitStubs({ + elements: new OsmElementLookup([osmWay(50, [5, 6])]), + // Node 6 is missing from the ancestor too, so the repair cannot supply it: + ancestor: new OsmElementLookup([osmNode(5)]) + }); + + await expect(factory.commit(result)).rejects + .toThrow(/way 50 references node 6\. The two workspaces cannot be merged without the missing elements\./); + + expect(workspacesClient.createBlankWorkspace).not.toHaveBeenCalled(); + }); + + it('reports a resolved set that still references a deleted element', async () => { + // A resolved set has already been repaired, with the user's deletions + // honored. Restoring node 6 from the ancestor here would undo the + // deletion, so the contradiction is reported instead. + const { factory, workspacesClient, result } = danglingMerge(); + + const resolved = new OsmElementLookup([osmWay(50, [5, 6]), osmNode(5)]); + + await expect(factory.commit(result, resolved)).rejects + .toThrow(/way 50 references node 6/); + + expect(workspacesClient.createBlankWorkspace).not.toHaveBeenCalled(); + }); +}); + +describe('osmChange ordering', () => { + it('emits a relation after the relation it references', async () => { + const child = osmRelation(1, []); + const parent = osmRelation(2, [{ type: 'relation', ref: 1, role: 'sub' }]); + + const { factory, osmClient, result } = makeCommitStubs({ + // Parent first, so only dependency ordering can fix the sequence: + elements: new OsmElementLookup([parent, child]) + }); + + await factory.commit(result); + + const [, , osc] = osmClient.uploadChangeset.mock.calls[0]!; + const doc = new DOMParser().parseFromString(osc, 'application/xml'); + const relations = Array.from(doc.getElementsByTagName('relation')); + + // The child must be emitted first so the parent's member ref resolves: + expect(relations[0]!.getElementsByTagName('member')).toHaveLength(0); + expect(relations[1]!.getElementsByTagName('member')[0]!.getAttribute('ref')) + .toBe(relations[0]!.getAttribute('id')); + }); +}); diff --git a/test/unit/services/merge-reference-repair.test.ts b/test/unit/services/merge-reference-repair.test.ts new file mode 100644 index 00000000..bfcc03e5 --- /dev/null +++ b/test/unit/services/merge-reference-repair.test.ts @@ -0,0 +1,176 @@ +import { describe, expect, it } from 'vitest'; + +import { applyResolutions, findDanglingReferences } from '~/services/merge'; +import { OsmElementLookup } from '~/util/osm'; + +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +describe('applyResolutions reference repair', () => { + it('restores the vertices of a way the user chose to keep', () => { + // Deleting a line in the editor deletes its untagged vertices too. When + // the other side edited that line, the vertices are dropped as an + // uncontested deletion while the line becomes a conflict, so keeping the + // line would leave it pointing at nothing and the upload would be refused. + const ancestor = new OsmElementLookup([ + osmNode(1), osmNode(2), osmWay(10, [1, 2]) + ]); + + const conflict = { + number: 1, + kind: 'deleted-in-a' as const, + type: 'way' as const, + elementId: 10, + a: osmWay(10, [1, 2]), + b: osmWay(10, [1, 2], { version: 2, tags: { surface: 'asphalt' } }), + base: osmWay(10, [1, 2]), + geometryConflict: false, + tagConflicts: [], + mergedTags: { surface: 'asphalt' } + }; + + const output = applyResolutions( + new OsmElementLookup(), + [conflict], + [{ resolved: true, chosenSide: 'b', tags: { surface: 'asphalt' } }], + ancestor + ); + + expect(output.has('way', 10)).toBe(true); + expect(output.has('node', 1)).toBe(true); + expect(output.has('node', 2)).toBe(true); + expect(findDanglingReferences(output)).toEqual([]); + }); + + it('follows references the restored elements themselves add', () => { + const ancestor = new OsmElementLookup([ + osmNode(1), osmWay(10, [1]), osmRelation(20, [{ type: 'way', ref: 10, role: '' }]) + ]); + + const conflict = { + number: 1, + kind: 'deleted-in-a' as const, + type: 'relation' as const, + elementId: 20, + a: osmRelation(20, [{ type: 'way', ref: 10, role: '' }]), + b: osmRelation(20, [{ type: 'way', ref: 10, role: '' }], { version: 2 }), + base: osmRelation(20, [{ type: 'way', ref: 10, role: '' }]), + geometryConflict: false, + tagConflicts: [], + mergedTags: { } + }; + + const output = applyResolutions( + new OsmElementLookup(), + [conflict], + [{ resolved: true, chosenSide: 'b', tags: { } }], + ancestor + ); + + // The relation pulls back the way, which in turn pulls back its node: + expect(output.has('way', 10)).toBe(true); + expect(output.has('node', 1)).toBe(true); + expect(findDanglingReferences(output)).toEqual([]); + }); + + it('leaves a reference the ancestor cannot supply for the caller to report', () => { + const conflict = { + number: 1, + kind: 'both-modified' as const, + type: 'way' as const, + elementId: 10, + a: osmWay(10, [1, 77], { version: 2 }), + b: osmWay(10, [1, 77], { version: 2 }), + base: osmWay(10, [1, 77]), + geometryConflict: false, + tagConflicts: [], + mergedTags: { } + }; + + const output = applyResolutions( + new OsmElementLookup([osmNode(1)]), + [conflict], + [{ resolved: true, chosenSide: 'a', tags: { } }], + new OsmElementLookup([osmNode(1)]) + ); + + expect(findDanglingReferences(output)).toEqual(['way 10 references node 77']); + }); + + it('does not resurrect an element the user chose to delete', () => { + // The kept way still references the deleted node, so the restore pass + // would otherwise put the ancestor's copy back, reversing the user's + // decision silently, and at the pre-import position. + const ancestor = new OsmElementLookup([ + osmNode(5, { lat: 47.6 }), osmNode(6), osmWay(1, [5, 6]) + ]); + + const nodeConflict = { + number: 1, + kind: 'deleted-in-b' as const, + type: 'node' as const, + elementId: 5, + a: osmNode(5, { lat: 47.7, version: 2 }), + b: osmNode(5, { lat: 47.6 }), + base: osmNode(5, { lat: 47.6 }), + geometryConflict: false, + tagConflicts: [], + mergedTags: { } + }; + + const wayConflict = { + number: 2, + kind: 'deleted-in-b' as const, + type: 'way' as const, + elementId: 1, + a: osmWay(1, [5, 6], { version: 2 }), + b: osmWay(1, [5, 6]), + base: osmWay(1, [5, 6]), + geometryConflict: false, + tagConflicts: [], + mergedTags: { } + }; + + const output = applyResolutions( + new OsmElementLookup(), + [nodeConflict, wayConflict], + [ + { resolved: true, chosenSide: 'delete', tags: { } }, + { resolved: true, chosenSide: 'a', tags: { } } + ], + ancestor + ); + + expect(output.has('node', 5)).toBe(false); + + // The contradiction is reported rather than silently healed: + expect(findDanglingReferences(output)).toContain('way 1 references node 5'); + }); +}); + +describe('findDanglingReferences', () => { + it('names a way whose node the merge does not contain', () => { + const elements = new OsmElementLookup([osmNode(1), osmWay(10, [1, 2])]); + + expect(findDanglingReferences(elements)).toEqual(['way 10 references node 2']); + }); + + it('names a relation whose member the merge does not contain', () => { + const elements = new OsmElementLookup([ + osmRelation(5, [{ type: 'way', ref: 99, role: 'outer' }]) + ]); + + expect(findDanglingReferences(elements)) + .toEqual(['relation 5 references way 99']); + }); + + it('reports nothing when every reference resolves', () => { + const elements = new OsmElementLookup([ + osmNode(1), + osmNode(2), + osmWay(10, [1, 2]), + osmRelation(5, [{ type: 'way', ref: 10, role: 'outer' }]) + ]); + + expect(findDanglingReferences(elements)).toEqual([]); + }); +}); diff --git a/test/unit/services/merge-resolutions.test.ts b/test/unit/services/merge-resolutions.test.ts new file mode 100644 index 00000000..d1ea74ee --- /dev/null +++ b/test/unit/services/merge-resolutions.test.ts @@ -0,0 +1,155 @@ +import { describe, expect, it } from 'vitest'; + +import { applyResolutions } from '~/services/merge'; +import { OsmElementLookup } from '~/util/osm'; + +import { osmNode } from '../../mocks/osm-elements'; + +import type { ConflictResolution } from '~/services/merge'; +import type { OsmNode } from '~/types/osm'; + +describe('applyResolutions', () => { + function conflictFixture() { + return { + number: 1, + kind: 'both-modified' as const, + type: 'node' as const, + elementId: 1, + a: osmNode(1, { lat: 10, lon: 10, version: 2, tags: { highway: 'footway', surface: 'asphalt' } }), + b: osmNode(1, { lat: 20, lon: 20, version: 2, tags: { highway: 'footway', surface: 'concrete' } }), + geometryConflict: true, + tagConflicts: ['surface'], + mergedTags: { highway: 'footway', surface: 'asphalt' } + }; + } + + it('puts the chosen side back into the merged set', () => { + const conflict = conflictFixture(); + const resolution: ConflictResolution = { + resolved: true, + chosenSide: 'b', + tags: { highway: 'footway', surface: 'concrete' } + }; + + const output = applyResolutions(new OsmElementLookup(), [conflict], [resolution]); + + expect((output.get('node', 1) as OsmNode).lat).toBe(20); + }); + + it('keeps tags neither side disputed', () => { + // A resolution that only picks a geometry must not strip highway=footway + // off the element. + const conflict = conflictFixture(); + const resolution: ConflictResolution = { + resolved: true, + chosenSide: 'a', + tags: { ...conflict.mergedTags } + }; + + const output = applyResolutions(new OsmElementLookup(), [conflict], [resolution]); + + expect(output.get('node', 1)!.tags).toEqual({ + highway: 'footway', + surface: 'asphalt' + }); + }); + + it('leaves the element out when the user honors a deletion', () => { + const conflict = { ...conflictFixture(), kind: 'deleted-in-a' as const }; + const resolution: ConflictResolution = { + resolved: true, + chosenSide: 'delete', + tags: { } + }; + + const output = applyResolutions( + new OsmElementLookup([osmNode(1, { lat: 0, lon: 0, version: 1 })]), + [conflict], + [resolution] + ); + + expect(output.has('node', 1)).toBe(false); + }); + + it('refuses to build a result from an unresolved conflict', () => { + const resolution: ConflictResolution = { + resolved: false, + chosenSide: null, + tags: { } + }; + + expect(() => applyResolutions( + new OsmElementLookup(), + [conflictFixture()], + [resolution] + )).toThrow(/has not been resolved/); + }); + + it('does not disturb the elements it was given', () => { + const elements = new OsmElementLookup([osmNode(2, { lat: 1, lon: 1, version: 1, tags: { a: 'b' } })]); + const conflict = conflictFixture(); + + applyResolutions(elements, [conflict], [{ + resolved: true, + chosenSide: 'a', + tags: { surface: 'asphalt' } + }]); + + expect(elements.has('node', 1)).toBe(false); + expect(elements.get('node', 2)!.tags).toEqual({ a: 'b' }); + }); +}); + +describe('applyResolutions geometry handling', () => { + it('keeps a one-sided geometry edit when only the tags are disputed', () => { + // checkGeometryConflict already settled the shape: only B moved it. Taking + // the chosen side's element wholesale would silently revert that move, + // and the UI shows no geometry control when geometryConflict is false. + const conflict = { + number: 1, + kind: 'both-modified' as const, + type: 'node' as const, + elementId: 1, + a: osmNode(1, { lat: 1, lon: 2, version: 2, tags: { surface: 'asphalt' } }), + b: osmNode(1, { lat: 9.9, lon: 8.8, version: 2, tags: { surface: 'concrete' } }), + base: osmNode(1, { lat: 1, lon: 2, tags: { surface: 'gravel' } }), + geometryConflict: false, + tagConflicts: ['surface'], + mergedTags: { surface: 'asphalt' } + }; + + const output = applyResolutions(new OsmElementLookup(), [conflict], [{ + resolved: true, + chosenSide: 'a', + tags: { surface: 'concrete' } + }]); + + const node = output.get('node', 1) as OsmNode; + + expect([node.lat, node.lon]).toEqual([9.9, 8.8]); + expect(node.tags).toEqual({ surface: 'concrete' }); + }); + + it('takes the chosen side outright when the geometry really is disputed', () => { + const conflict = { + number: 1, + kind: 'both-modified' as const, + type: 'node' as const, + elementId: 1, + a: osmNode(1, { lat: 10, lon: 10, version: 2 }), + b: osmNode(1, { lat: 20, lon: 20, version: 2 }), + base: osmNode(1, { lat: 0, lon: 0 }), + geometryConflict: true, + tagConflicts: [], + mergedTags: { } + }; + + const output = applyResolutions(new OsmElementLookup(), [conflict], [{ + resolved: true, + chosenSide: 'b', + tags: { } + }]); + + expect((output.get('node', 1) as OsmNode).lat).toBe(20); + }); +}); diff --git a/test/unit/services/merge-targets.test.ts b/test/unit/services/merge-targets.test.ts new file mode 100644 index 00000000..1fea403c --- /dev/null +++ b/test/unit/services/merge-targets.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { ThreeWayWorkspaceMerger } from '~/services/merge'; + +import { workspace } from '../../mocks/merge'; + +import type { OsmApiClient } from '~/services/osm'; +import type { WorkspacesClient } from '~/services/workspaces'; +import type { Workspace } from '~/types/workspaces'; + +describe('ThreeWayWorkspaceMerger.getMergeTargets', () => { + function targetsFor(current: Workspace, candidates: Workspace[]) { + const workspacesClient = { + getMyWorkspaces: vi.fn(async () => candidates) + } as unknown as WorkspacesClient; + + return new ThreeWayWorkspaceMerger( + workspacesClient, + { } as unknown as OsmApiClient, + current + ).getMergeTargets(); + } + + it('offers workspaces sharing the dataset and project group', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, title: 'Sibling' }) + ]); + + expect(targets.map(t => t.id)).toEqual([2]); + }); + + it('never offers the workspace itself', async () => { + const targets = await targetsFor(workspace(), [workspace({ id: 1 })]); + + expect(targets).toEqual([]); + }); + + it('offers nothing when the workspace has no dataset to share', async () => { + // Two workspaces that both lack a dataset ID are unrelated, not matching. + const blank = workspace({ tdeiRecordId: undefined }); + const targets = await targetsFor(blank, [ + workspace({ id: 2, tdeiRecordId: undefined }) + ]); + + expect(targets).toEqual([]); + }); + + // Imports run as a background job, so a workspace can be listed while it is + // still filling. Merging one mid-import compares a partial dataset and + // reports it as an import mismatch, which blames the imports rather than + // saying one has not finished. + it('excludes a workspace whose import is still running', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, importStatus: 'in-progress' }) + ]); + + expect(targets).toEqual([]); + }); + + it('excludes a workspace whose import failed', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, importStatus: 'failed' }) + ]); + + expect(targets).toEqual([]); + }); + + // A workspace that never had a TDEI import reports `NA`, and one built + // before the field existed reports nothing at all. Neither is in flight. + it('still offers a workspace with a settled or absent import status', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, importStatus: 'completed' }), + workspace({ id: 3, importStatus: 'NA' }), + workspace({ id: 4, importStatus: undefined }) + ]); + + expect(targets.map(t => t.id).sort()).toEqual([2, 3, 4]); + }); + + // `MergeResult.type` is taken from workspace A, so merging across types + // would produce a workspace typed after A holding B's data. + it('excludes a workspace of a different type', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, type: 'pathways' }) + ]); + + expect(targets).toEqual([]); + }); + + it('excludes a workspace in another project group', async () => { + const targets = await targetsFor(workspace(), [ + workspace({ id: 2, tdeiProjectGroupId: '99999999-9999-4999-8999-999999999999' }) + ]); + + expect(targets).toEqual([]); + }); +}); diff --git a/test/unit/services/merge-three-way.test.ts b/test/unit/services/merge-three-way.test.ts new file mode 100644 index 00000000..08218c6d --- /dev/null +++ b/test/unit/services/merge-three-way.test.ts @@ -0,0 +1,311 @@ +import { describe, expect, it, vi } from 'vitest'; + +import { ThreeWayWorkspaceMerger } from '~/services/merge'; + +import { PROJECT_GROUP_ID, runMerge, workspace } from '../../mocks/merge'; +import { osmNode, osmWay } from '../../mocks/osm-elements'; + +import type { OsmApiClient } from '~/services/osm'; +import type { WorkspacesClient } from '~/services/workspaces'; +import type { OsmElement, OsmNode, OsmWay } from '~/types/osm'; +import type { OsmElementLookup } from '~/util/osm'; + +function ids(elements: OsmElementLookup): string[] { + return [...elements].map(element => `${element.type}/${element.id}`).sort(); +} + +describe('threeWayMerge element selection', () => { + it('takes the edit when only one side modified an element', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 }), osmNode(2, { lat: 0, lon: 0, version: 1 })], + a: [osmNode(1, { lat: 47.6, lon: 0, version: 2 }), osmNode(2, { lat: 0, lon: 0, version: 1 })], + b: [osmNode(1, { lat: 0, lon: 0, version: 1 }), osmNode(2, { lat: 47.7, lon: 0, version: 2 })] + }; + + const { result } = await runMerge(scenario); + + expect(result.conflicts).toHaveLength(0); + expect((result.elements.get('node', 1) as OsmNode).lat).toBe(47.6); + expect((result.elements.get('node', 2) as OsmNode).lat).toBe(47.7); + }); + + it('carries through elements created after the import in either workspace', async () => { + const scenario = { + ancestor: [osmNode(999)], // deleted in both, so it drops out + a: [osmNode(10, { lat: 1, lon: 1, version: 1 })], + b: [osmNode(20, { lat: 2, lon: 2, version: 1 })] + }; + + const { result } = await runMerge(scenario); + + expect(ids(result.elements)).toEqual(['node/10', 'node/20']); + }); + + it('drops an element both sides deleted', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [], + b: [] + }; + + const { result } = await runMerge(scenario); + + expect(ids(result.elements)).toEqual([]); + expect(result.conflicts).toHaveLength(0); + }); + + it('honors a deletion the other side left untouched', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [], + b: [osmNode(1, { lat: 0, lon: 0, version: 1 })] + }; + + const { result } = await runMerge(scenario); + + expect(ids(result.elements)).toEqual([]); + expect(result.conflicts).toHaveLength(0); + }); + + it('auto-merges tags both sides changed in non-overlapping ways', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1, tags: { highway: 'footway' } })], + a: [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { highway: 'footway', surface: 'asphalt' } })], + b: [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { highway: 'footway', lit: 'yes' } })] + }; + + const { result } = await runMerge(scenario); + + expect(result.conflicts).toHaveLength(0); + expect(result.elements.get('node', 1)!.tags).toEqual({ + highway: 'footway', + surface: 'asphalt', + lit: 'yes' + }); + }); +}); + +describe('threeWayMerge independently created elements', () => { + it('keeps both when each workspace created a different element under the same ID', async () => { + // Each workspace has its own ID sequence, so this collision is expected + // and means two distinct features, not one disputed one. + const scenario = { + ancestor: [osmNode(999)], // deleted in both, so it drops out + a: [osmNode(5, { lat: 47.6, lon: -122.3, version: 1 })], + b: [osmNode(5, { lat: 47.9, lon: -122.9, version: 1 })] + }; + + const { result } = await runMerge(scenario); + + expect([...result.elements]).toHaveLength(2); + expect(result.conflicts).toHaveLength(0); + + const positions = [...result.elements].map(e => (e as OsmNode).lat).sort(); + expect(positions).toEqual([47.6, 47.9]); + }); + + it('repoints a way in the second workspace at the re-identified node', async () => { + const scenario = { + ancestor: [osmNode(999)], // deleted in both, so it drops out + a: [osmNode(5, { lat: 47.6, lon: -122.3, version: 1 })], + b: [osmNode(5, { lat: 47.9, lon: -122.9, version: 1 }), osmWay(8, [5], { version: 1 })] + }; + + const { result } = await runMerge(scenario); + + const movedWay = [...result.elements].find(e => e.type === 'way') as OsmWay; + const reIdentified = [...result.elements] + .find(e => e.type === 'node' && (e as OsmNode).lat === 47.9)!; + + expect(movedWay.nodes).toEqual([reIdentified.id]); + expect(reIdentified.id).not.toBe(5); + }); +}); + +describe('threeWayMerge conflicts', () => { + it('reports a conflict when both sides moved the same node differently', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [osmNode(1, { lat: 10, lon: 10, version: 2 })], + b: [osmNode(1, { lat: 20, lon: 20, version: 2 })] + }; + + const { result } = await runMerge(scenario); + + expect(result.conflicts).toHaveLength(1); + expect(result.conflicts[0]!.kind).toBe('both-modified'); + expect(result.conflicts[0]!.geometryConflict).toBe(true); + }); + + it('leaves a conflicted element out of the merged set so neither side is lost', async () => { + // Both sides carry the same ID. Storing them together keeps only the last + // one written, and the user's choice then has nothing to apply to. + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [osmNode(1, { lat: 10, lon: 10, version: 2 })], + b: [osmNode(1, { lat: 20, lon: 20, version: 2 })] + }; + + const { result } = await runMerge(scenario); + + expect(result.elements.has('node', 1)).toBe(false); + expect(result.conflicts[0]!.a).not.toBe(result.conflicts[0]!.b); + expect((result.conflicts[0]!.a as OsmNode).lat).toBe(10); + expect((result.conflicts[0]!.b as OsmNode).lat).toBe(20); + }); + + it('shows each side its own tag values rather than a merged value', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1, tags: { surface: 'gravel' } })], + a: [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { surface: 'asphalt' } })], + b: [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { surface: 'concrete' } })] + }; + + const { result } = await runMerge(scenario); + const conflict = result.conflicts[0]!; + + expect(conflict.a.tags!.surface).toBe('asphalt'); + expect(conflict.b.tags!.surface).toBe('concrete'); + expect(conflict.tagConflicts).toEqual(['surface']); + }); + + it('classifies a delete on one side against an edit on the other', async () => { + const deletedInA = await runMerge({ + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [], + b: [osmNode(1, { lat: 5, lon: 5, version: 2 })] + }); + + expect(deletedInA.result.conflicts[0]!.kind).toBe('deleted-in-a'); + + const deletedInB = await runMerge({ + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 })], + a: [osmNode(1, { lat: 5, lon: 5, version: 2 })], + b: [] + }); + + expect(deletedInB.result.conflicts[0]!.kind).toBe('deleted-in-b'); + }); + + it('numbers conflicts from one', async () => { + const scenario = { + ancestor: [osmNode(1, { lat: 0, lon: 0, version: 1 }), osmNode(2, { lat: 0, lon: 0, version: 1 })], + a: [osmNode(1, { lat: 10, lon: 0, version: 2 }), osmNode(2, { lat: 10, lon: 0, version: 2 })], + b: [osmNode(1, { lat: 20, lon: 0, version: 2 }), osmNode(2, { lat: 20, lon: 0, version: 2 })] + }; + + const { result } = await runMerge(scenario); + + expect(result.conflicts.map(c => c.number)).toEqual([1, 2]); + }); + + it('does not modify the elements handed to it', async () => { + // The inputs belong to the caller and the ancestor is read repeatedly, so + // mutating either corrupts every comparison that follows. + const ancestor = [osmNode(1, { lat: 0, lon: 0, version: 1, tags: { surface: 'gravel' } })]; + const a = [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { surface: 'asphalt' } })]; + const b = [osmNode(1, { lat: 0, lon: 0, version: 2, tags: { surface: 'concrete' } })]; + + const snapshot = JSON.stringify({ ancestor, a, b }); + + await runMerge({ ancestor, a, b }); + + expect(JSON.stringify({ ancestor, a, b })).toBe(snapshot); + }); +}); + +describe('ThreeWayWorkspaceMerger ancestor lookup', () => { + it('reads the earliest changeset rather than assuming changeset 1', async () => { + const { osmClient } = await runMerge({ ancestor: [osmNode(999)], a: [], b: [] }); + + expect(osmClient.listChangesets).toHaveBeenCalled(); + expect(osmClient.getOsmChange).toHaveBeenCalledWith(1, 4); + }); + + it('fails with a clear message when the workspace has no history', async () => { + const osmClient = { + getWorkspaceData: vi.fn(async () => []), + listChangesets: vi.fn(async () => []) + }; + + const merger = new ThreeWayWorkspaceMerger( + { } as unknown as WorkspacesClient, + osmClient as unknown as OsmApiClient, + workspace() + ); + + await expect(merger.merge(2, PROJECT_GROUP_ID, 'Merged')) + .rejects.toThrow(/no common ancestor/); + }); +}); + +describe('a source workspace whose import has not finished', () => { + // The target is filtered out of the picker, but the workspace the user + // started from is not, and it would otherwise fail later as a bogus import + // mismatch rather than saying the import is still running. + it.each(['in-progress', 'failed'] as const)( + 'refuses to merge from a workspace whose import is %s', + async (importStatus) => { + const merger = new ThreeWayWorkspaceMerger( + { } as unknown as WorkspacesClient, + { } as unknown as OsmApiClient, + workspace({ importStatus }) + ); + + await expect(merger.merge(2, PROJECT_GROUP_ID, 'Merged')) + .rejects.toThrow(/does not hold all of its data yet/); + } + ); +}); + +describe('workspaces whose imports do not line up', () => { + // Each workspace is its own database with its own id sequences, so element 5 + // in one is unrelated to element 5 in the other unless the two imports + // assigned matching ids. The ancestor is read from workspace A, and B is + // matched against it by id, so a mismatch makes every B element read as + // untouched and hands the merge to A. Refusing beats losing B's work. + async function mergeWith(ancestorA: OsmElement[], ancestorB: OsmElement[]) { + const osmClient = { + // Each workspace still holds what its own import created. A workspace + // reporting no data at all is a different failure, guarded separately. + getWorkspaceData: vi.fn(async (workspaceId: number) => + workspaceId === 1 ? ancestorA : ancestorB + ), + listChangesets: vi.fn(async () => [ + { id: 4, created_at: new Date('2026-01-01T00:00:00Z') } + ]), + getOsmChange: vi.fn(async (workspaceId: number) => ({ + create: workspaceId === 1 ? ancestorA : ancestorB, + modify: [], + delete: [] + })) + }; + + const merger = new ThreeWayWorkspaceMerger( + { } as unknown as WorkspacesClient, + osmClient as unknown as OsmApiClient, + workspace() + ); + + return await merger.merge(2, PROJECT_GROUP_ID, 'Merged'); + } + + it('refuses when the two imports created different numbers of elements', async () => { + await expect(mergeWith([osmNode(1)], [osmNode(1), osmNode(2)])) + .rejects.toThrow(/not imported the same way/); + }); + + it('refuses when an id is missing from the other import', async () => { + await expect(mergeWith([osmNode(1)], [osmNode(2)])) + .rejects.toThrow(/exists in one import and not the other/); + }); + + it('refuses when the same id sits somewhere else in the other import', async () => { + await expect(mergeWith([osmNode(1)], [osmNode(1, { lat: 47.9 })])) + .rejects.toThrow(/different place/); + }); + + it('merges when the two imports agree', async () => { + await expect(mergeWith([osmNode(1)], [osmNode(1)])).resolves.toBeDefined(); + }); +}); diff --git a/test/unit/services/merge-validation.test.ts b/test/unit/services/merge-validation.test.ts new file mode 100644 index 00000000..f1df84d8 --- /dev/null +++ b/test/unit/services/merge-validation.test.ts @@ -0,0 +1,173 @@ +import { describe, expect, it } from 'vitest'; + +import { findImportMismatch, findUnusableTags } from '~/services/merge'; +import { OsmElementLookup } from '~/util/osm'; + +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +import type { OsmElement } from '~/types/osm'; + +describe('findUnusableTags', () => { + function lookupWith(tags: Record) { + return new OsmElementLookup([osmNode(1, { tags })]); + } + + it('passes tags the API will accept', () => { + expect(findUnusableTags(lookupWith({ highway: 'crossing' }))).toEqual([]); + }); + + it('reports a value over the API limit', () => { + const problems = findUnusableTags(lookupWith({ note: 'x'.repeat(256) })); + + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/longer than 255/); + expect(problems[0]).toContain('node 1'); + }); + + it('accepts a value exactly at the limit', () => { + expect(findUnusableTags(lookupWith({ note: 'x'.repeat(255) }))).toEqual([]); + }); + + it('reports a key over the API limit', () => { + expect(findUnusableTags(lookupWith({ ['k'.repeat(256)]: 'v' }))[0]) + .toMatch(/tag name/); + }); + + // The server counts characters. A surrogate pair is one character to it and + // two units to `String.length`, so counting units would reject a value the + // server accepts. + it('counts code points, not UTF-16 units', () => { + expect(findUnusableTags(lookupWith({ note: '😀'.repeat(255) }))).toEqual([]); + expect(findUnusableTags(lookupWith({ note: '😀'.repeat(256) }))).toHaveLength(1); + }); + + // XML 1.0 cannot carry these even escaped, so the serializer emits a + // document the server cannot parse, and its error names no tag. + it('reports a control character a tag value cannot carry', () => { + const problems = findUnusableTags(lookupWith({ note: 'before\u0001after' })); + + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/XML cannot carry/); + }); + + // A lone surrogate serializes, but fetch's UTF-8 encoder rewrites it to + // U+FFFD, so the tag would upload corrupted rather than fail. A valid pair + // (any astral character) must still be accepted. + it('reports a lone surrogate but accepts a valid pair', () => { + expect(findUnusableTags(lookupWith({ note: 'ab\uD800cd' }))).toHaveLength(1); + expect(findUnusableTags(lookupWith({ note: 'ab\uDC00cd' }))).toHaveLength(1); + expect(findUnusableTags(lookupWith({ note: 'ok \u{1F600} fine' }))).toEqual([]); + }); + + it('allows tab, newline and carriage return', () => { + expect(findUnusableTags(lookupWith({ note: 'a\tb\nc\rd' }))).toEqual([]); + }); + + it('leaves an untagged element alone', () => { + expect(findUnusableTags(new OsmElementLookup([osmNode(1)]))).toEqual([]); + }); + + // A member role is arbitrary data that reaches the serializer the same way a + // tag value does, and the API caps it the same way. + it('reports a relation member role over the API limit', () => { + const problems = findUnusableTags(new OsmElementLookup([ + osmRelation(3, [{ type: 'way', ref: 10, role: 'r'.repeat(256) }]) + ])); + + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/relation 3: the role of way 10 is longer than 255/); + }); + + it('reports a relation member role XML cannot carry', () => { + const problems = findUnusableTags(new OsmElementLookup([ + osmRelation(3, [{ type: 'node', ref: 4, role: 'in\u0001ner' }]) + ])); + + expect(problems).toHaveLength(1); + expect(problems[0]).toMatch(/the role of node 4 contains a character XML cannot carry/); + }); + + it('accepts ordinary and empty relation member roles', () => { + expect(findUnusableTags(new OsmElementLookup([ + osmRelation(3, [ + { type: 'way', ref: 10, role: 'outer' }, + { type: 'way', ref: 11, role: '' } + ]) + ]))).toEqual([]); + }); +}); + +describe('findImportMismatch', () => { + function lookup(elements: OsmElement[]) { + return new OsmElementLookup(elements); + } + + it('accepts two imports that created the same elements', () => { + expect(findImportMismatch( + lookup([osmNode(1), osmWay(2, [1])]), + lookup([osmNode(1), osmWay(2, [1])]) + )).toBeUndefined(); + }); + + // Two runs of the same import assign their own changeset ids, user ids and + // timestamps, so comparing that metadata would refuse every legitimate merge. + it('ignores changeset, user and timestamp differences', () => { + expect(findImportMismatch( + lookup([osmNode(1, { changeset: 1, uid: 7, timestamp: new Date('2026-01-01T00:00:00Z') })]), + lookup([osmNode(1, { changeset: 99, uid: 42, timestamp: new Date('2026-06-30T12:00:00Z') })]) + )).toBeUndefined(); + }); + + it('reports a different number of elements', () => { + expect(findImportMismatch(lookup([osmNode(1)]), lookup([osmNode(1), osmNode(2)]))) + .toMatch(/created 1 and 2 elements/); + }); + + it('reports an id present in only one import', () => { + expect(findImportMismatch(lookup([osmNode(1)]), lookup([osmNode(2)]))) + .toMatch(/node 1 exists in one import and not the other/); + }); + + it('reports a node that moved between imports', () => { + expect(findImportMismatch(lookup([osmNode(1)]), lookup([osmNode(1, { lat: 47.9 })]))) + .toMatch(/node 1 is in a different place/); + }); + + it('reports a way whose node list differs', () => { + expect(findImportMismatch(lookup([osmWay(2, [1, 3])]), lookup([osmWay(2, [1, 4])]))) + .toMatch(/way 2 is in a different place/); + }); + + // Tags are compared as well as geometry. An importer that leaves elements in + // place but tags them differently has still broken the ID matching the merge + // reads the ancestor by. + it('reports an element whose tag value differs between imports', () => { + expect(findImportMismatch( + lookup([osmNode(1, { tags: { highway: 'crossing' } })]), + lookup([osmNode(1, { tags: { highway: 'footway' } })]) + )).toMatch(/node 1 carries different tags/); + }); + + it('reports an element whose tag key differs between imports', () => { + expect(findImportMismatch( + lookup([osmNode(1, { tags: { highway: 'crossing' } })]), + lookup([osmNode(1, { tags: { footway: 'crossing' } })]) + )).toMatch(/node 1 carries different tags/); + }); + + it('reports a tag only one import applied', () => { + expect(findImportMismatch( + lookup([osmNode(1, { tags: { highway: 'crossing' } })]), + lookup([osmNode(1, { tags: { highway: 'crossing', surface: 'asphalt' } })]) + )).toMatch(/node 1 carries different tags/); + }); + + // The API omits `tags` entirely for an untagged element, which is every + // vertex node in an import, so absent and empty have to read as the same. + it('treats absent tags and an empty tag set as the same', () => { + expect(findImportMismatch( + lookup([osmNode(1)]), + lookup([osmNode(1, { tags: { } })]) + )).toBeUndefined(); + }); +}); diff --git a/test/unit/services/osm.test.ts b/test/unit/services/osm.test.ts index 9d12ab25..66fa7001 100644 --- a/test/unit/services/osm.test.ts +++ b/test/unit/services/osm.test.ts @@ -1,6 +1,7 @@ import { http, HttpResponse } from 'msw'; -import { describe, expect, it } from 'vitest'; -import { OsmApiClient } from '~/services/osm'; +import { describe, expect, it, vi } from 'vitest'; +import { BaseHttpClientError } from '~/services/http'; +import { OsmApiClient, OsmApiClientError } from '~/services/osm'; import { server } from '../../mocks/server'; import type { TdeiClient } from '~/services/tdei'; @@ -41,7 +42,7 @@ function stubDownload(xml: string = OSM_CHANGE_XML) { describe('OsmApiClient.getOsmChange', () => { // Regression: parseOsmChangeXml is async (it resolves a Promise once the // streaming sax parse completes). Using it without `await` left a Promise - // here, which JSON.stringify's to "{}" — the bug that made the exported + // here, which JSON.stringify's to "{}", the bug that made the exported // changesets/{id}.json empty even when the .osc had content. it('parses the downloaded osmChange XML into a populated object', async () => { stubDownload(); @@ -116,7 +117,7 @@ describe('OsmApiClient.getChangesetComments', () => { describe('OsmApiClient comment posting', () => { // Regression: the comment text must be sent in the query string, not a - // multipart body — the OSM API only reads params[:text] from the query, and + // multipart body. The OSM API only reads params[:text] from the query, and // this backend 401s a write whose text it can't find. it('posts a changeset comment with the message text as a query parameter', async () => { const urls: string[] = []; @@ -160,3 +161,90 @@ describe('OsmApiClient comment posting', () => { expect(urls.some(u => u.includes('text=thanks+for+flagging'))).toBe(true); }); }); + +describe('OsmApiClient.getWorkspaceData', () => { + // The API reports no bounding box for a workspace that has never held a + // node, and the archive export reads such a workspace legitimately, so this + // must not throw. + it('reads an empty workspace as no elements rather than failing', async () => { + const client = makeClient(); + + vi.spyOn(client, 'getExportBbox').mockResolvedValue(undefined); + + await expect(client.getWorkspaceData(1)).resolves.toEqual([]); + }); + + it('exports an empty workspace as an empty document rather than failing', async () => { + const client = makeClient(); + + vi.spyOn(client, 'getExportBbox').mockResolvedValue(undefined); + + const blob = await client.exportWorkspaceXml(1); + + expect(await blob.text()).toContain(' { + const client = makeClient(); + let requested = false; + + server.use( + http.get(`${OSM_API_BASE}map.json`, () => { + requested = true; + + return HttpResponse.json({ elements: [] }); + }) + ); + + vi.spyOn(client, 'getExportBbox').mockResolvedValue(undefined); + + await client.getWorkspaceData(1); + + expect(requested).toBe(false); + }); +}); + +describe('OsmApiClient errors', () => { + it('reports a failed OSM request as an OSM API error', async () => { + server.use( + http.delete(`${OSM_API_BASE}workspaces/7`, () => new HttpResponse(null, { status: 500 })) + ); + + const error = await makeClient().deleteWorkspace(7).catch((e: unknown) => e); + + expect(error).toBeInstanceOf(OsmApiClientError); + expect((error as OsmApiClientError).response.status).toBe(500); + }); + + it('passes a failed session refresh through rather than as the OSM server\'s answer', async () => { + // The refresh fails before the OSM request is sent, so nothing here came + // from the OSM server. + const refreshFailure = new BaseHttpClientError(new Response(null, { status: 503 })); + const client = new OsmApiClient(OSM_WEB_BASE, OSM_API_BASE, { + sendProtectedRequest: async () => { + throw refreshFailure; + } + } as unknown as TdeiClient); + + await expect(client.deleteWorkspace(7)).rejects.toBe(refreshFailure); + }); + + it('passes through a refresh that fails after the OSM server answered 401', async () => { + // The OSM 401 has already been recorded by then, so only telling the two + // errors apart, not merely noticing that a request was sent, keeps the + // refresh failure from being reported as the OSM server's answer. + server.use( + http.delete(`${OSM_API_BASE}workspaces/7`, () => new HttpResponse(null, { status: 401 })) + ); + + const refreshFailure = new BaseHttpClientError(new Response(null, { status: 503 })); + const client = new OsmApiClient(OSM_WEB_BASE, OSM_API_BASE, { + sendProtectedRequest: async (send: (accessToken: string) => Promise) => { + await send('expired-token').catch(() => undefined); + throw refreshFailure; + } + } as unknown as TdeiClient); + + await expect(client.deleteWorkspace(7)).rejects.toBe(refreshFailure); + }); +}); diff --git a/test/unit/services/workspaces-lifecycle.test.ts b/test/unit/services/workspaces-lifecycle.test.ts new file mode 100644 index 00000000..60474a8c --- /dev/null +++ b/test/unit/services/workspaces-lifecycle.test.ts @@ -0,0 +1,260 @@ +import { http, HttpResponse } from 'msw'; +import { describe, expect, it, vi } from 'vitest'; +import { OsmApiClientError } from '~/services/osm'; +import { WorkspacesClient, WorkspacesClientError } from '~/services/workspaces'; +import { server } from '../../mocks/server'; +import { TEST_API_BASE } from '../../mocks/fixtures'; + +import type { OsmApiClient } from '~/services/osm'; +import type { TdeiClient } from '~/services/tdei'; + +// Creating, provisioning and deleting a workspace. The tdei stub runs +// protected requests with a test token, and MSW intercepts at fetch, so the +// real HTTP client runs end to end. +const tdeiClient = { + sendProtectedRequest: async ( + request: (accessToken: string) => Promise + ) => await request('test-access-token'), + auth: { complete: false, accessToken: '' } +} as unknown as TdeiClient; +const osmClient = {} as unknown as OsmApiClient; + +describe('WorkspacesClient.createWorkspace', () => { + const workspace = { + title: 'New workspace', + type: 'osw' as const, + tdeiProjectGroupId: '11111111-1111-4111-8111-111111111111' + }; + + function makeClient() { + return new WorkspacesClient(TEST_API_BASE, TEST_API_BASE, tdeiClient, osmClient); + } + + it('returns the workspaceId the API responds with', async () => { + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => HttpResponse.json({ workspaceId: 77 }, { status: 201 })) + ); + + await expect(makeClient().createWorkspace({ ...workspace })).resolves.toBe(77); + }); + + it.each([ + ['a missing id', { }], + ['a non-integer id', { workspaceId: 1.5 }], + ['a string id', { workspaceId: '77' }], + ['a null body', null] + ])('rejects %s rather than returning it', async (_label, body) => { + // A failed merge commit rolls back by deleting the returned ID, so an + // unchecked value here would aim a delete at whatever came back. + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => HttpResponse.json(body)) + ); + + await expect(makeClient().createWorkspace({ ...workspace })) + .rejects.toThrow(/valid integer workspace ID/); + }); +}); + +describe('WorkspacesClient.createBlankWorkspace', () => { + function makeBlankClient(osm: unknown) { + return new WorkspacesClient(TEST_API_BASE, TEST_API_BASE, tdeiClient, osm as OsmApiClient); + } + + function blankWorkspace(type: 'osw' | 'pathways') { + return { + title: 'Empty workspace', + type, + tdeiProjectGroupId: '11111111-1111-1111-1111-111111111111' + }; + } + + it.each(['osw', 'pathways'] as const)( + 'provisions an empty OSM store for a blank %s workspace', + async (type) => { + const createOsmWorkspace = vi.fn().mockResolvedValue(undefined); + const client = makeBlankClient({ createWorkspace: createOsmWorkspace }); + + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => { + expect(createOsmWorkspace).not.toHaveBeenCalled(); + return HttpResponse.json({ workspaceId: 1909 }, { status: 201 }); + }) + ); + + await expect(client.createBlankWorkspace(blankWorkspace(type))).resolves.toBe(1909); + expect(createOsmWorkspace).toHaveBeenCalledOnce(); + expect(createOsmWorkspace).toHaveBeenCalledWith(1909); + } + ); + + it('does not provision OSM when workspace record creation fails', async () => { + const createOsmWorkspace = vi.fn().mockResolvedValue(undefined); + const client = makeBlankClient({ createWorkspace: createOsmWorkspace }); + + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => + new HttpResponse(null, { status: 500, statusText: 'Server Error' })) + ); + + await expect(client.createBlankWorkspace(blankWorkspace('pathways'))) + .rejects.toBeInstanceOf(WorkspacesClientError); + + expect(createOsmWorkspace).not.toHaveBeenCalled(); + }); + + it('deletes the workspace row when OSM provisioning fails', async () => { + // Left behind, the row sits on the dashboard with no OSM database behind + // it, and every retry adds another. + const deleteOsmWorkspace = vi.fn().mockResolvedValue(undefined); + const client = makeBlankClient({ + createWorkspace: vi.fn().mockRejectedValue(new Error('osm provisioning failed')), + deleteWorkspace: deleteOsmWorkspace + }); + + let deletedPath: string | undefined; + + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => + HttpResponse.json({ workspaceId: 1909 }, { status: 201 })), + http.delete(`${TEST_API_BASE}workspaces/1909`, ({ request }) => { + deletedPath = new URL(request.url).pathname; + return new HttpResponse(null, { status: 204 }); + }) + ); + + await expect(client.createBlankWorkspace(blankWorkspace('osw'))) + .rejects.toThrow('osm provisioning failed'); + + expect(deletedPath).toBe('/workspaces/1909'); + expect(deleteOsmWorkspace).toHaveBeenCalledWith(1909); + }); + + it('still deletes the row when the database delete fails', async () => { + // Provisioning may have failed before creating the database, and then + // there is nothing for the database delete to remove. + const client = makeBlankClient({ + createWorkspace: vi.fn().mockRejectedValue(new Error('osm provisioning failed')), + deleteWorkspace: vi.fn().mockRejectedValue(osmError(500)) + }); + + let rowDeleted = false; + + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => + HttpResponse.json({ workspaceId: 1909 }, { status: 201 })), + http.delete(`${TEST_API_BASE}workspaces/1909`, () => { + rowDeleted = true; + return new HttpResponse(null, { status: 204 }); + }) + ); + + await expect(client.createBlankWorkspace(blankWorkspace('osw'))) + .rejects.toThrow('osm provisioning failed'); + + expect(rowDeleted).toBe(true); + }); + + it('reports the provisioning failure even when the rollback also fails', async () => { + // The drop fails in a way that still sends the row delete, so both halves + // of the rollback fail here. + const client = makeBlankClient({ + createWorkspace: vi.fn().mockRejectedValue(new Error('osm provisioning failed')), + deleteWorkspace: vi.fn().mockRejectedValue(osmError(500)) + }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => undefined); + + server.use( + http.post(`${TEST_API_BASE}workspaces`, () => + HttpResponse.json({ workspaceId: 1909 }, { status: 201 })), + http.delete(`${TEST_API_BASE}workspaces/1909`, () => + new HttpResponse(null, { status: 500, statusText: 'Server Error' })) + ); + + // Restored in `finally`: a failed assertion would otherwise leave the + // warning silenced for every later test in this file. + try { + await expect(client.createBlankWorkspace(blankWorkspace('osw'))) + .rejects.toThrow('osm provisioning failed'); + + // Left unlogged, the unusable workspace it leaves behind would have no trace. + expect(warn).toHaveBeenCalledWith('Rollback of workspace 1909 failed.', expect.any(WorkspacesClientError)); + } + finally { + warn.mockRestore(); + } + }); +}); + +function osmError(status: number): OsmApiClientError { + return new OsmApiClientError(new Response(null, { status })); +} + +describe('WorkspacesClient.deleteWorkspace', () => { + function makeDeleteClient(osm: unknown) { + return new WorkspacesClient(TEST_API_BASE, TEST_API_BASE, tdeiClient, osm as OsmApiClient); + } + + function recordRowDelete(calls: string[]) { + server.use( + http.delete(`${TEST_API_BASE}workspaces/7`, () => { + calls.push('row'); + return new HttpResponse(null, { status: 204 }); + }) + ); + } + + it('drops the database before deleting the row', async () => { + // The drop resolves late, so a row delete sent alongside it rather than + // after it would be recorded first: + const calls: string[] = []; + const client = makeDeleteClient({ + deleteWorkspace: vi.fn(async () => { + await new Promise(resolve => setTimeout(resolve, 20)); + calls.push('database'); + }) + }); + + recordRowDelete(calls); + await client.deleteWorkspace(7); + + expect(calls).toEqual(['database', 'row']); + }); + + it.each([401, 403])('keeps the row when the database drop is refused with %i', async (status) => { + const calls: string[] = []; + const client = makeDeleteClient({ + deleteWorkspace: vi.fn().mockRejectedValue(osmError(status)) + }); + + recordRowDelete(calls); + + await expect(client.deleteWorkspace(7)).rejects.toBeInstanceOf(OsmApiClientError); + expect(calls).toEqual([]); + }); + + it('keeps the row when the database drop fails without a response', async () => { + const calls: string[] = []; + const client = makeDeleteClient({ + deleteWorkspace: vi.fn().mockRejectedValue(new TypeError('Failed to fetch')) + }); + + recordRowDelete(calls); + + await expect(client.deleteWorkspace(7)).rejects.toThrow('Failed to fetch'); + expect(calls).toEqual([]); + }); + + it('deletes the row when the workspace has no database to drop', async () => { + // The OSM API answers a drop for a database that does not exist with a + // server error. Stopping there would make the row impossible to delete. + const calls: string[] = []; + const client = makeDeleteClient({ + deleteWorkspace: vi.fn().mockRejectedValue(osmError(500)) + }); + + recordRowDelete(calls); + await client.deleteWorkspace(7); + + expect(calls).toEqual(['row']); + }); +}); diff --git a/test/unit/services/workspaces.test.ts b/test/unit/services/workspaces.test.ts index bfeade8a..c326551f 100644 --- a/test/unit/services/workspaces.test.ts +++ b/test/unit/services/workspaces.test.ts @@ -1,5 +1,5 @@ import { http, HttpResponse } from 'msw'; -import { describe, expect, it, vi } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { WorkspacesClient, WorkspacesClientError } from '~/services/workspaces'; import { server } from '../../mocks/server'; import { TEST_API_BASE } from '../../mocks/fixtures'; @@ -115,62 +115,40 @@ describe('WorkspacesClient.getWorkspaceBbox', () => { await expect(makeBboxClient().getWorkspaceBbox(1)).resolves.toBeUndefined(); }); -}); -describe('WorkspacesClient.createBlankWorkspace', () => { - it.each(['osw', 'pathways'] as const)( - 'provisions an empty OSM store for a blank %s workspace', - async (type) => { - const createOsmWorkspace = vi.fn().mockResolvedValue(undefined); - const client = new WorkspacesClient( - TEST_API_BASE, - TEST_API_BASE, - tdeiClient, - { createWorkspace: createOsmWorkspace } as unknown as OsmApiClient - ); - const workspace = { - title: 'Empty workspace', - type, - tdeiProjectGroupId: '11111111-1111-1111-1111-111111111111' - }; - - server.use( - http.post(`${TEST_API_BASE}workspaces`, () => { - expect(createOsmWorkspace).not.toHaveBeenCalled(); - return HttpResponse.json({ workspaceId: 1909 }, { status: 201 }); - }) - ); - - await expect(client.createBlankWorkspace(workspace)).resolves.toBe(1909); - expect(createOsmWorkspace).toHaveBeenCalledOnce(); - expect(createOsmWorkspace).toHaveBeenCalledWith(1909); - } - ); - - it('does not provision OSM when workspace record creation fails', async () => { - const createOsmWorkspace = vi.fn().mockResolvedValue(undefined); - const client = new WorkspacesClient( - TEST_API_BASE, - TEST_API_BASE, - tdeiClient, - { createWorkspace: createOsmWorkspace } as unknown as OsmApiClient + // The backend computes the box as a MIN/MAX over the workspace's nodes, and + // an aggregate over no rows returns a row of nulls rather than no row, so an + // empty workspace answers 200 with null coordinates rather than 204. Passing + // that on would put "null" into a bbox query string and NaN into a map fit. + it('returns undefined when the coordinates come back null', async () => { + server.use( + http.get(`${NEW_API_BASE}workspaces/1/bbox`, () => + HttpResponse.json({ min_lat: null, min_lon: null, max_lat: null, max_lon: null }) + ) ); - const workspace = { - title: 'Empty workspace', - type: 'pathways' as const, - tdeiProjectGroupId: '11111111-1111-1111-1111-111111111111' - }; + await expect(makeBboxClient().getWorkspaceBbox(1)).resolves.toBeUndefined(); + }); + + it('returns undefined when only some coordinates are missing', async () => { server.use( - http.post(`${TEST_API_BASE}workspaces`, () => { - return new HttpResponse(null, { status: 500, statusText: 'Server Error' }); - }) + http.get(`${NEW_API_BASE}workspaces/1/bbox`, () => + HttpResponse.json({ min_lat: 47.6, min_lon: null, max_lat: 47.62, max_lon: -122.32 }) + ) ); - await expect(client.createBlankWorkspace(workspace)).rejects.toBeInstanceOf( - WorkspacesClientError + await expect(makeBboxClient().getWorkspaceBbox(1)).resolves.toBeUndefined(); + }); + + it('keeps a box whose coordinates are legitimately zero', async () => { + // 0/0 is a real position, so it must not read as missing. + const bbox = { min_lat: 0, min_lon: 0, max_lat: 0, max_lon: 0 }; + + server.use( + http.get(`${NEW_API_BASE}workspaces/1/bbox`, () => HttpResponse.json(bbox)) ); - expect(createOsmWorkspace).not.toHaveBeenCalled(); + + await expect(makeBboxClient().getWorkspaceBbox(1)).resolves.toEqual(bbox); }); }); diff --git a/test/unit/util/collections.test.ts b/test/unit/util/collections.test.ts new file mode 100644 index 00000000..a310ed50 --- /dev/null +++ b/test/unit/util/collections.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest'; + +import { shallowArrayEqual, shallowMembersEqual } from '~/util/collections'; + +import type { OsmRelationRef } from '~/types/osm'; + +describe('shallowArrayEqual', () => { + it('compares element by element', () => { + expect(shallowArrayEqual([1, 2, 3], [1, 2, 3])).toBe(true); + expect(shallowArrayEqual([1, 2, 3], [1, 3, 2])).toBe(false); + }); + + it('treats a length difference as unequal', () => { + expect(shallowArrayEqual([1, 2], [1, 2, 3])).toBe(false); + }); + + it('treats two empty arrays as equal', () => { + expect(shallowArrayEqual([], [])).toBe(true); + }); +}); + +describe('shallowMembersEqual', () => { + const members: OsmRelationRef[] = [ + { type: 'way', ref: 1, role: 'outer' }, + { type: 'node', ref: 2, role: '' } + ]; + + it('compares type, ref and role of each member', () => { + expect(shallowMembersEqual(members, members.map(m => ({ ...m })))).toBe(true); + }); + + it('is order sensitive, because member order is part of a relation', () => { + expect(shallowMembersEqual(members, [...members].reverse())).toBe(false); + }); + + it('notices a changed role', () => { + expect(shallowMembersEqual(members, [ + { type: 'way', ref: 1, role: 'inner' }, + { type: 'node', ref: 2, role: '' } + ])).toBe(false); + }); + + it('notices a changed ref', () => { + expect(shallowMembersEqual(members, [ + { type: 'way', ref: 99, role: 'outer' }, + { type: 'node', ref: 2, role: '' } + ])).toBe(false); + }); + + it('notices a changed type', () => { + expect(shallowMembersEqual(members, [ + { type: 'relation', ref: 1, role: 'outer' }, + { type: 'node', ref: 2, role: '' } + ])).toBe(false); + }); + + it('treats two empty member lists as equal', () => { + expect(shallowMembersEqual([], [])).toBe(true); + }); +}); diff --git a/test/unit/util/map-style.test.ts b/test/unit/util/map-style.test.ts new file mode 100644 index 00000000..e5837b13 --- /dev/null +++ b/test/unit/util/map-style.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from 'vitest'; + +import { bingImageryStyle } from '~/util/map-style'; + +describe('bingImageryStyle', () => { + it('returns a separate style object per call', () => { + // MapLibre keeps the stylesheet it is handed and mutates it in place. Two + // components use this one, so a shared object lets the first map to load + // corrupt the second's sources. + const first = bingImageryStyle(); + const second = bingImageryStyle(); + + expect(second).toEqual(first); + expect(second).not.toBe(first); + expect(second.sources).not.toBe(first.sources); + expect(second.sources.bing).not.toBe(first.sources.bing); + expect(second.layers).not.toBe(first.layers); + }); +}); diff --git a/test/unit/util/osm-merge-elements.test.ts b/test/unit/util/osm-merge-elements.test.ts new file mode 100644 index 00000000..e6a8337f --- /dev/null +++ b/test/unit/util/osm-merge-elements.test.ts @@ -0,0 +1,214 @@ +import { describe, expect, it } from 'vitest'; + +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +import { + checkConflict, + checkGeometryConflict, + mergeNodes, + mergeOsmElements, + mergeRelations, + mergeWays +} from '~/util/osm-merge'; + +import type { OsmWay } from '~/types/osm'; + +describe('checkGeometryConflict', () => { + it('compares way node lists and relation member lists', () => { + expect(checkGeometryConflict(osmWay(1, [1, 2]), osmWay(1, [1, 3]))).toBe(true); + expect(checkGeometryConflict(osmWay(1, [1, 2]), osmWay(1, [1, 2]))).toBe(false); + + const members = [{ type: 'way' as const, ref: 1, role: 'outer' }]; + + expect(checkGeometryConflict( + osmRelation(1, members), + osmRelation(1, [...members]) + )).toBe(false); + + expect(checkGeometryConflict( + osmRelation(1, members), + osmRelation(1, [{ type: 'way', ref: 2, role: 'outer' }]) + )).toBe(true); + }); + + it('treats a base of a different element type as no common ancestor', () => { + // A way cannot account for where a node sits, so the difference between + // the two sides stays a conflict. + expect(checkGeometryConflict( + osmNode(1, { lat: 47.7 }), + osmNode(1, { lat: 47.8 }), + osmWay(1, [1, 2]) + )).toBe(true); + }); +}); + +describe('checkConflict', () => { + it('finds no conflict when only one side moved a node', () => { + const base = osmNode(1, { lat: 47.6 }); + + expect(checkConflict(osmNode(1, { lat: 47.7 }), osmNode(1, { lat: 47.6 }), base)).toBe(false); + expect(checkConflict(osmNode(1, { lat: 47.6 }), osmNode(1, { lat: 47.8 }), base)).toBe(false); + }); + + it('finds a conflict when both sides moved a node to different places', () => { + const base = osmNode(1, { lat: 47.6 }); + + expect(checkConflict( + osmNode(1, { lat: 47.7 }), + osmNode(1, { lat: 47.8 }), + base + )).toBe(true); + }); + + it('finds a conflict when the two sides are different element types', () => { + expect(checkConflict(osmNode(1), osmWay(1, [2, 3]))).toBe(true); + }); + + it('finds a conflict from tags alone when the geometry matches', () => { + const base = osmNode(1, { tags: { surface: 'gravel' } }); + + expect(checkConflict( + osmNode(1, { tags: { surface: 'asphalt' } }), + osmNode(1, { tags: { surface: 'concrete' } }), + base + )).toBe(true); + }); + + it('finds a conflict when one side clears every tag and the other edits them', () => { + const base = osmNode(1, { tags: { surface: 'gravel' } }); + + expect(checkConflict( + osmNode(1, { tags: { } }), + osmNode(1, { tags: { surface: 'concrete' } }), + base + )).toBe(true); + }); +}); + +describe('mergeNodes', () => { + it('takes the side that moved rather than averaging the two positions', () => { + // Averaging invents a location neither surveyor recorded, which for a + // crossing or kerb is a real positional error. + const base = osmNode(1, { lat: 47.6, lon: -122.3 }); + const merged = mergeNodes( + osmNode(1, { lat: 47.6, lon: -122.3 }), + osmNode(1, { lat: 47.8, lon: -122.5 }), + base + ); + + expect(merged.lat).toBe(47.8); + expect(merged.lon).toBe(-122.5); + }); + + it('keeps the shared position when neither side moved', () => { + const merged = mergeNodes(osmNode(1), osmNode(1)); + + expect([merged.lat, merged.lon]).toEqual([47.6, -122.3]); + }); + + it('keeps the A-side position when the base is a different element type', () => { + const merged = mergeNodes( + osmNode(1, { lat: 47.7 }), + osmNode(1, { lat: 47.8 }), + osmWay(1, [1, 2]) + ); + + expect([merged.lat, merged.lon]).toEqual([47.7, -122.3]); + }); +}); + +describe('mergeWays', () => { + it('takes the node list from the side that changed it', () => { + const base = osmWay(1, [1, 2, 3]); + const merged = mergeWays(osmWay(1, [1, 2, 3]), osmWay(1, [1, 4, 3]), base); + + expect(merged.nodes).toEqual([1, 4, 3]); + }); + + it('keeps the A-side node list when only A changed it', () => { + const base = osmWay(1, [1, 2, 3]); + const merged = mergeWays(osmWay(1, [1, 9, 3]), osmWay(1, [1, 2, 3]), base); + + expect(merged.nodes).toEqual([1, 9, 3]); + }); + + it('merges tags from both sides while taking one side\'s geometry', () => { + const base = osmWay(1, [1, 2], { tags: { highway: 'footway' } }); + const merged = mergeWays( + osmWay(1, [1, 2], { tags: { highway: 'footway', surface: 'asphalt' } }), + osmWay(1, [1, 2], { tags: { highway: 'footway', lit: 'yes' } }), + base + ); + + expect(merged.tags).toEqual({ + highway: 'footway', + surface: 'asphalt', + lit: 'yes' + }); + }); + + it('copies the node list so the merged way does not alias its source', () => { + const a = osmWay(1, [1, 2]); + const merged = mergeWays(a, osmWay(1, [1, 2])); + + merged.nodes.push(3); + + expect(a.nodes).toEqual([1, 2]); + }); +}); + +describe('mergeRelations', () => { + const outer = { type: 'way' as const, ref: 1, role: 'outer' }; + + it('takes the member list from the side that changed it', () => { + const base = osmRelation(1, [outer]); + const changed = [outer, { type: 'way' as const, ref: 2, role: 'inner' }]; + const merged = mergeRelations( + osmRelation(1, [{ ...outer }]), + osmRelation(1, changed), + base + ); + + expect(merged.members).toEqual(changed); + }); + + it('merges tags from both sides', () => { + const base = osmRelation(1, [outer], { tags: { type: 'multipolygon' } }); + const merged = mergeRelations( + osmRelation(1, [{ ...outer }], { tags: { type: 'multipolygon', name: 'Park' } }), + osmRelation(1, [{ ...outer }], { tags: { type: 'multipolygon', access: 'yes' } }), + base + ); + + expect(merged.tags).toEqual({ + type: 'multipolygon', + name: 'Park', + access: 'yes' + }); + }); + + it('copies each member so the merged relation does not alias its source', () => { + const a = osmRelation(1, [{ ...outer }]); + const merged = mergeRelations(a, osmRelation(1, [{ ...outer }])); + + merged.members[0]!.role = 'inner'; + + expect(a.members[0]!.role).toBe('outer'); + }); +}); + +describe('mergeOsmElements', () => { + it('refuses to merge two different element types', () => { + expect(() => mergeOsmElements(osmNode(1), osmWay(1, [2]))) + .toThrow(/type mismatch/); + }); + + it('copies the way node list instead of sharing it', () => { + const a = osmWay(1, [1, 2]); + const merged = mergeOsmElements(a, osmWay(1, [1, 2])) as OsmWay; + + merged.nodes.push(99); + + expect(a.nodes).toEqual([1, 2]); + }); +}); diff --git a/test/unit/util/osm-merge-tags.test.ts b/test/unit/util/osm-merge-tags.test.ts new file mode 100644 index 00000000..e769503e --- /dev/null +++ b/test/unit/util/osm-merge-tags.test.ts @@ -0,0 +1,143 @@ +import { describe, expect, it } from 'vitest'; + +import { osmNode } from '../../mocks/osm-elements'; + +import { mergeTagsThreeWay } from '~/util/osm-merge'; + +import type { OsmTags } from '~/types/osm'; + +/** The ancestor is an element, so that an untagged one still counts as one. */ +function ancestor(tags: OsmTags) { + return osmNode(1, { tags }); +} + +describe('mergeTagsThreeWay', () => { + it('keeps a value both sides left alone', () => { + const base = { highway: 'footway' }; + const result = mergeTagsThreeWay({ ...base }, { ...base }, ancestor(base)); + + expect(result.tags).toEqual({ highway: 'footway' }); + expect(result.conflicts).toEqual([]); + }); + + it('takes the edit when only one side changed a value', () => { + const base = { surface: 'asphalt' }; + + expect(mergeTagsThreeWay({ surface: 'concrete' }, { ...base }, ancestor(base))) + .toEqual({ tags: { surface: 'concrete' }, conflicts: [] }); + + expect(mergeTagsThreeWay({ ...base }, { surface: 'gravel' }, ancestor(base))) + .toEqual({ tags: { surface: 'gravel' }, conflicts: [] }); + }); + + it('keeps a tag only one side added, without calling it a conflict', () => { + // Two surveyors recording different attributes of the same feature is the + // ordinary case, not a disagreement. + const result = mergeTagsThreeWay( + { highway: 'footway', surface: 'asphalt' }, + { highway: 'footway', lit: 'yes' }, + ancestor({ highway: 'footway' }) + ); + + expect(result.tags).toEqual({ + highway: 'footway', + surface: 'asphalt', + lit: 'yes' + }); + expect(result.conflicts).toEqual([]); + }); + + it('honors a deletion made by one side', () => { + const result = mergeTagsThreeWay( + { highway: 'footway' }, + { highway: 'footway', surface: 'asphalt' }, + ancestor({ highway: 'footway', surface: 'asphalt' }) + ); + + expect(result.tags).toEqual({ highway: 'footway' }); + expect(result.conflicts).toEqual([]); + }); + + it('reports a key both sides changed to different values', () => { + const result = mergeTagsThreeWay( + { surface: 'asphalt' }, + { surface: 'concrete' }, + ancestor({ surface: 'gravel' }) + ); + + expect(result.conflicts).toEqual(['surface']); + }); + + it('reports a delete-versus-edit on the same key', () => { + const result = mergeTagsThreeWay( + { }, + { surface: 'concrete' }, + ancestor({ surface: 'asphalt' }) + ); + + expect(result.conflicts).toEqual(['surface']); + }); + + it('never concatenates two values into one tag', () => { + // A value like "asphalt;concrete" is not valid data, and it silently + // invents a third answer neither side gave. + const result = mergeTagsThreeWay( + { surface: 'asphalt' }, + { surface: 'concrete' }, + ancestor({ surface: 'gravel' }) + ); + + expect(result.tags.surface).not.toContain(';'); + expect(result.tags.surface).toBe('asphalt'); + }); + + it('treats an empty-string value as a real value, not as absent', () => { + const result = mergeTagsThreeWay({ note: '' }, { note: 'x' }, ancestor({ note: '' })); + + expect(result.conflicts).toEqual([]); + expect(result.tags).toEqual({ note: 'x' }); + }); + + it('treats an untagged ancestor element as a common ancestor', () => { + // The API omits `tags` entirely for an untagged element, which is every + // vertex node in an import. Reading that absence as "no ancestor" would + // turn every one-sided tag edit into a conflict. + expect(mergeTagsThreeWay({ surface: 'asphalt' }, { }, osmNode(1))) + .toEqual({ tags: { surface: 'asphalt' }, conflicts: [] }); + }); + + it('treats every difference as a conflict when there is no common ancestor', () => { + const result = mergeTagsThreeWay({ surface: 'asphalt' }, { lit: 'yes' }); + + expect(result.conflicts.sort()).toEqual(['lit', 'surface']); + }); +}); + +describe('tag keys that collide with Object.prototype', () => { + it.each(['toString', 'constructor', 'valueOf', 'hasOwnProperty', '__proto__'])( + 'treats a deleted %s tag as deleted, not as the inherited member', + (key) => { + // Reading these off a plain object literal returns the prototype member + // rather than undefined, which makes a deletion read as "unchanged" and + // writes a function into the merged tags, and into the uploaded XML. + const result = mergeTagsThreeWay( + { highway: 'footway' }, + { [key]: 'x', highway: 'footway' }, + ancestor({ [key]: 'x', highway: 'footway' }) + ); + + expect(result.tags).toEqual({ highway: 'footway' }); + expect(typeof result.tags[key]).not.toBe('function'); + } + ); + + it('keeps such a key when it is a real tag both sides carry', () => { + const result = mergeTagsThreeWay( + { toString: 'a value' }, + { toString: 'a value' }, + ancestor({ toString: 'a value' }) + ); + + expect(result.tags.toString).toBe('a value'); + }); +}); diff --git a/test/unit/util/osm.test.ts b/test/unit/util/osm.test.ts new file mode 100644 index 00000000..37fd7f7c --- /dev/null +++ b/test/unit/util/osm.test.ts @@ -0,0 +1,203 @@ +import { describe, expect, it } from 'vitest'; +import { reactive } from 'vue'; + +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +import { + copyOsmElement, + editorHashFor, + editorRoute, + geometryEqual, + mapHash, + newerOsmElement, + OsmElementLookup, + wayToGeoJsonCoords +} from '~/util/osm'; + +describe('geometryEqual', () => { + it('ignores tags', () => { + expect(geometryEqual( + osmNode(1, { tags: { a: 'b' } }), + osmNode(1, { tags: { c: 'd' } }) + )).toBe(true); + }); + + it('compares node coordinates', () => { + expect(geometryEqual(osmNode(1), osmNode(1, { lat: 47.9 }))).toBe(false); + }); + + it('compares way node lists in order', () => { + expect(geometryEqual(osmWay(1, [1, 2]), osmWay(1, [1, 2]))).toBe(true); + expect(geometryEqual(osmWay(1, [1, 2]), osmWay(1, [2, 1]))).toBe(false); + }); + + it('compares relation members', () => { + const members = [{ type: 'way' as const, ref: 1, role: 'outer' }]; + + expect(geometryEqual(osmRelation(1, members), osmRelation(1, [...members]))).toBe(true); + expect(geometryEqual( + osmRelation(1, members), + osmRelation(1, [{ type: 'way', ref: 2, role: 'outer' }]) + )).toBe(false); + }); + + it('treats different element types as unequal', () => { + expect(geometryEqual(osmNode(1), osmWay(1, [2]))).toBe(false); + }); +}); + +describe('newerOsmElement', () => { + it('picks the element with the later timestamp', () => { + const older = osmNode(1, { timestamp: new Date('2026-01-01T00:00:00Z') }); + const newer = osmNode(1, { timestamp: new Date('2026-06-01T00:00:00Z') }); + + expect(newerOsmElement(older, newer)).toBe(newer); + expect(newerOsmElement(newer, older)).toBe(newer); + }); +}); + +describe('copyOsmElement', () => { + it('copies tags and node lists deeply', () => { + const original = osmWay(1, [1, 2], { tags: { highway: 'footway' } }); + const copy = copyOsmElement(original); + + copy.nodes.push(3); + copy.tags!.highway = 'path'; + + expect(original.nodes).toEqual([1, 2]); + expect(original.tags).toEqual({ highway: 'footway' }); + }); + + it('copies relation members so roles can be edited independently', () => { + const original = osmRelation(1, [{ type: 'way', ref: 5, role: 'outer' }]); + const copy = copyOsmElement(original); + + copy.members[0]!.role = 'inner'; + + expect(original.members[0]!.role).toBe('outer'); + }); + + it('works on an element read back out of reactive state', () => { + // structuredClone throws DataCloneError on a Proxy, and elements are + // routinely read back out of Vue state. + const proxied = reactive(osmNode(1, { tags: { a: 'b' } })); + + expect(() => copyOsmElement(proxied)).not.toThrow(); + expect(copyOsmElement(proxied).tags).toEqual({ a: 'b' }); + }); +}); + +describe('OsmElementLookup', () => { + it('iterates nodes, then ways, then relations', () => { + // The osmChange builder depends on this order to resolve way and relation + // references to the placeholders it assigns. + const lookup = new OsmElementLookup([ + osmRelation(1, []), + osmWay(1, [1]), + osmNode(1) + ]); + + expect([...lookup].map(e => e.type)).toEqual(['node', 'way', 'relation']); + }); + + it('keys separately per element type', () => { + const lookup = new OsmElementLookup([osmNode(1), osmWay(1, [1])]); + + expect(lookup.get('node', 1)!.type).toBe('node'); + expect(lookup.get('way', 1)!.type).toBe('way'); + expect(lookup.size).toBe(2); + }); + + it('removes and reports membership by type and ID', () => { + const lookup = new OsmElementLookup([osmNode(1)]); + + expect(lookup.has('node', 1)).toBe(true); + lookup.remove('node', 1); + expect(lookup.has('node', 1)).toBe(false); + }); + + it('stays usable after a round trip through reactive state', () => { + // A reactive Proxy cannot forward #private field access, so a lookup built + // on private fields throws TypeError on every method once it has been + // stored in Vue state. The conflicts page stores its lookup this way. + const state = reactive({ + lookup: new OsmElementLookup([osmNode(1), osmWay(2, [1])]) + }); + + expect(() => state.lookup.get('node', 1)).not.toThrow(); + expect(state.lookup.get('node', 1)!.id).toBe(1); + expect([...state.lookup]).toHaveLength(2); + expect(() => new OsmElementLookup(state.lookup)).not.toThrow(); + }); +}); + +describe('wayToGeoJsonCoords', () => { + it('resolves each node reference to a coordinate pair', () => { + const lookup = new OsmElementLookup([ + osmNode(1, { lat: 47.6, lon: -122.3 }), + osmNode(2, { lat: 47.7, lon: -122.4 }) + ]); + + expect(wayToGeoJsonCoords(osmWay(9, [1, 2]), lookup)).toEqual([ + [-122.3, 47.6], + [-122.4, 47.7] + ]); + }); + + it('returns undefined when a node is missing rather than a shortcut line', () => { + // Dropping the missing vertex would draw a straight line past it, which + // reads as real geometry rather than as missing data. + const lookup = new OsmElementLookup([osmNode(1), osmNode(3)]); + + expect(wayToGeoJsonCoords(osmWay(9, [1, 2, 3]), lookup)).toBeUndefined(); + }); +}); + +describe('editorHashFor', () => { + it('centres on the extent of the nodes at the lowest editable zoom', () => { + const elements = [ + osmNode(1, { lat: 47.6, lon: -122.4 }), + osmNode(2, { lat: 47.8, lon: -122.2 }), + osmWay(9, [1, 2]) + ]; + + const [zoom, lat, lon] = editorHashFor(elements)!.replace('#map=', '').split('/').map(Number); + + expect(zoom).toBe(16); + expect(lat).toBeCloseTo(47.7); + expect(lon).toBeCloseTo(-122.3); + }); + + it('treats zero as a real coordinate', () => { + expect(editorHashFor([osmNode(1, { lat: 0, lon: 0 })])).toBe('#map=16/0/0'); + }); + + it('returns undefined without nodes rather than a made-up position', () => { + expect(editorHashFor([osmWay(9, [1, 2])])).toBeUndefined(); + expect(editorHashFor([])).toBeUndefined(); + }); +}); + +describe('mapHash', () => { + it('writes the view as zoom, latitude, longitude', () => { + expect(mapHash({ zoom: 11.5, lat: 47.6, lon: -122.3 })).toBe('#map=11.5/47.6/-122.3'); + }); +}); + +describe('editorRoute', () => { + it('names the editor for the workspace type and opens it at the hash', () => { + expect(editorRoute(99, 'osw', '#map=16/47.6/-122.3')).toEqual({ + path: '/workspace/99/edit', + query: { datatype: 'osw' }, + hash: '#map=16/47.6/-122.3' + }); + }); + + it('leaves the hash out rather than inventing a view', () => { + expect(editorRoute(99, 'pathways')).toEqual({ + path: '/workspace/99/edit', + query: { datatype: 'pathways' } + }); + expect(editorRoute(99, 'osw', undefined)).not.toHaveProperty('hash'); + }); +}); diff --git a/test/unit/util/osmChange.test.ts b/test/unit/util/osmChange.test.ts new file mode 100644 index 00000000..0760cad3 --- /dev/null +++ b/test/unit/util/osmChange.test.ts @@ -0,0 +1,175 @@ +import { describe, expect, it } from 'vitest'; + +import { osmNode, osmRelation, osmWay } from '../../mocks/osm-elements'; + +import { OsmChangeBuilder } from '~/util/osmChange'; + +/** Pulls the elements out of one osmChange section, in document order. */ +function section(xml: string, name: 'create' | 'modify' | 'delete') { + const doc = new DOMParser().parseFromString(xml, 'application/xml'); + const block = doc.getElementsByTagName(name)[0]!; + + return Array.from(block.children).map((element) => { + const children = Array.from(element.children); + + return { + tag: element.tagName.toLowerCase(), + id: element.getAttribute('id'), + version: element.getAttribute('version'), + changeset: element.getAttribute('changeset'), + refs: children + .filter(child => ['nd', 'member'].includes(child.tagName.toLowerCase())) + .map(child => child.getAttribute('ref')), + tags: Object.fromEntries( + children + .filter(child => child.tagName.toLowerCase() === 'tag') + .map(child => [child.getAttribute('k'), child.getAttribute('v')]) + ) + }; + }); +} + +describe('OsmChangeBuilder placeholder IDs', () => { + it('gives every created element a negative placeholder, whatever its version', () => { + // The API rejects the whole changeset when a carries a positive + // ID, and an element's version in its source workspace says nothing about + // whether it exists in the target. + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(500)); + builder.create(osmNode(501, { version: 4 })); + + expect(section(builder.serializeXml(), 'create').map(e => e.id)) + .toEqual(['-1', '-2']); + }); + + it('numbers placeholders independently per element type', () => { + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(1)); + builder.create(osmWay(1, [1])); + builder.create(osmRelation(1, [{ type: 'way', ref: 1, role: '' }])); + + expect(section(builder.serializeXml(), 'create').map(e => [e.tag, e.id])) + .toEqual([['node', '-1'], ['way', '-1'], ['relation', '-1']]); + }); + + it('repoints way node refs at the placeholders assigned to those nodes', () => { + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(500, { version: 3 })); + builder.create(osmNode(501, { version: 3 })); + builder.create(osmWay(900, [500, 501], { version: 2 })); + + expect(section(builder.serializeXml(), 'create')[2]!.refs).toEqual(['-1', '-2']); + }); + + it('repoints relation member refs, including references to other relations', () => { + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(10)); + builder.create(osmWay(20, [10])); + builder.create(osmRelation(30, [{ type: 'way', ref: 20, role: 'outer' }])); + builder.create(osmRelation(31, [ + { type: 'node', ref: 10, role: '' }, + { type: 'relation', ref: 30, role: 'sub' } + ])); + + const created = section(builder.serializeXml(), 'create'); + + expect(created[2]!.refs).toEqual(['-1']); + expect(created[3]!.refs).toEqual(['-1', '-1']); + }); + + it('throws, naming the referrer, rather than emitting a ref it cannot resolve', () => { + // Falling back to the original ID here produces a reference the server + // cannot resolve, and the upload fails partway through with a message that + // points at the server instead of at the changeset being built. + const builder = new OsmChangeBuilder(77); + + expect(() => builder.create(osmWay(900, [12345], { version: 2 }))) + .toThrow(/way 900 references node 12345/); + }); +}); + +describe('OsmChangeBuilder element serialization', () => { + it('writes lat and lon as separate attributes', () => { + const builder = new OsmChangeBuilder(77); + builder.create(osmNode(1)); + + const xml = builder.serializeXml(); + + expect(xml).toMatch(/lat="47\.6"/); + expect(xml).toMatch(/lon="-122\.3"/); + }); + + it('stamps every element with the changeset it was built for', () => { + const builder = new OsmChangeBuilder(77); + builder.create(osmNode(1)); + + expect(section(builder.serializeXml(), 'create')[0]!.changeset).toBe('77'); + }); + + it('omits version on create and carries it on modify and delete', () => { + // Version is the optimistic-concurrency check for an element that already + // exists. A created element has no previous version to check against. + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(1)); + builder.modify(osmNode(2, { version: 5 })); + builder.delete(osmNode(3, { version: 9 })); + + const xml = builder.serializeXml(); + + expect(section(xml, 'create')[0]!.version).toBeNull(); + expect(section(xml, 'modify')[0]!.version).toBe('5'); + expect(section(xml, 'delete')[0]!.version).toBe('9'); + }); + + it('keeps the original ID for modify and delete', () => { + const builder = new OsmChangeBuilder(77); + + builder.modify(osmNode(42, { version: 5 })); + builder.delete(osmNode(43, { version: 9 })); + + const xml = builder.serializeXml(); + + expect(section(xml, 'modify')[0]!.id).toBe('42'); + expect(section(xml, 'delete')[0]!.id).toBe('43'); + }); + + it('escapes XML metacharacters in tag keys and values', () => { + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(1, { + tags: { 'name': 'A & "C"', 'evil"/> "C"', + 'evil"/> { + const builder = new OsmChangeBuilder(77); + + builder.create(osmNode(1, { + tags: { surface: undefined as unknown as string, highway: 'footway' } + })); + + expect(section(builder.serializeXml(), 'create')[0]!.tags) + .toEqual({ highway: 'footway' }); + }); + + it('produces a well-formed document with no elements', () => { + const xml = new OsmChangeBuilder(77).serializeXml(); + const doc = new DOMParser().parseFromString(xml, 'application/xml'); + + expect(doc.getElementsByTagName('parsererror')).toHaveLength(0); + expect(section(xml, 'create')).toEqual([]); + }); +}); diff --git a/types/osm.ts b/types/osm.ts index 72826183..6b906470 100644 --- a/types/osm.ts +++ b/types/osm.ts @@ -13,7 +13,7 @@ interface BaseElement { user: string; uid: number; type: OsmElementType; - tags: OsmTags; + tags?: OsmTags; }; export interface OsmNode extends BaseElement { diff --git a/util/collections.ts b/util/collections.ts new file mode 100644 index 00000000..282e441c --- /dev/null +++ b/util/collections.ts @@ -0,0 +1,26 @@ +import type { OsmRelationRef } from '~/types/osm'; + +export function shallowArrayEqual(a: T[], b: T[]): boolean { + if (a === b) { + return true; + } + + if (a.length !== b.length) { + return false; + } + + return a.every((v, i) => v === b[i]); +} + +export function shallowMembersEqual( + a: OsmRelationRef[], + b: OsmRelationRef[] +): boolean { + if (a.length !== b.length) { + return false; + } + + return a.every((m, i) => + m.type === b[i]?.type && m.ref === b[i]?.ref && m.role === b[i]?.role + ); +} diff --git a/util/map-style.ts b/util/map-style.ts index fa04678c..6bfd10f7 100644 --- a/util/map-style.ts +++ b/util/map-style.ts @@ -4,6 +4,34 @@ // style (sources, layers, attribution) from this single URL. export const OPENFREEMAP_STYLE_URL = 'https://tiles.openfreemap.org/styles/positron'; +export function bingImageryStyle(): import('maplibre-gl').StyleSpecification { + return { + version: 8, + sources: { + bing: { + type: 'raster', + scheme: 'xyz', + tiles: [ + 'https://ecn.t0.tiles.virtualearth.net/tiles/a{quadkey}.jpeg?g=587&mkt=en-gb&n=z', + 'https://ecn.t1.tiles.virtualearth.net/tiles/a{quadkey}.jpeg?g=587&mkt=en-gb&n=z', + 'https://ecn.t2.tiles.virtualearth.net/tiles/a{quadkey}.jpeg?g=587&mkt=en-gb&n=z', + 'https://ecn.t3.tiles.virtualearth.net/tiles/a{quadkey}.jpeg?g=587&mkt=en-gb&n=z' + ], + tileSize: 256, + maxzoom: 20, + attribution: 'Imagery © Microsoft Corporation' + } + }, + layers: [ + { + id: 'imagery', + type: 'raster', + source: 'bing' + } + ] + }; +} + export interface MaplibreMapHandle { maplibregl: typeof import('maplibre-gl'); map: import('maplibre-gl').Map; @@ -31,7 +59,7 @@ export async function createMaplibreMap( container, style: OPENFREEMAP_STYLE_URL, center: options.center, - zoom: options.zoom, + zoom: options.zoom }); map.addControl(new maplibregl.NavigationControl({ showCompass: false }), options.controlPosition ?? 'top-right'); diff --git a/util/osm-merge.ts b/util/osm-merge.ts new file mode 100644 index 00000000..326c4868 --- /dev/null +++ b/util/osm-merge.ts @@ -0,0 +1,197 @@ +import { geometryEqual, newerOsmElement } from '~/util/osm'; + +import type { + OsmElement, + OsmNode, + OsmRelation, + OsmTags, + OsmWay +} from '~/types/osm'; + +export interface TagMergeResult { + tags: OsmTags; + conflicts: string[]; +} + +function ownTags(tags: OsmTags | undefined): OsmTags { + const own: OsmTags = Object.create(null); + + // This module is deliberately free of Vue and cannot unwrap a reactive + // proxy: copying one here reads each key through the proxy's get trap, so a + // key named `hasOwnProperty` comes back as Vue's own function. Callers that + // hold tags in the reactive graph unwrap with `toRaw` at their own boundary: + // + return Object.assign(own, tags); +} + +export function mergeTagsThreeWay( + a: OsmTags | undefined, + b: OsmTags | undefined, + base?: OsmElement +): TagMergeResult { + const aTags = ownTags(a); + const bTags = ownTags(b); + + const baseTags = ownTags(base?.tags); + const tags: OsmTags = Object.create(null); + const conflicts: string[] = []; + + const keys = new Set([ + ...Object.keys(aTags), + ...Object.keys(bTags), + ...Object.keys(baseTags) + ]); + + for (const key of keys) { + const aValue = aTags[key]; + const bValue = bTags[key]; + const baseValue = baseTags[key]; + + if (aValue === bValue) { + if (aValue !== undefined) { + tags[key] = aValue; + } + continue; + } + + if (base !== undefined) { + if (aValue === baseValue) { + if (bValue !== undefined) { + tags[key] = bValue; + } + continue; + } + + if (bValue === baseValue) { + if (aValue !== undefined) { + tags[key] = aValue; + } + continue; + } + } + + conflicts.push(key); + + if (aValue !== undefined) { + tags[key] = aValue; + } + } + + return { tags, conflicts }; +} + +export function checkGeometryConflict( + a: OsmElement, + b: OsmElement, + base?: OsmElement +): boolean { + if (a.type !== b.type) { + return true; + } + + if (geometryEqual(a, b)) { + return false; + } + + if (base !== undefined && (geometryEqual(a, base) || geometryEqual(b, base))) { + return false; + } + + return true; +} + +export function checkConflict( + a: OsmElement, + b: OsmElement, + base?: OsmElement +): boolean { + if (checkGeometryConflict(a, b, base)) { + return true; + } + + return mergeTagsThreeWay(a.tags, b.tags, base).conflicts.length > 0; +} + +function geometrySource(a: T, b: T, base?: OsmElement): T { + if (base !== undefined && geometryEqual(a, base)) { + return b; // A did not change, so take geometry from B + } + + return a; +} + +export function mergeNodes(a: OsmNode, b: OsmNode, base?: OsmElement): OsmNode { + const newer = newerOsmElement(a, b); + const geometry = geometrySource(a, b, base); + + return { + id: newer.id, + type: 'node', + lat: geometry.lat, + lon: geometry.lon, + version: newer.version + 1, + timestamp: new Date(), + changeset: 0, + user: newer.user, + uid: newer.uid, + tags: mergeTagsThreeWay(a.tags, b.tags, base).tags + }; +} + +export function mergeWays(a: OsmWay, b: OsmWay, base?: OsmElement): OsmWay { + const newer = newerOsmElement(a, b); + const geometry = geometrySource(a, b, base); + + return { + id: newer.id, + type: 'way', + version: newer.version + 1, + timestamp: new Date(), + changeset: 0, + user: newer.user, + uid: newer.uid, + nodes: [...geometry.nodes], + tags: mergeTagsThreeWay(a.tags, b.tags, base).tags + }; +} + +export function mergeRelations( + a: OsmRelation, + b: OsmRelation, + base?: OsmElement +): OsmRelation { + const newer = newerOsmElement(a, b); + const geometry = geometrySource(a, b, base); + + return { + id: newer.id, + type: 'relation', + version: newer.version + 1, + timestamp: new Date(), + changeset: 0, + user: newer.user, + uid: newer.uid, + members: geometry.members.map(member => ({ ...member })), + tags: mergeTagsThreeWay(a.tags, b.tags, base).tags + }; +} + +export function mergeOsmElements( + a: OsmElement, + b: OsmElement, + base?: OsmElement +): OsmElement { + if (a.type !== b.type) { + throw new Error(`OSM element type mismatch: ${a.type} != ${b.type}`); + } + + if (a.type === 'node') { + return mergeNodes(a, b as OsmNode, base); + } + + if (a.type === 'way') { + return mergeWays(a, b as OsmWay, base); + } + + return mergeRelations(a, b as OsmRelation, base); +} diff --git a/util/osm.ts b/util/osm.ts new file mode 100644 index 00000000..d3afcfe5 --- /dev/null +++ b/util/osm.ts @@ -0,0 +1,154 @@ +import { OSM_ELEMENT_TYPES } from '~/types/osm'; + +import { shallowArrayEqual, shallowMembersEqual } from '~/util/collections'; + +import type { + OsmElement, + OsmElementType, + OsmNode, + OsmRelation, + OsmWay +} from '~/types/osm'; +import type { WorkspaceType } from '~/types/workspaces'; + +export function geometryEqual(a: OsmElement, b: OsmElement): boolean { + if (a.type !== b.type) { + return false; + } + + if (a.type === 'node') { + const other = b as OsmNode; + return a.lat === other.lat && a.lon === other.lon; + } + + if (a.type === 'way') { + return shallowArrayEqual(a.nodes, (b as OsmWay).nodes); + } + + return shallowMembersEqual(a.members, (b as OsmRelation).members); +} + +export function newerOsmElement(a: T, b: T): T { + return a.timestamp.getTime() > b.timestamp.getTime() ? a : b; +} + +export function wayToGeoJsonCoords( + way: OsmWay, + lookup: OsmElementLookup +): [number, number][] | undefined { + const coords: [number, number][] = []; + + for (const nodeId of way.nodes) { + const node = lookup.get('node', nodeId) as OsmNode | undefined; + + if (!node) { + return undefined; + } + + coords.push([node.lon, node.lat]); + } + + return coords; +} + +export function editorHashFor(elements: Iterable): string | undefined { + let minLat = Infinity; + let maxLat = -Infinity; + let minLon = Infinity; + let maxLon = -Infinity; + + for (const element of elements) { + if (element.type === 'node') { + minLat = Math.min(minLat, element.lat); + maxLat = Math.max(maxLat, element.lat); + minLon = Math.min(minLon, element.lon); + maxLon = Math.max(maxLon, element.lon); + } + } + + if (minLat === Infinity) { + return undefined; + } + + return mapHash({ zoom: 16, lat: (minLat + maxLat) / 2, lon: (minLon + maxLon) / 2 }); +} + +export function mapHash(view: { zoom: number; lat: number; lon: number }): string { + return `#map=${view.zoom}/${view.lat}/${view.lon}`; +} + +export function editorRoute(workspaceId: number, datatype: WorkspaceType, hash?: string) { + return { + path: `/workspace/${workspaceId}/edit`, + query: { datatype }, + ...(hash ? { hash } : { }) + }; +} + +export function copyOsmElement(element: T): T { + const copy = { ...element, timestamp: new Date(element.timestamp) }; + + if (element.tags) { + copy.tags = { ...element.tags }; + } + + if (copy.type === 'way') { + copy.nodes = [...copy.nodes]; + } else if (copy.type === 'relation') { + copy.members = copy.members.map(member => ({ ...member })); + } + + return copy; +} + +type OsmElementMap = Map; + +export class OsmElementLookup implements Iterable { + readonly types: Record = { + node: new Map(), + way: new Map(), + relation: new Map() + }; + + constructor(elements?: Iterable) { + if (elements) { + for (const element of elements) { + this.insert(element); + } + } + } + + get(type: OsmElementType, id: number): OsmElement | undefined { + return this.types[type].get(id); + } + + has(type: OsmElementType, id: number): boolean { + return this.types[type].has(id); + } + + insert(element: OsmElement) { + this.types[element.type].set(element.id, element); + } + + remove(type: OsmElementType, id: number) { + this.types[type].delete(id); + } + + drop(element: OsmElement) { + this.remove(element.type, element.id); + } + + get size(): number { + return this.types.node.size + this.types.way.size + this.types.relation.size; + } + + elementsForType(type: OsmElementType): MapIterator { + return this.types[type].values(); + } + + * [Symbol.iterator](): Iterator { + for (const type of OSM_ELEMENT_TYPES) { + yield* this.elementsForType(type); + } + } +} diff --git a/util/osmChange.ts b/util/osmChange.ts new file mode 100644 index 00000000..5481c0ac --- /dev/null +++ b/util/osmChange.ts @@ -0,0 +1,127 @@ +import * as xml from '~/util/xml'; + +import type { OsmChangeActionType, OsmElement, OsmElementType } from '~/types/osm'; + +const OSC_TEMPLATE = ''; + +export class OsmChangeBuilder { + readonly #doc: XMLDocument; + readonly #createNode: ChildNode; + readonly #modifyNode: ChildNode; + readonly #deleteNode: ChildNode; + readonly #changesetId: number; + + readonly #idLookup: Record> = { + node: new Map(), + way: new Map(), + relation: new Map() + }; + + readonly #placeholderSequence: Record = { + node: -1, + way: -1, + relation: -1 + }; + + constructor(changesetId: number) { + this.#doc = xml.parse(OSC_TEMPLATE); + + const osmChange = this.#doc.firstChild!; + + this.#createNode = osmChange.childNodes[0]!; + this.#modifyNode = osmChange.childNodes[1]!; + this.#deleteNode = osmChange.childNodes[2]!; + this.#changesetId = changesetId; + } + + create(element: OsmElement) { + this.#createNode.appendChild(this.#element2Xml(element, 'create')); + } + + modify(element: OsmElement) { + this.#modifyNode.appendChild(this.#element2Xml(element, 'modify')); + } + + delete(element: OsmElement) { + this.#deleteNode.appendChild(this.#element2Xml(element, 'delete')); + } + + serializeXml(): string { + return xml.serialize(this.#doc); + } + + #element2Xml(element: OsmElement, action: OsmChangeActionType): Element { + const feature = xml.makeNode(this.#doc, element.type, { + id: this.#getId(element, action), + changeset: this.#changesetId + }); + + // The API uses "version" for optimistic concurrency on an element that + // already exists. A created element has no previous version to check: + // + if (action !== 'create') { + feature.setAttribute('version', element.version.toString()); + } + + if (element.type === 'node') { + feature.setAttribute('lat', element.lat.toString()); + feature.setAttribute('lon', element.lon.toString()); + } else if (element.type === 'way') { + for (const ref of element.nodes) { + feature.appendChild(xml.makeNode(this.#doc, 'nd', { + ref: this.#getIdFor('node', ref, element) + })); + } + } else if (element.type === 'relation') { + for (const member of element.members) { + feature.appendChild(xml.makeNode(this.#doc, 'member', { + ref: this.#getIdFor(member.type, member.ref, element), + type: member.type, + role: member.role + })); + } + } + + // A for...in loop here could pollute this by walking the prototype chain: + // + for (const [key, value] of Object.entries(element.tags ?? { })) { + // A tag with no value is a tag the caller meant to drop. Serializing it + // would write the string "undefined" into the dataset: + // + if (value === undefined) { + continue; + } + + feature.appendChild(xml.makeNode(this.#doc, 'tag', { k: key, v: value })); + } + + return feature; + } + + #getId(element: OsmElement, action: OsmChangeActionType): number { + const id = action === 'create' + ? this.#placeholderSequence[element.type]-- + : element.id; + + this.#idLookup[element.type].set(element.id, id); + + return id; + } + + #getIdFor(type: OsmElementType, id: number, referrer: OsmElement): number { + const placeholder = this.#idLookup[type].get(id); + + if (placeholder === undefined) { + // Emitting the original ID here would produce a reference the server + // cannot resolve, and the upload would fail partway through with a + // message that points at the server rather than at this builder: + // + throw new Error( + `${referrer.type} ${referrer.id} references ${type} ${id}, ` + + 'which is not part of this changeset' + ); + } + + return placeholder; + } +} diff --git a/util/xml.ts b/util/xml.ts index 6005131c..30019ed0 100644 --- a/util/xml.ts +++ b/util/xml.ts @@ -12,8 +12,10 @@ export function serialize(input: XMLDocument): string { export function makeNode(doc: XMLDocument, tag: string, attrs: Record): Element { const node = doc.createElement(tag); - for (const prop in attrs) { - node.setAttribute(prop, String(attrs[prop])); + // A for...in loop here could pollute this by walking the prototype chain: + // + for (const [prop, value] of Object.entries(attrs)) { + node.setAttribute(prop, String(value)); } return node;