Patch and put requests' tests are working, some bugs were fixed.

.vscode/launch.json

@@ -10,7 +10,7 @@
             "name": "Jasmine Tests with SQLite",
             "env": {
                 "MUDBASE_DB_CONFIG": "{\"client\": \"sqlite3\", \"connection\": \":memory:\"}",
-                "TEST_RANDOM_SEED": "0.77715"
+                "TEST_RANDOM_SEED": "0.04801"
             },
             "program": "${workspaceFolder}/server/node_modules/jasmine-ts/lib/index",
             "args": [

server/db/Tag.ts

@@ -5,14 +5,25 @@ import { TagBaseWithRefs, TagWithDetails, TagWithId, TagWithRefs, TagWithRefsWit
 import { DBError, DBErrorKind } from "../endpoints/types";
 import { makeNotFoundError } from "./common";
 
-export async function getTagChildrenRecursive(id: number, userId: number, trx: any): Promise<number[]> {
+export async function getTagChildrenRecursive(id: number,
+    userId: number,
+    trx: any,
+    visited: number[] = [], // internal, for cycle detection
+): Promise<number[]> {
+
+    // check for cycles, these are not allowed.
+    // a cycle would be if the same ID occurs more than once in the visited set.
+    if ((new Set<number>(visited)).size < visited.length) {
+        throw new Error('cyclic tag dependency')
+    }
+
     const directChildren = (await trx.select('id')
         .from('tags')
         .where({ 'user': userId })
         .where({ 'parentId': id })).map((r: any) => r.id);
 
     const indirectChildrenPromises = directChildren.map(
-        (child: number) => getTagChildrenRecursive(child, userId, trx)
+        (child: number) => getTagChildrenRecursive(child, userId, trx, [...visited, id])
     );
     const indirectChildrenNested = await Promise.all(indirectChildrenPromises);
     const indirectChildren = indirectChildrenNested.flat();
@@ -151,7 +162,7 @@ export async function modifyTag(userId: number, tagId: number, tag: TagBaseWithR
                 .from('tags')
                 .where({ 'user': userId })
                 .where({ 'id': tag.parentId })
-                .then((ts: any) => ts.map((t: any) => t['tagId'])) :
+                .then((ts: any) => ts.length ? ts.map((t: any) => t['tagId']) : null) :
             (async () => { return null })();
 
         // Start retrieving the tag itself.
@@ -161,13 +172,27 @@ export async function modifyTag(userId: number, tagId: number, tag: TagBaseWithR
             .where({ id: tagId })
             .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined)
 
+        // Start retrieving all current children. This is to prevent
+        // cycles.
+        const childrenPromise = getTagChildrenRecursive(tagId, userId, trx);
+
         // Wait for the requests to finish.
-        var [dbTag, parent] = await Promise.all([tagPromise, parentTagIdPromise]);
+        var [dbTag, parent, children] = await Promise.all([tagPromise, parentTagIdPromise, childrenPromise]);
+
+        // Check that modifying this will not cause a dependency cycle.
+        if (tag.parentId && [...children, tagId].includes(tag.parentId)) {
+            const e: DBError = {
+                name: "DBError",
+                kind: DBErrorKind.ResourceConflict,
+                message: 'Modifying this tag would cause a tag parent cycle.',
+            };
+            throw e;
+        }
 
         // Check that we found all objects we need.
         if ((tag.parentId && !parent) ||
             !dbTag) {
-                throw makeNotFoundError();
+            throw makeNotFoundError();
         }
 
         // Modify the tag.

server/db/Track.ts

@@ -24,7 +24,7 @@ export async function getTrack(id: number, userId: number, knex: Knex):
 
     const artistsPromise: Promise<api.ArtistWithId[]> =
         knex.select('artistId')
-            .from('artists_tracks')
+            .from('tracks_artists')
             .where({ 'trackId': id })
             .then((artists: any) => artists.map((artist: any) => artist['artistId']))
             .then((ids: number[]) =>
@@ -166,7 +166,7 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
         const artistIdsPromise: Promise<number[] | undefined> =
             track.artistIds ?
                 trx.select('artistId')
-                    .from('artists_tracks')
+                    .from('tracks_artists')
                     .whereIn('artistId', track.artistIds)
                     .then((as: any) => as.map((a: any) => a['artistId']))
                 : (async () => undefined)();
@@ -180,12 +180,25 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
                     .then((ts: any) => ts.map((t: any) => t['tagId'])) :
                 (async () => undefined)();
 
+        // Start retrieving album if we are modifying that.
+        const albumIdPromise =
+            track.albumId ?
+                trx.select('id')
+                    .from('albums')
+                    .where({ 'user': userId })
+                    .where({ id: track.albumId })
+                    .then((r: any) => (r && r[0]) ? r[0]['id'] : undefined) :
+        (async () => undefined)();
+
+        let blablums = await trx.select('id').from('albums');
+
         // Wait for the requests to finish.
-        var [oldTrack, artists, tags] = await Promise.all([trackIdPromise, artistIdsPromise, tagIdsPromise]);;
+        var [oldTrack, artists, tags, album] = await Promise.all([trackIdPromise, artistIdsPromise, tagIdsPromise, albumIdPromise]);;
 
         // Check that we found all objects we need.
         if ((!artists || !_.isEqual(artists.sort(), (track.artistIds || []).sort())) ||
             (!tags || !_.isEqual(tags.sort(), (track.tagIds || []).sort())) ||
+            (!album && track.albumId) ||
             !oldTrack) {
             throw makeNotFoundError();
         }
@@ -194,7 +207,7 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
         var update: any = {};
         if ("name" in track) { update["name"] = track.name; }
         if ("storeLinks" in track) { update["storeLinks"] = JSON.stringify(track.storeLinks || []); }
-        if ("albumId" in track) { update["albumId"] = track.albumId; }
+        if ("albumId" in track) { update["album"] = track.albumId; }
 
         const modifyTrackPromise = trx('tracks')
             .where({ 'user': userId })
@@ -202,7 +215,7 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
             .update(update)
 
         // Remove unlinked artists.
-        const removeUnlinkedArtists = artists ? trx('artists_tracks')
+        const removeUnlinkedArtists = artists ? trx('tracks_artists')
             .where({ 'trackId': trackId })
             .whereNotIn('artistId', track.artistIds || [])
             .delete() : undefined;
@@ -214,7 +227,7 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
             .delete() : undefined;
 
         // Link new artists.
-        const addArtists = artists ? trx('artists_tracks')
+        const addArtists = artists ? trx('tracks_artists')
             .where({ 'trackId': trackId })
             .then((as: any) => as.map((a: any) => a['artistId']))
             .then((doneArtistIds: number[]) => {
@@ -232,7 +245,7 @@ export async function modifyTrack(userId: number, trackId: number, track: TrackB
                 // Link them
                 return Promise.all(
                     insertObjects.map((obj: any) =>
-                        trx('artists_tracks').insert(obj)
+                        trx('tracks_artists').insert(obj)
                     )
                 );
             }) : undefined;
@@ -279,7 +292,7 @@ export async function deleteTrack(userId: number, trackId: number, knex: Knex):
         // FIXME remove
 
         let tracks = await trx.select('id', 'name')
-        .from('tracks');
+            .from('tracks');
         console.log("All tracks:", tracks);
 
         // Start by retrieving the track itself for sanity.

server/endpoints/Album.ts

@@ -8,10 +8,11 @@ import { GetArtist } from './Artist';
 
 export const GetAlbum: EndpointHandler = async (req: any, res: any, knex: Knex) => {
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   try {
     const maybeAlbum: api.GetAlbumResponse | null =
-      await getAlbum(req.params.id, userId, knex);
+      await getAlbum(id, userId, knex);
 
     if (maybeAlbum) {
       await res.send(maybeAlbum);
@@ -57,11 +58,12 @@ export const PutAlbum: EndpointHandler = async (req: any, res: any, knex: Knex)
   }
   const reqObject: api.PutAlbumRequest = req.body;
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   console.log("User ", userId, ": Put Album ", reqObject);
 
   try {
-    await modifyAlbum(userId, req.params.id, reqObject, knex);
+    await modifyAlbum(userId, id, reqObject, knex);
     res.status(200).send();
   } catch (e) {
     handleErrorsInEndpoint(e);
@@ -79,11 +81,12 @@ export const PatchAlbum: EndpointHandler = async (req: any, res: any, knex: Knex
   }
   const reqObject: api.PatchAlbumRequest = req.body;
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   console.log("User ", userId, ": Patch Album ", reqObject);
 
   try {
-    await modifyAlbum(userId, req.params.id, reqObject, knex);
+    await modifyAlbum(userId, id, reqObject, knex);
     res.status(200).send();
   } catch (e) {
     handleErrorsInEndpoint(e);
@@ -92,11 +95,12 @@ export const PatchAlbum: EndpointHandler = async (req: any, res: any, knex: Knex
 
 export const DeleteAlbum: EndpointHandler = async (req: any, res: any, knex: Knex) => {
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
-  console.log("User ", userId, ": Delete Album ", req.params.id);
+  console.log("User ", userId, ": Delete Album ", id);
 
   try {
-    await deleteAlbum(userId, req.params.id, knex);
+    await deleteAlbum(userId, id, knex);
     res.status(200).send();
 
   } catch (e) {

server/endpoints/Artist.ts

@@ -6,9 +6,10 @@ import { createArtist, deleteArtist, getArtist, modifyArtist } from '../db/Artis
 
 export const GetArtist: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
     try {
-        let artist = await getArtist(req.params.id, userId, knex);
+        let artist = await getArtist(id, userId, knex);
         await res.status(200).send(artist);
     } catch (e) {
         handleErrorsInEndpoint(e)
@@ -50,11 +51,12 @@ export const PutArtist: EndpointHandler = async (req: any, res: any, knex: Knex)
     }
     const reqObject: api.PutArtistRequest = req.body;
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
     console.log("User ", userId, ": Put Artist ", reqObject);
 
     try {
-        await modifyArtist(userId, req.params.id, reqObject, knex);
+        await modifyArtist(userId, id, reqObject, knex);
         res.status(200).send();
 
     } catch (e) {
@@ -73,11 +75,12 @@ export const PatchArtist: EndpointHandler = async (req: any, res: any, knex: Kne
     }
     const reqObject: api.PatchArtistRequest = req.body;
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
     console.log("User ", userId, ": Patch Artist ", reqObject);
 
     try {
-        await modifyArtist(userId, req.params.id, reqObject, knex);
+        await modifyArtist(userId, id, reqObject, knex);
         res.status(200).send();
 
     } catch (e) {
@@ -87,11 +90,12 @@ export const PatchArtist: EndpointHandler = async (req: any, res: any, knex: Kne
 
 export const DeleteArtist: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
-    console.log("User ", userId, ": Delete Artist ", req.params.id);
+    console.log("User ", userId, ": Delete Artist ", id);
 
     try {
-        await deleteArtist(userId, req.params.id, knex);
+        await deleteArtist(userId, id, knex);
         res.status(200).send();
 
     } catch (e) {

server/endpoints/Integration.ts

@@ -33,7 +33,8 @@ export const PostIntegration: EndpointHandler = async (req: any, res: any, knex:
 
 export const GetIntegration: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     try {
-        let integration = await getIntegration(req.user.id, req.params.id, knex);
+        let id = parseInt(req.params.id);
+        let integration = await getIntegration(req.user.id, id, knex);
         res.status(200).send(integration);
     } catch (e) {
         handleErrorsInEndpoint(e)
@@ -54,11 +55,12 @@ export const ListIntegrations: EndpointHandler = async (req: any, res: any, knex
 
 export const DeleteIntegration: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
-    console.log("User ", userId, ": Delete Integration ", req.params.id);
+    console.log("User ", userId, ": Delete Integration ", id);
 
     try {
-        await deleteIntegration(userId, req.params.id, knex);
+        await deleteIntegration(userId, id, knex);
         res.status(200).send();
 
     } catch (e) {
@@ -77,11 +79,12 @@ export const PutIntegration: EndpointHandler = async (req: any, res: any, knex:
     }
     const reqObject: api.PutIntegrationRequest = req.body;
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
     console.log("User ", userId, ": Put Integration ", reqObject);
 
     try {
-        await modifyIntegration(userId, req.params.id, reqObject, knex);
+        await modifyIntegration(userId, id, reqObject, knex);
         res.status(200).send();
 
     } catch (e) {
@@ -102,9 +105,10 @@ export const PatchIntegration: EndpointHandler = async (req: any, res: any, knex
     const { id: userId } = req.user;
 
     console.log("User ", userId, ": Patch Integration ", reqObject);
+    let id = parseInt(req.params.id);
 
     try {
-        await modifyIntegration(userId, req.params.id, reqObject, knex);
+        await modifyIntegration(userId, id, reqObject, knex);
         res.status(200).send();
 
     } catch (e) {

server/endpoints/Tag.ts

@@ -33,11 +33,12 @@ export const PostTag: EndpointHandler = async (req: any, res: any, knex: Knex) =
 
 export const DeleteTag: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
-    console.log("User ", userId, ": Delete Tag ", req.params.id);
+    let id = parseInt(req.params.id);
+    console.log("User ", userId, ": Delete Tag ", id);
 
     try {
 
-        await deleteTag(userId, req.params.id, knex);
+        await deleteTag(userId, id, knex);
         res.status(200).send();
 
     } catch (e) {
@@ -47,8 +48,9 @@ export const DeleteTag: EndpointHandler = async (req: any, res: any, knex: Knex)
 
 export const GetTag: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
     try {
-        let tag = await getTag(req.params.id, userId, knex);
+        let tag = await getTag(id, userId, knex);
         await res.status(200).send(tag);
     } catch (e) {
         handleErrorsInEndpoint(e)
@@ -66,11 +68,12 @@ export const PutTag: EndpointHandler = async (req: any, res: any, knex: Knex) =>
     }
     const reqObject: api.PutTagRequest = req.body;
     const { id: userId } = req.user;
+    let id: number = parseInt(req.params.id);
 
     console.log("User ", userId, ": Put Tag ", reqObject);
 
     try {
-        await modifyTag(userId, req.params.id, reqObject, knex);
+        await modifyTag(userId, id, reqObject, knex);
         res.status(200).send();
     } catch (e) {
         handleErrorsInEndpoint(e);
@@ -88,11 +91,12 @@ export const PatchTag: EndpointHandler = async (req: any, res: any, knex: Knex)
     }
     const reqObject: api.PatchTagRequest = req.body;
     const { id: userId } = req.user;
+    let id = parseInt(req.params.id);
 
     console.log("User ", userId, ": Patch Tag ", reqObject);
 
     try {
-        await modifyTag(userId, req.params.id, reqObject, knex);
+        await modifyTag(userId, id, reqObject, knex);
         res.status(200).send();
     } catch (e) {
         handleErrorsInEndpoint(e);
@@ -102,9 +106,10 @@ export const PatchTag: EndpointHandler = async (req: any, res: any, knex: Knex)
 export const MergeTag: EndpointHandler = async (req: any, res: any, knex: Knex) => {
     const { id: userId } = req.user;
 
-    console.log("User ", userId, ": Merge Tag ", req.params.id, req.params.toId);
-    const fromId = req.params.id;
-    const toId = req.params.toId;
+    const fromId = parseInt(req.params.id);
+    const toId = parseInt(req.params.toId);
+
+    console.log("User ", userId, ": Merge Tag ", fromId, req.params.toId);
 
     try {
         await mergeTag(userId, fromId, toId, knex);

server/endpoints/Track.ts

@@ -31,9 +31,10 @@ export const PostTrack: EndpointHandler = async (req: any, res: any, knex: Knex)
 
 export const GetTrack: EndpointHandler = async (req: any, res: any, knex: Knex) => {
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   try {
-    let track = await getTrack(req.params.id, userId, knex);
+    let track = await getTrack(id, userId, knex);
     await res.status(200).send(track);
   } catch (e) {
     handleErrorsInEndpoint(e)
@@ -51,11 +52,12 @@ export const PutTrack: EndpointHandler = async (req: any, res: any, knex: Knex)
   }
   const reqObject: api.PutTrackRequest = req.body;
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   console.log("User ", userId, ": Put Track ", reqObject);
 
   try {
-    await modifyTrack(userId, req.params.id, reqObject, knex);
+    await modifyTrack(userId, id, reqObject, knex);
     res.status(200).send();
   } catch (e) {
     handleErrorsInEndpoint(e);
@@ -73,11 +75,12 @@ export const PatchTrack: EndpointHandler = async (req: any, res: any, knex: Knex
   }
   const reqObject: api.PatchTrackRequest = req.body;
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
   console.log("User ", userId, ": Patch Track ", reqObject);
 
   try {
-    await modifyTrack(userId, req.params.id, reqObject, knex);
+    await modifyTrack(userId, id, reqObject, knex);
     res.status(200).send();
   } catch (e) {
     handleErrorsInEndpoint(e);
@@ -86,11 +89,12 @@ export const PatchTrack: EndpointHandler = async (req: any, res: any, knex: Knex
 
 export const DeleteTrack: EndpointHandler = async (req: any, res: any, knex: Knex) => {
   const { id: userId } = req.user;
+  let id = parseInt(req.params.id);
 
-  console.log("User ", userId, ": Delete Track ", req.params.id);
+  console.log("User ", userId, ": Delete Track ", id);
 
   try {
-      await deleteTrack(userId, req.params.id, knex);
+      await deleteTrack(userId, id, knex);
       res.status(200).send();
 
   } catch (e) {

server/test/integration/flows/ResourceFlow.ts

@@ -75,21 +75,21 @@ function normalizeDB(oldDb: ReferenceDatabase) {
         db[userId].tags.forEach((x: TagWithRefsWithId) => { x.id = remapId(x.id, tagMap); })
     }
     for (const userId in db) {
-        // Now remap the references.
+        // Now remap and sort the references.
         db[userId].tracks.forEach((x: TrackWithRefsWithId) => {
-            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap));
-            x.artistIds = x.artistIds.map((id: number) => remapId(id, artistMap));
+            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap)).sort();
+            x.artistIds = x.artistIds.map((id: number) => remapId(id, artistMap)).sort();
             x.albumId = x.albumId ? remapId(x.albumId, albumMap) : null;
         });
         db[userId].albums.forEach((x: AlbumWithRefsWithId) => {
-            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap));
-            x.artistIds = x.artistIds.map((id: number) => remapId(id, artistMap));
-            x.trackIds = x.trackIds.map((id: number) => remapId(id, trackMap));
+            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap)).sort();
+            x.artistIds = x.artistIds.map((id: number) => remapId(id, artistMap)).sort();
+            x.trackIds = x.trackIds.map((id: number) => remapId(id, trackMap)).sort();
         });
         db[userId].artists.forEach((x: ArtistWithRefsWithId) => {
-            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap));
-            x.albumIds = x.albumIds.map((id: number) => remapId(id, albumMap));
-            x.trackIds = x.trackIds.map((id: number) => remapId(id, trackMap));
+            x.tagIds = x.tagIds.map((id: number) => remapId(id, tagMap)).sort();
+            x.albumIds = x.albumIds.map((id: number) => remapId(id, albumMap)).sort();
+            x.trackIds = x.trackIds.map((id: number) => remapId(id, trackMap)).sort();
         });
         db[userId].tags.forEach((x: TagWithRefsWithId) => {
             x.parentId = x.parentId ? remapId(x.parentId, tagMap) : null;
@@ -118,32 +118,48 @@ function transformActionIDs(action: DBAction, mappings: IDMappings, rng: any) {
     }
 
     switch (r.type) {
-        case DBActionType.CreateTrack: {
-            let track = r.payload as TrackWithRefsWithId;
-            track.tagIds.forEach((id: number) => doMap(id, mappings.tags));
-            track.artistIds.forEach((id: number) => doMap(id, mappings.artists));
-            track.albumId = track.albumId ? doMap(track.albumId, mappings.albums) : null;
-            break;
-        }
-        case DBActionType.CreateArtist: {
-            let artist = r.payload as ArtistWithRefsWithId;
-            artist.tagIds.forEach((id: number) => doMap(id, mappings.tags));
-            artist.albumIds.forEach((id: number) => doMap(id, mappings.albums));
-            artist.trackIds.forEach((id: number) => doMap(id, mappings.tracks));
-            break;
-        }
-        case DBActionType.CreateAlbum: {
-            let album = r.payload as AlbumWithRefsWithId;
-            album.tagIds.forEach((id: number) => doMap(id, mappings.tags));
-            album.artistIds.forEach((id: number) => doMap(id, mappings.artists));
-            album.trackIds.forEach((id: number) => doMap(id, mappings.tracks));
-            break;
-        }
-        case DBActionType.CreateTag: {
-            let tag = r.payload as TagWithRefsWithId;
-            tag.parentId = tag.parentId ? doMap(tag.parentId, mappings.tags) : null;
-            break;
-        }
+        case DBActionType.CreateTrack:
+        case DBActionType.PutTrack:
+        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));
+                if (track.albumId) { track.albumId = doMap(track.albumId, mappings.albums); }
+                if (track.id) { track.id = doMap(track.id, mappings.tracks); }
+                break;
+            }
+        case DBActionType.CreateArtist:
+        case DBActionType.PutArtist:
+        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));
+                if (artist.id) { artist.id = doMap(artist.id, mappings.artists); }
+                break;
+            }
+        case DBActionType.CreateAlbum:
+        case DBActionType.PutAlbum:
+        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));
+                if (album.id) { album.id = doMap(album.id, mappings.albums); }
+                break;
+            }
+        case DBActionType.CreateTag:
+        case DBActionType.PutTag:
+        case DBActionType.PatchTag:
+            {
+                let tag = r.payload as TagWithRefsWithId;
+                if (tag.parentId) { tag.parentId = doMap(tag.parentId, mappings.tags); }
+                if (tag.id) { tag.id = doMap(tag.id, mappings.tags); }
+                break;
+            }
         case DBActionType.DeleteTrack:
         case DBActionType.DeleteArtist:
         case DBActionType.DeleteAlbum:

server/test/reference_model/DBReferenceModel.ts

@@ -1,5 +1,6 @@
 import { AlbumBaseWithRefs, AlbumWithRefsWithId, ArtistBaseWithRefs, ArtistWithRefsWithId, DBDataFormat, PostAlbumRequest, PostArtistRequest, PostTagRequest, PostTrackRequest, TagBaseWithRefs, TagWithRefsWithId, TrackBaseWithRefs, TrackWithDetails, TrackWithRefsWithId } from "../../../client/src/api/api";
 import { makeNotFoundError } from "../../db/common";
+import { DBError, DBErrorKind } from "../../endpoints/types";
 import filterInPlace from "../../lib/filterInPlace";
 let _ = require('lodash');
 
@@ -29,7 +30,7 @@ function checkExists(objects: any[], ids: number[]) {
 
 // If not in the array, put the number in the array.
 function ensureInSet(n: number, s: number[]) {
-    if (!(n in s)) { s.push(n); }
+    if (!s.includes(n)) { s.push(n); }
 }
 
 // For a set of objects, ensure they point to another object.
@@ -302,11 +303,9 @@ export function deleteAlbum(userId: number, id: number, db: ReferenceDatabase):
 }
 
 // Delete a tag.
-export function deleteTag(userId: number, id: number, db: ReferenceDatabase, recursiveIdsSoFar: number[] = []): void {
+export function deleteTag(userId: number, id: number, db: ReferenceDatabase): void {
     // Tags are special in that deleting them also deletes their children. Implement that here
     // with recursive calls.
-    let _recursiveIdsSoFar = [...recursiveIdsSoFar, id]
-
     if (!(userId in db)) { throw makeNotFoundError() }
     let tag = db[userId].tags.find((o: any) => 'id' in o && o.id === id);
     if (!tag) {
@@ -314,10 +313,7 @@ export function deleteTag(userId: number, id: number, db: ReferenceDatabase, rec
     }
     let children = db[userId].tags.filter((t: TagWithRefsWithId) => t.parentId === id);
     children.forEach((child: TagWithRefsWithId) => {
-        // Prevent cyclic dependencies.
-        if (!_recursiveIdsSoFar.includes(child.id)) {
-            deleteTag(userId, child.id, db, _recursiveIdsSoFar)
-        }
+        deleteTag(userId, child.id, db)
     })
 
     // Do the actual deletion of this tag.
@@ -372,6 +368,32 @@ export function modifyAlbum(userId: number, id: number, updates: AlbumBaseWithRe
 
 // Modify a tag.
 export function modifyTag(userId: number, id: number, updates: TagBaseWithRefs, db: ReferenceDatabase): void {
+    // A special case for tracks is that we do not allow "parent cycles", which could happen
+    // through modification. Catch that here.
+    if (!(userId in db)) {
+        throw makeNotFoundError()
+    }
+    let tag = (db[userId].tags as any[]).find((o: any) => 'id' in o && o.id === id);
+    if (!tag) {
+        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()];
+    }
+
+    if (updates.parentId && [...getChildrenRecursive(tag), id].includes(updates.parentId)) {
+        const e: DBError = {
+            name: "DBError",
+            kind: DBErrorKind.ResourceConflict,
+            message: 'Modifying this tag would cause a tag parent cycle.',
+        };
+        throw e;
+    }
+
     return modifyObject(userId, id, updates, 'tags',
         [{ field: 'parentId', otherObjectType: 'tags', doReverseReference: false },],
         [],

server/test/reference_model/randomGen.ts

@@ -251,6 +251,9 @@ export function applyReferenceDBAction(
             if (e.kind === DBErrorKind.ResourceNotFound) {
                 status = 404;
                 response = {};
+            } else if(e.kind === DBErrorKind.ResourceConflict) {
+                status = 409;
+                response = {};
             }
         } else {
             throw e;
@@ -422,7 +425,7 @@ export function randomDBAction(
                         DBActionType.PatchTrack | DBActionType.PatchTag | DBActionType.PatchAlbum | DBActionType.PatchArtist
                         | DBActionType.PutTrack | DBActionType.PutTag | DBActionType.PutAlbum | DBActionType.PutArtist
                     let validIdx = Math.floor(randomNumGen() * objectArrays[_type].length);
-                    let validId = objectArrays[_type][validIdx].id;
+                    let validId = objectArrays[_type][validIdx] ? objectArrays[_type][validIdx].id : 1;
                     object.payload.id = applyDistribution(params[_type].validId, randomNumGen) ? validId : validId + 99999;
                 }
                 return object;
@@ -444,7 +447,7 @@ export function randomDBAction(
                 [DBActionType.DeleteTag]: db[userId].tags,
             }
             let validIdx = Math.floor(randomNumGen() * objectArrays[type].length);
-            let validId = objectArrays[type][validIdx].id;
+            let validId = objectArrays[type][validIdx] ? objectArrays[type][validIdx].id : 1;
             return {
                 type: type,
                 payload: applyDistribution(params[type].validId, randomNumGen) ? validId : validId + 99999,