Almost there with testing, some concurrency issues

client/src/api.ts

@@ -22,7 +22,7 @@ export enum SongQueryFilterOp {
 export enum SongQueryElemProperty {
     id = "id",
     artistIds = "artistIds",
-    albumIds = "albumIds"
+    albumIds = "albumIds",
 }
 export interface SongQueryElem {
     prop?: SongQueryElemProperty,

server/endpoints/CreateSongEndpointHandler.ts

@@ -1,6 +1,7 @@
 const models = require('../models');
 import * as api from '../../client/src/api';
 import { EndpointError, EndpointHandler, catchUnhandledErrors } from './types';
+const { Op } = require("sequelize");
 
 export const CreateSongEndpointHandler: EndpointHandler = async (req: any, res: any) => {
   if (!api.checkCreateSongRequest(req)) {
@@ -13,68 +14,49 @@ export const CreateSongEndpointHandler: EndpointHandler = async (req: any, res:
   const reqObject: api.CreateSongRequest = req.body;
 
   // Start retrieving the artist instances to link the song to.
-  var artistInstancePromises: Promise<any>[] = [];
+  var artistInstancesPromise = reqObject.artistIds && models.Artist.findAll({
-  reqObject.artistIds?.forEach((artistId: Number) => {
+    where: {
-    artistInstancePromises.push(
+      id: {
-      models.Artist.findAll({
+        [Op.in]: reqObject.artistIds
-        where: { id: artistId }
+      }
-      })
-        .then((artist: any[]) => {
-          if (artist.length != 1) {
-            const e: EndpointError = {
-              internalMessage: 'There is no artist with id ' + artistId + '.',
-              httpStatus: 400
-            };
-            throw e;
     }
-          return artist[0];
-        })
-    );
   });
-  var artistInstancesPromise = Promise.all(artistInstancePromises);
 
   // Start retrieving the album instances to link the song to.
-  var albumInstancePromises: Promise<any>[] = [];
+  var albumInstancesPromise = reqObject.albumIds && models.Album.findAll({
-  reqObject.albumIds?.forEach((albumId: Number) => {
+    where: {
-    albumInstancePromises.push(
+      id: {
-      models.Album.findAll({
+        [Op.in]: reqObject.albumIds
-        where: { id: albumId }
+      }
-      })
-        .then((album: any[]) => {
-          if (album.length != 1) {
-            const e: EndpointError = {
-              internalMessage: 'There is no album with id ' + albumId + '.',
-              httpStatus: 400
-            };
-            throw e;
     }
-          return album[0];
-        })
-    );
   });
-  var albumInstancesPromise = Promise.all(albumInstancePromises);
 
   // Upon finish retrieving artists and albums, create the song and associate it.
   await Promise.all([artistInstancesPromise, albumInstancesPromise])
     .then((values: any) => {
       var [artists, albums] = values;
-      models.Song.create({
+
+      if ((reqObject.artistIds && artists.length !== reqObject.artistIds.length) ||
+        (reqObject.albumIds && albums.length !== reqObject.albumIds.length)) {
+        const e: EndpointError = {
+          internalMessage: 'Not all albums and/or artists exist for CreateSong request: ' + JSON.stringify(req.body),
+          httpStatus: 400
+        };
+        throw e;
+      }
+
+      var song = models.Song.build({
         title: reqObject.title,
+      });
+      artists && song.addArtists(artists);
+      albums && song.addAlbums(albums);
+      return song.save();
     })
-        .then(Promise.all([
-          (song: any) => {
-            song.addArtists(artists);
-          },
-          (song: any) => {
-            song.addAlbums(albums);
-          }
-        ]))
     .then((song: any) => {
       const responseObject: api.CreateSongResponse = {
         id: song.id
       };
       res.status(200).send(responseObject);
     })
-    })
     .catch(catchUnhandledErrors);
 }

server/endpoints/QuerySongsEndpointHandler.ts

@@ -3,42 +3,45 @@ const { Op } = require("sequelize");
 import * as api from '../../client/src/api';
 import { EndpointError, EndpointHandler, catchUnhandledErrors } from './types';
 
-const getSequelizeWhere = (queryElem: api.SongQueryElem) => {
+const sequelizeOps: any = {
-    var and = [];
+    [api.SongQueryFilterOp.Eq]: Op.eq,
+    [api.SongQueryFilterOp.Ne]: Op.ne,
+    [api.SongQueryFilterOp.In]: Op.in,
+    [api.SongQueryFilterOp.NotIn]: Op.notIn,
+    [api.SongQueryElemOp.And]: Op.and,
+    [api.SongQueryElemOp.Or]: Op.or,
+};
 
-    var sequelizeOps:any = {};
+const sequelizeProps: any = {
-    sequelizeOps[api.SongQueryFilterOp.Eq] = Op.eq;
+    [api.SongQueryElemProperty.id]: "id",
-    sequelizeOps[api.SongQueryFilterOp.Ne] = Op.ne;
+    [api.SongQueryElemProperty.artistIds]: "$Artists.id$",
-    sequelizeOps[api.SongQueryFilterOp.In] = Op.in;
+    [api.SongQueryElemProperty.albumIds]: "$Albums.id$",
-    sequelizeOps[api.SongQueryFilterOp.NotIn] = Op.notIn;
+};
-    sequelizeOps[api.SongQueryElemOp.And] = Op.and;
-    sequelizeOps[api.SongQueryElemOp.Or] = Op.or;
 
-    var sequelizeProps:any = {};
+// Returns the "where" clauses for Sequelize, per object type.
-    sequelizeProps[api.SongQueryElemProperty.id] = "id";
+const getSequelizeWhere = (queryElem: api.SongQueryElem) => {
-    sequelizeProps[api.SongQueryElemProperty.artistIds] = "artistIds";
+    var where: any = {
-    sequelizeProps[api.SongQueryElemProperty.albumIds] = "albumIds";
+        [Op.and]: []
+    };
 
     if (queryElem.prop && queryElem.propOperator && queryElem.propOperand) {
-        const prop = sequelizeProps[queryElem.prop];
+        // Visit a filter-like subquery leaf.
-        const op = sequelizeOps[queryElem.propOperator];
+        where[Op.and].push({
-        var filter:any = {};
+            [sequelizeProps[queryElem.prop]]: {
-        filter[op] = queryElem.propOperand;
+                [sequelizeOps[queryElem.propOperator]]: queryElem.propOperand
-        var where:any = {};
+            }
-        where[prop] = filter;
+        });
-        and.push(where);
     }
     if (queryElem.childrenOperator && queryElem.children) {
+        // Recursively visit a nested subquery.
+
         const children = queryElem.children.map((child: api.SongQueryElem) => getSequelizeWhere(child));
-        const op = sequelizeOps[queryElem.childrenOperator];
+        where[Op.and].push({
-        var where:any = {};
+            [sequelizeOps[queryElem.childrenOperator]]: children
-        where[op] = children;
+        });
-        and.push(where)
     }
 
-    return {
+    return where;
-        [Op.and]: and
-    };
 }
 
 export const QuerySongsEndpointHandler: EndpointHandler = async (req: any, res: any) => {
@@ -51,8 +54,11 @@ export const QuerySongsEndpointHandler: EndpointHandler = async (req: any, res:
     }
     const reqObject: api.QuerySongsRequest = req.body;
 
+    console.log("Query with where: ", getSequelizeWhere(reqObject.query));
+
     await models.Song.findAll({
-        where: getSequelizeWhere(reqObject.query)
+        where: getSequelizeWhere(reqObject.query),
+        include: [models.Artist, models.Album]
     })
         .then((songs: any[]) => {
             const response: api.QuerySongsResponse = {

server/test/integration/flows/CreateArtistFlow.js

@@ -4,6 +4,7 @@ const express = require('express');
 const models = require('../../../models');
 import { SetupApp } from '../../../app';
 import { expect } from 'chai';
+import * as helpers from './helpers';
 
 async function init() {
     chai.use(chaiHttp);

server/test/integration/flows/CreateSongFlow.js

@@ -4,6 +4,7 @@ const express = require('express');
 const models = require('../../../models');
 import { SetupApp } from '../../../app';
 import { expect } from 'chai';
+import * as helpers from './helpers';
 
 async function init() {
     chai.use(chaiHttp);
@@ -25,13 +26,13 @@ describe('POST /song with no title', () => {
                     expect(res).to.have.status(400);
                     done();
                 });
-        });
+        })
     });
 });
 
 describe('POST /song with only a title', () => {
     it('should return the first available id', done => {
-        init().then((app) => {
+        init().then(async(app) => {
             chai
                 .request(app)
                 .post('/song')
@@ -52,7 +53,7 @@ describe('POST /song with only a title', () => {
 
 describe('POST /song with a nonexistent artist Id', () => {
     it('should fail', done => {
-        init().then((app) => {
+        init().then(async (app) => {
             chai
                 .request(app)
                 .post('/song')
@@ -72,38 +73,10 @@ describe('POST /song with a nonexistent artist Id', () => {
 describe('POST /song with an existing artist Id', () => {
     it('should succeed', done => {
         init().then((app) => {
-            async function createArtist() {
+            var req = chai.request(app).keepOpen();
-                await chai.request(app)
+            helpers.createArtist(req, { name: "MyArtist" }, 200, { id: 1 })
-                    .post('/artist')
+                .then(() => { helpers.createSong(req, { name: "MySong" }, 200, { id: 1 }) })
-                    .send({
+                .then(req.close)
-                        name: "MyArtist"
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: 1
-                        });
-                    });
-            }
-
-            async function createSong() {
-                chai.request(app)
-                    .post('/song')
-                    .send({
-                        title: "MySong",
-                        artistIds: [1]
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: 1
-                        });
-                    });
-            }
-
-            init()
-                .then(createArtist)
-                .then(createSong)
                 .then(done);
         });
     });
@@ -112,39 +85,11 @@ describe('POST /song with an existing artist Id', () => {
 describe('POST /song with two existing artist Ids', () => {
     it('should succeed', done => {
         init().then((app) => {
-            async function createArtist(name, expectId) {
+            var req = chai.request(app).keepOpen();
-                await chai.request(app)
+            helpers.createArtist(req, { name: "Artist1" }, 200, { id: 1 })
-                    .post('/artist')
+                .then(() => { helpers.createArtist(req, { name: "Artist2" }, 200, { id: 2 }) })
-                    .send({
+                .then(() => { helpers.createSong(req, { name: "MySong", artistIds: [1, 2] }, 200, { id: 1 }) })
-                        name: name
+                .then(req.close)
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: expectId
-                        });
-                    });
-            }
-
-            async function createSong() {
-                chai.request(app)
-                    .post('/song')
-                    .send({
-                        title: "MySong",
-                        artistIds: [1, 2]
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: 1
-                        });
-                    });
-            }
-
-            init()
-                .then(() => { createArtist('Artist1', 1); })
-                .then(() => { createArtist('Artist2', 2); })
-                .then(createSong)
                 .then(done);
         });
     });
@@ -153,35 +98,10 @@ describe('POST /song with two existing artist Ids', () => {
 describe('POST /song with an existent and a nonexistent artist Id', () => {
     it('should fail', done => {
         init().then((app) => {
-            async function createArtist(name, expectId) {
+            var req = chai.request(app).keepOpen();
-                await chai.request(app)
+            helpers.createArtist(req, { name: "Artist1" }, 200, { id: 1 })
-                    .post('/artist')
+                .then(() => { helpers.createSong(req, { name: "MySong", artistIds: [1, 2] }, 400) })
-                    .send({
+                .then(req.close)
-                        name: name
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: expectId
-                        });
-                    });
-            }
-
-            async function createSong() {
-                chai.request(app)
-                    .post('/song')
-                    .send({
-                        title: "MySong",
-                        artistIds: [1, 2]
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(400);
-                    });
-            }
-
-            init()
-                .then(() => { createArtist('Artist1', 1); })
-                .then(createSong)
                 .then(done);
         });
     });

server/test/integration/flows/ModifyArtistFlow.js

@@ -4,6 +4,7 @@ const express = require('express');
 const models = require('../../../models');
 import { SetupApp } from '../../../app';
 import { expect } from 'chai';
+import * as helpers from './helpers';
 
 async function init() {
     chai.use(chaiHttp);
@@ -25,58 +26,20 @@ describe('PUT /artist on nonexistent artist', () => {
                 })
                 .then((res) => {
                     expect(res).to.have.status(400);
+                    done();
+                })
         })
-                .then(done)
-        });
     });
 });
 
 describe('PUT /artist with an existing artist', () => {
     it('should succeed', done => {
         init().then((app) => {
-            async function createArtist(req) {
-                await req
-                    .post('/artist')
-                    .send({
-                        name: "MyArtist"
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            id: 1
-                        });
-                    });
-            }
-
-            async function modifyArtist(req) {
-                await req
-                    .put('/artist/1')
-                    .send({
-                        name: "MyNewArtist",
-                        id: 1
-                    })
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                    });
-            }
-
-            async function checkArtist(req) {
-                await req
-                    .get('/artist/1')
-                    .then((res) => {
-                        expect(res).to.have.status(200);
-                        expect(res.body).to.deep.equal({
-                            name: "MyNewArtist"
-                        });
-                    })
-            }
-
             var req = chai.request(app).keepOpen();
-
+            helpers.createArtist(req, { name: "MyArtist" }, 200, { id: 1 })
-            createArtist(req)
+                .then(() => { helpers.modifyArtist(req, 1, { name: "MyNewArtist" }, 200) })
-                .then(() => modifyArtist(req))
+                .then(() => { helpers.checkArtist(req, 1, 200, { name: "MyNewArtist" } )})
-                .then(() => checkArtist(req))
+                .then(req.close)
-                .then(() => req.close())
                 .then(done);
         });
     });

server/test/integration/flows/QuerySongsFlow.js

@@ -4,6 +4,7 @@ const express = require('express');
 const models = require('../../../models');
 import { SetupApp } from '../../../app';
 import { expect } from 'chai';
+import * as helpers from './helpers';
 
 async function init() {
     chai.use(chaiHttp);
@@ -28,46 +29,123 @@ describe('POST /song/query with no songs', () => {
                     expect(res.body).to.deep.equal({
                         ids: []
                     });
-                    done();
-                });
                 });
+        })
+        .then(done);
     });
 });
 
-describe('POST /song/query with several songs', () => {
+describe('POST /song/query with several songs and filters', () => {
-    it('should give empty list', done => {
+    it('should give all correct results', done => {
         init().then((app) => {
-            async function createSong(req) {
+            async function checkAllSongs(req) {
                 await req
-                    .post('/song')
+                    .post('/song/query')
+                    .send({ "query": {} })
+                    .then((res) => {
+                        expect(res).to.have.status(200);
+                        expect(res.body).to.deep.equal({
+                            ids: [1, 2, 3]
+                        });
+                    });
+            }
+
+            async function checkIdIn(req) {
+                await req
+                    .post('/song/query')
                     .send({
-                        title: "Song"
+                        "query": {
+                            "prop": "id",
+                            "propOperator": "IN",
+                            "propOperand": [1, 3, 5]
+                        }
                     })
                     .then((res) => {
                         expect(res).to.have.status(200);
+                        expect(res.body).to.deep.equal({
+                            ids: [1, 3]
+                        });
                     });
             }
 
-            async function checkSongs(req) {
+            async function checkIdNotIn(req) {
                 await req
                     .post('/song/query')
-                    .send({ "query": {} })
+                    .send({
+                        "query": {
+                            "prop": "id",
+                            "propOperator": "NOTIN",
+                            "propOperand": [1, 3, 5]
+                        }
+                    })
                     .then((res) => {
                         expect(res).to.have.status(200);
                         expect(res.body).to.deep.equal({
-                            ids: [1, 2, 3]
+                            ids: [2]
+                        });
+                    });
+            }
+
+            async function checkArtistIdIn(req) {
+                await req
+                    .post('/song/query')
+                    .send({
+                        "query": {
+                            "prop": "artistIds",
+                            "propOperator": "IN",
+                            "propOperand": [1]
+                        }
+                    })
+                    .then((res) => {
+                        expect(res).to.have.status(200);
+                        expect(res.body).to.deep.equal({
+                            ids: [1, 2]
+                        });
+                    });
+            }
+
+            async function checkOrRelation(req) {
+                await req
+                    .post('/song/query')
+                    .send({
+                        "query": {
+                            "childrenOperator": "OR",
+                            "children": [
+                                {
+                                    "prop": "artistIds",
+                                    "propOperator": "IN",
+                                    "propOperand": [2]
+                                },
+                                {
+                                    "prop": "id",
+                                    "propOperator": "EQ",
+                                    "propOperand": 1
+                                }
+                            ]
+                        }
+                    })
+                    .then((res) => {
+                        expect(res).to.have.status(200);
+                        expect(res.body).to.deep.equal({
+                            ids: [1, 3]
                         });
                     });
             }
 
             var req = chai.request(app).keepOpen();
 
-            createSong(req)
+            helpers.createArtist(req, { name: "Artist1" }, 200)
-                .then(() => createSong(req))
+                .then(() => helpers.createArtist(req, { name: "Artist2" }, 200))
-                .then(() => createSong(req))
+                .then(() => helpers.createSong(req, { title: "Song1", artistIds: [1] }, 200))
-                .then(() => checkSongs(req))
+                .then(() => helpers.createSong(req, { title: "Song2", artistIds: [1] }, 200))
-                .then(() => req.close())
+                .then(() => helpers.createSong(req, { title: "Song3", artistIds: [2] }, 200))
+                .then(() => checkAllSongs(req))
+                .then(() => checkIdIn(req))
+                .then(() => checkIdNotIn(req))
+                .then(() => checkArtistIdIn(req))
+                .then(() => checkOrRelation(req))
+                .then(req.close)
                 .then(done)
-        });
+        })
     });
 });

server/test/integration/flows/helpers.js

@@ -0,0 +1,59 @@
+import { expect } from "chai";
+
+export async function createSong(
+    req,
+    props = { title: "Song" },
+    expectStatus = undefined,
+    expectResponse = undefined
+) {
+    await req
+        .post('/song')
+        .send(props)
+        .then((res) => {
+            expectStatus && expect(res).to.have.status(expectStatus);
+            expectResponse && expect(res.body).to.deep.equal(expectResponse);
+        });
+}
+
+export async function createArtist(
+    req,
+    props = { name: "Artist" },
+    expectStatus = undefined,
+    expectResponse = undefined
+) {
+    await req
+        .post('/artist')
+        .send(props)
+        .then((res) => {
+            expectStatus && expect(res).to.have.status(expectStatus);
+            expectResponse && expect(res.body).to.deep.equal(expectResponse);
+        });
+}
+
+export async function modifyArtist(
+    req,
+    id = 1,
+    props = { name: "NewArtist" },
+    expectStatus = undefined,
+) {
+    await req
+        .put('/artist/' + id)
+        .send(props)
+        .then((res) => {
+            expectStatus && expect(res).to.have.status(expectStatus);
+        });
+}
+
+export async function checkArtist(
+    req,
+    id,
+    expectStatus = undefined,
+    expectResponse = undefined,
+) {
+    await req
+        .get('/artist/' + id)
+        .then((res) => {
+            expectStatus && expect(res).to.have.status(expectStatus);
+            expectResponse && expect(res.body).to.deep.equal(expectResponse);
+        })
+}