Tag merge working and tested.

.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": [

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<number[] | undefined> =
             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<number[] | undefined> =
             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.

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.

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.

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" },

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

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<boolean>,
     validId: Distribution<boolean>,
 }
+export interface RandomMergeTagDistribution {
+    validFromId: Distribution<boolean>,
+    validToId: Distribution<boolean>,
+}
 
 export interface RandomDeleteObjectDistribution {
     validId: Distribution<boolean>,
@@ -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,
+            }
+        }
     }
 }