diff --git a/.vscode/launch.json b/.vscode/launch.json index eb75935..56c4732 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -10,7 +10,7 @@ "name": "Jasmine Tests with SQLite", "env": { "MUDBASE_DB_CONFIG": "{\"client\": \"sqlite3\", \"connection\": \":memory:\"}", - "TEST_RANDOM_SEED": "0.04801" + "TEST_RANDOM_SEED": "0.92820" }, "program": "${workspaceFolder}/server/node_modules/jasmine-ts/lib/index", "args": [ diff --git a/server/db/Artist.ts b/server/db/Artist.ts index 99eeffc..604bf30 100644 --- a/server/db/Artist.ts +++ b/server/db/Artist.ts @@ -170,28 +170,28 @@ export async function modifyArtist(userId: number, artistId: number, artist: Art // Start retrieving albums if we are modifying those. const albumIdsPromise: Promise = artist.albumIds ? - trx.select('albumId') - .from('artists_albums') + trx.select('id') + .from('albums') .whereIn('id', artist.albumIds) - .then((as: any) => as.map((a: any) => a['albumId'])) + .then((as: any) => as.map((a: any) => a['id'])) : (async () => undefined)(); // Start retrieving tracks if we are modifying those. const trackIdsPromise: Promise = artist.trackIds ? - trx.select('trackId') - .from('tracks_artists') + trx.select('id') + .from('tracks') .whereIn('id', artist.trackIds) - .then((as: any) => as.map((a: any) => a['trackId'])) + .then((as: any) => as.map((a: any) => a['id'])) : (async () => undefined)(); // Start retrieving tags if we are modifying those. const tagIdsPromise = artist.tagIds ? trx.select('id') - .from('artists_tags') + .from('tags') .whereIn('id', artist.tagIds) - .then((ts: any) => ts.map((t: any) => t['tagId'])) : + .then((ts: any) => ts.map((t: any) => t['id'])) : (async () => undefined)(); // Wait for the requests to finish. diff --git a/server/db/Tag.ts b/server/db/Tag.ts index fa6412a..dcc091d 100644 --- a/server/db/Tag.ts +++ b/server/db/Tag.ts @@ -222,28 +222,73 @@ export async function mergeTag(userId: number, fromId: number, toId: number, kne .where({ id: toId }) .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined) + // Start retrieving any children of the 'from' tag + const childrenPromise = getTagChildrenRecursive(fromId, userId, trx); + // Wait for the requests to finish. - var [fromTagId, toTagId] = await Promise.all([fromTagIdPromise, toTagIdPromise]); + var [fromTagId, toTagId, fromChildren] = await Promise.all([fromTagIdPromise, toTagIdPromise, childrenPromise]); // Check that we found all objects we need. if (!fromTagId || !toTagId) { throw makeNotFoundError(); } - // Assign new tag ID to any objects referencing the to-be-merged tag. + // Check that we are not merging to itself and not merging with its own children + if (fromTagId === toTagId) { + const e: DBError = { + name: "DBError", + kind: DBErrorKind.ResourceConflict, + message: 'Cannot merge a tag into itself', + }; + throw e; + } + if (fromChildren.includes(toId)) { + const e: DBError = { + name: "DBError", + kind: DBErrorKind.ResourceConflict, + message: 'Cannot merge a tag with one of its children.', + }; + throw e; + } + + // Move any child tags under the new tag. const cPromise = trx('tags') .where({ 'user': userId }) .where({ 'parentId': fromId }) .update({ 'parentId': toId }); - const sPromise = trx('songs_tags') - .where({ 'tagId': fromId }) - .update({ 'tagId': toId }); - const arPromise = trx('artists_tags') - .where({ 'tagId': fromId }) - .update({ 'tagId': toId }); - const alPromise = trx('albums_tags') - .where({ 'tagId': fromId }) - .update({ 'tagId': toId }); + + // Assign new tag ID to any objects referencing the to-be-merged tag. + let doReplacement = async (table: string, otherIdField: string) => { + // Store the items referencing the old tag. + let referencesFrom = await trx(table) + .select([otherIdField]) + .where({ 'tagId': fromId }) + .then((r: any) => r.map((result: any) => result[otherIdField])) + + // Store the items referencing the new tag. + let referencesTo = await trx(table) + .select([otherIdField]) + .where({ 'tagId': toId }) + .then((r: any) => r.map((result: any) => result[otherIdField])) + + let referencesEither = [...referencesFrom, ...referencesTo]; + let referencesBoth = referencesEither.filter((id: number) => referencesFrom.includes(id) && referencesTo.includes(id)); + let referencesOnlyFrom = referencesEither.filter((id: number) => referencesFrom.includes(id) && !referencesTo.includes(id)); + + // For items referencing only the from tag, update to the to tag. + await trx(table) + .whereIn(otherIdField, referencesOnlyFrom) + .where({ 'tagId': fromId }) + .update({ 'tagId': toId }); + // For items referencing both, just remove the reference to the from tag. + await trx(table) + .whereIn(otherIdField, referencesBoth) + .where({ 'tagId': fromId }) + .delete(); + } + const sPromise = doReplacement('tracks_tags', 'trackId'); + const arPromise = doReplacement('artists_tags', 'artistId'); + const alPromise = doReplacement('albums_tags', 'albumId'); await Promise.all([sPromise, arPromise, alPromise, cPromise]); // Delete the original tag. diff --git a/server/test/integration/flows/ResourceFlow.ts b/server/test/integration/flows/ResourceFlow.ts index cb770ab..07bc993 100644 --- a/server/test/integration/flows/ResourceFlow.ts +++ b/server/test/integration/flows/ResourceFlow.ts @@ -123,8 +123,8 @@ function transformActionIDs(action: DBAction, mappings: IDMappings, rng: any) { case DBActionType.PatchTrack: { let track = r.payload as TrackWithRefsWithId; - track.tagIds && track.tagIds.forEach((id: number) => doMap(id, mappings.tags)); - track.artistIds && track.artistIds.forEach((id: number) => doMap(id, mappings.artists)); + track.tagIds && track.tagIds.forEach((id: number, idx: number) => track.tagIds[idx] = doMap(id, mappings.tags)); + track.artistIds && track.artistIds.forEach((id: number, idx: number) => track.artistIds[idx] = doMap(id, mappings.artists)); if (track.albumId) { track.albumId = doMap(track.albumId, mappings.albums); } if (track.id) { track.id = doMap(track.id, mappings.tracks); } break; @@ -134,9 +134,9 @@ function transformActionIDs(action: DBAction, mappings: IDMappings, rng: any) { case DBActionType.PatchArtist: { let artist = r.payload as ArtistWithRefsWithId; - artist.tagIds && artist.tagIds.forEach((id: number) => doMap(id, mappings.tags)); - artist.albumIds && artist.albumIds.forEach((id: number) => doMap(id, mappings.albums)); - artist.trackIds && artist.trackIds.forEach((id: number) => doMap(id, mappings.tracks)); + artist.tagIds && artist.tagIds.forEach((id: number, idx: number) => artist.tagIds[idx] = doMap(id, mappings.tags)); + artist.albumIds && artist.albumIds.forEach((id: number, idx: number) => artist.albumIds[idx] = doMap(id, mappings.albums)); + artist.trackIds && artist.trackIds.forEach((id: number, idx: number) => artist.trackIds[idx] = doMap(id, mappings.tracks)); if (artist.id) { artist.id = doMap(artist.id, mappings.artists); } break; } @@ -145,10 +145,12 @@ function transformActionIDs(action: DBAction, mappings: IDMappings, rng: any) { case DBActionType.PatchAlbum: { let album = r.payload as AlbumWithRefsWithId; - album.tagIds && album.tagIds.forEach((id: number) => doMap(id, mappings.tags)); - album.artistIds && album.artistIds.forEach((id: number) => doMap(id, mappings.artists)); - album.trackIds && album.trackIds.forEach((id: number) => doMap(id, mappings.tracks)); + console.log(`album trackIds before: ${JSON.stringify(album.trackIds)}`) + album.tagIds && album.tagIds.forEach((id: number, idx: number) => album.tagIds[idx] = doMap(id, mappings.tags)); + album.artistIds && album.artistIds.forEach((id: number, idx: number) => album.artistIds[idx] = doMap(id, mappings.artists)); + album.trackIds && album.trackIds.forEach((id: number, idx: number) => album.trackIds[idx] = doMap(id, mappings.tracks)); if (album.id) { album.id = doMap(album.id, mappings.albums); } + console.log(`album trackIds after: ${JSON.stringify(album.trackIds)}`) break; } case DBActionType.CreateTag: @@ -174,6 +176,12 @@ function transformActionIDs(action: DBAction, mappings: IDMappings, rng: any) { r.payload = (mappings as any)[keys[r.type]][r.payload]; break; } + case DBActionType.MergeTag: + { + r.payload.fromId = doMap(r.payload.fromId, mappings.tags); + r.payload.toId = doMap(r.payload.toId, mappings.tags); + break; + } } return r; } @@ -234,22 +242,23 @@ describe('Randomized model-based DB back-end tests', () => { // be generated. let dist: RandomDBActionDistribution = { type: new Map([ - [DBActionType.CreateTrack, 0.0625], - [DBActionType.CreateArtist, 0.0625], - [DBActionType.CreateAlbum, 0.0625], - [DBActionType.CreateTag, 0.0625], - [DBActionType.PutTrack, 0.0625], - [DBActionType.PutArtist, 0.0625], - [DBActionType.PutAlbum, 0.0625], - [DBActionType.PutTag, 0.0625], - [DBActionType.PatchTrack, 0.0625], - [DBActionType.PatchArtist, 0.0625], - [DBActionType.PatchAlbum, 0.0625], - [DBActionType.PatchTag, 0.0625], - [DBActionType.DeleteTrack, 0.0625], - [DBActionType.DeleteArtist, 0.0625], - [DBActionType.DeleteAlbum, 0.0625], - [DBActionType.DeleteTag, 0.0625], + [DBActionType.CreateTrack, 0.05883], + [DBActionType.CreateArtist, 0.05883], + [DBActionType.CreateAlbum, 0.05883], + [DBActionType.CreateTag, 0.05883], + [DBActionType.PutTrack, 0.05883], + [DBActionType.PutArtist, 0.05883], + [DBActionType.PutAlbum, 0.05883], + [DBActionType.PutTag, 0.05883], + [DBActionType.PatchTrack, 0.05883], + [DBActionType.PatchArtist, 0.05883], + [DBActionType.PatchAlbum, 0.05883], + [DBActionType.PatchTag, 0.05883], + [DBActionType.DeleteTrack, 0.05883], + [DBActionType.DeleteArtist, 0.05883], + [DBActionType.DeleteAlbum, 0.05883], + [DBActionType.DeleteTag, 0.05883], + [DBActionType.MergeTag, 0.05883], ]), userId: new Map([[1, 1.0]]), createTrackParams: { @@ -413,6 +422,10 @@ describe('Randomized model-based DB back-end tests', () => { deleteTagParams: { validId: new Map([[false, 0.2], [true, 0.8]]) }, + mergeTagParams: { + validFromId: new Map([[false, 0.2], [true, 0.8]]), + validToId: new Map([[false, 0.2], [true, 0.8]]), + } } // Loop to generate and execute a bunch of random actions. diff --git a/server/test/integration/helpers.ts b/server/test/integration/helpers.ts index a0d3ae2..43da605 100644 --- a/server/test/integration/helpers.ts +++ b/server/test/integration/helpers.ts @@ -248,6 +248,21 @@ export async function deleteTag( }); } +export async function mergeTag( + req: any, + fromId = 1, + toId = 2, + expectStatus: number | undefined = undefined, +) { + return await req + .post('/tag/' + fromId + '/merge/' + toId) + .send() + .then((res: any) => { + expectStatus && expect(res).to.have.status(expectStatus); + return res; + }); +} + export async function createAlbum( req: any, props = { name: "Album" }, diff --git a/server/test/reference_model/DBReferenceModel.ts b/server/test/reference_model/DBReferenceModel.ts index 5de36f2..b56717a 100644 --- a/server/test/reference_model/DBReferenceModel.ts +++ b/server/test/reference_model/DBReferenceModel.ts @@ -398,4 +398,65 @@ export function modifyTag(userId: number, id: number, updates: TagBaseWithRefs, [{ field: 'parentId', otherObjectType: 'tags', doReverseReference: false },], [], db); +} + +// Merge a tag into another. +// In effect, this means to re-tag any objects tagged with the 'from' tag +// with the 'to' tag instead, and then to delete the 'from' tag. +// If the tag has children, it may not be merged. +export function mergeTag(userId: number, fromId: number, toId: number, db: ReferenceDatabase): void { + // Existence checks + if (!(userId in db)) { + throw makeNotFoundError() + } + let fromTag = (db[userId].tags as any[]).find((o: any) => 'id' in o && o.id === fromId); + let toTag = (db[userId].tags as any[]).find((o: any) => 'id' in o && o.id === toId); + if (!fromTag || !toTag) { + throw makeNotFoundError() + } + + let getChildrenRecursive: (tag: TagWithRefsWithId) => number[] = (tag: TagWithRefsWithId) => { + let directChildren: TagWithRefsWithId[] = db[userId].tags + .filter((t: TagWithRefsWithId) => t.parentId === tag.id); + let indirectChildren: number[][] = directChildren.map((child: TagWithRefsWithId) => getChildrenRecursive(child)) + return [...(directChildren.map((t: TagWithRefsWithId) => t.id)), ...indirectChildren.flat()]; + } + let fromChildren = getChildrenRecursive(fromTag); + + // Conflict checks + if (fromId === toId) { + const e: DBError = { + name: "DBError", + kind: DBErrorKind.ResourceConflict, + message: 'Cannot merge a tag into itself.', + }; + throw e; + } + if (fromChildren.includes(toId)) { + const e: DBError = { + name: "DBError", + kind: DBErrorKind.ResourceConflict, + message: 'Cannot merge a tag into one of its children.', + }; + throw e; + } + + // Re-tag any objects tagged with the 'from' tag. + let objects: (TrackWithRefsWithId | ArtistWithRefsWithId | AlbumWithRefsWithId)[] = + [...db[userId].tracks, ...db[userId].albums, ...db[userId].artists]; + objects.forEach((object: TrackWithRefsWithId | ArtistWithRefsWithId | AlbumWithRefsWithId) => { + if(object.tagIds.includes(fromId)) { + object.tagIds = object.tagIds.filter((tid: number) => tid !== fromId); + ensureInSet(toId, object.tagIds); + } + }) + // Move any child tags under the "from" tag to the "to" tag. + db[userId].tags.forEach((t: TagWithRefsWithId) => { + if(t.parentId === fromId) { + t.parentId = toId; + } + }) + + // Delete the 'from' tag. + deleteTag(userId, fromId, db); } \ No newline at end of file diff --git a/server/test/reference_model/randomGen.ts b/server/test/reference_model/randomGen.ts index 3a7a19b..1096e5c 100644 --- a/server/test/reference_model/randomGen.ts +++ b/server/test/reference_model/randomGen.ts @@ -1,6 +1,6 @@ import { AlbumBaseWithRefs, AlbumWithRefs, AlbumWithRefsWithId, ArtistBaseWithRefs, ArtistWithRefs, ArtistWithRefsWithId, TagBaseWithRefs, TagWithRefs, TagWithRefsWithId, TrackBaseWithRefs, TrackWithRefs, TrackWithRefsWithId } from "../../../client/src/api/api"; import { userEndpoints } from "../../endpoints/User"; -import { createAlbum, createArtist, createTag, createTrack, deleteAlbum, deleteArtist, deleteTag, deleteTrack, modifyAlbum, modifyArtist, modifyTag, modifyTrack, ReferenceDatabase } from "./DBReferenceModel"; +import { createAlbum, createArtist, createTag, createTrack, deleteAlbum, deleteArtist, deleteTag, deleteTrack, mergeTag, modifyAlbum, modifyArtist, modifyTag, modifyTrack, ReferenceDatabase } from "./DBReferenceModel"; import * as helpers from '../integration/helpers'; import { DBErrorKind, isDBError } from "../../endpoints/types"; let _ = require('lodash'); @@ -22,6 +22,7 @@ export enum DBActionType { PatchAlbum = "PatchAlbum", PatchArtist = "PatchArtist", PatchTag = "PatchTag", + MergeTag = "MergeTag", } export interface DBAction { @@ -51,6 +52,7 @@ export interface RandomDBActionDistribution { putAlbumParams: RandomPutAlbumDistribution, putArtistParams: RandomPutArtistDistribution, putTagParams: RandomPutTagDistribution, + mergeTagParams: RandomMergeTagDistribution, } export interface RandomCreateTrackDistribution { @@ -133,6 +135,10 @@ export interface RandomPatchTagDistribution extends RandomPutTagDistribution { replaceParent: Distribution, validId: Distribution, } +export interface RandomMergeTagDistribution { + validFromId: Distribution, + validToId: Distribution, +} export interface RandomDeleteObjectDistribution { validId: Distribution, @@ -245,6 +251,13 @@ export function applyReferenceDBAction( status = 200; break; } + case DBActionType.MergeTag: + { + mergeTag(action.userId, action.payload.fromId, action.payload.toId, db); + response = {}, + status = 200; + break; + } } } catch (e) { if (isDBError(e)) { @@ -331,6 +344,13 @@ export async function applyRealDBAction( response = res.body; break; } + case DBActionType.MergeTag: + { + let res = await helpers.mergeTag(req, action.payload.fromId, action.payload.toId); + status = res.status; + response = res.body; + break; + } } return { response: response, status: status }; @@ -454,6 +474,21 @@ export function randomDBAction( userId: userId, } } + case DBActionType.MergeTag: { + let params = distribution.mergeTagParams; + let validIdx1 = Math.floor(randomNumGen() * db[userId].tags.length); + let validIdx2 = Math.floor(randomNumGen() * db[userId].tags.length); + let validId1 = db[userId].tags[validIdx1] ? db[userId].tags[validIdx1].id : 1; + let validId2 = db[userId].tags[validIdx2] ? db[userId].tags[validIdx2].id : 1; + return { + type: type, + payload: { + fromId: applyDistribution(params.validFromId, randomNumGen) ? validId1 : validId1 + 99999, + toId: applyDistribution(params.validToId, randomNumGen) ? validId2 : validId2 + 99999, + }, + userId: userId, + } + } } }