diff --git a/client/src/api.ts b/client/src/api.ts index 701acbb..720f750 100644 --- a/client/src/api.ts +++ b/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, diff --git a/server/endpoints/CreateSongEndpointHandler.ts b/server/endpoints/CreateSongEndpointHandler.ts index 2497a89..e40cbe3 100644 --- a/server/endpoints/CreateSongEndpointHandler.ts +++ b/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[] = []; - reqObject.artistIds?.forEach((artistId: Number) => { - artistInstancePromises.push( - models.Artist.findAll({ - 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 = reqObject.artistIds && models.Artist.findAll({ + where: { + id: { + [Op.in]: reqObject.artistIds + } + } }); - var artistInstancesPromise = Promise.all(artistInstancePromises); // Start retrieving the album instances to link the song to. - var albumInstancePromises: Promise[] = []; - reqObject.albumIds?.forEach((albumId: Number) => { - albumInstancePromises.push( - models.Album.findAll({ - 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 = reqObject.albumIds && models.Album.findAll({ + where: { + id: { + [Op.in]: reqObject.albumIds + } + } }); - 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, - }) - .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); - }) + }); + artists && song.addArtists(artists); + albums && song.addAlbums(albums); + return song.save(); + }) + .then((song: any) => { + const responseObject: api.CreateSongResponse = { + id: song.id + }; + res.status(200).send(responseObject); }) .catch(catchUnhandledErrors); } \ No newline at end of file diff --git a/server/endpoints/QuerySongsEndpointHandler.ts b/server/endpoints/QuerySongsEndpointHandler.ts index bfd4265..fd8397c 100644 --- a/server/endpoints/QuerySongsEndpointHandler.ts +++ b/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) => { - var and = []; +const sequelizeOps: any = { + [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 = {}; - sequelizeOps[api.SongQueryFilterOp.Eq] = Op.eq; - sequelizeOps[api.SongQueryFilterOp.Ne] = Op.ne; - sequelizeOps[api.SongQueryFilterOp.In] = Op.in; - sequelizeOps[api.SongQueryFilterOp.NotIn] = Op.notIn; - sequelizeOps[api.SongQueryElemOp.And] = Op.and; - sequelizeOps[api.SongQueryElemOp.Or] = Op.or; +const sequelizeProps: any = { + [api.SongQueryElemProperty.id]: "id", + [api.SongQueryElemProperty.artistIds]: "$Artists.id$", + [api.SongQueryElemProperty.albumIds]: "$Albums.id$", +}; - var sequelizeProps:any = {}; - sequelizeProps[api.SongQueryElemProperty.id] = "id"; - sequelizeProps[api.SongQueryElemProperty.artistIds] = "artistIds"; - sequelizeProps[api.SongQueryElemProperty.albumIds] = "albumIds"; +// Returns the "where" clauses for Sequelize, per object type. +const getSequelizeWhere = (queryElem: api.SongQueryElem) => { + var where: any = { + [Op.and]: [] + }; if (queryElem.prop && queryElem.propOperator && queryElem.propOperand) { - const prop = sequelizeProps[queryElem.prop]; - const op = sequelizeOps[queryElem.propOperator]; - var filter:any = {}; - filter[op] = queryElem.propOperand; - var where:any = {}; - where[prop] = filter; - and.push(where); + // Visit a filter-like subquery leaf. + where[Op.and].push({ + [sequelizeProps[queryElem.prop]]: { + [sequelizeOps[queryElem.propOperator]]: queryElem.propOperand + } + }); } 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]; - var where:any = {}; - where[op] = children; - and.push(where) + where[Op.and].push({ + [sequelizeOps[queryElem.childrenOperator]]: children + }); } - return { - [Op.and]: and - }; + return where; } 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 = { diff --git a/server/test/integration/flows/CreateArtistFlow.js b/server/test/integration/flows/CreateArtistFlow.js index 7d8f4c2..7d75239 100644 --- a/server/test/integration/flows/CreateArtistFlow.js +++ b/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); diff --git a/server/test/integration/flows/CreateSongFlow.js b/server/test/integration/flows/CreateSongFlow.js index 2f06838..277442a 100644 --- a/server/test/integration/flows/CreateSongFlow.js +++ b/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() { - await chai.request(app) - .post('/artist') - .send({ - 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) + var req = chai.request(app).keepOpen(); + helpers.createArtist(req, { name: "MyArtist" }, 200, { id: 1 }) + .then(() => { helpers.createSong(req, { name: "MySong" }, 200, { id: 1 }) }) + .then(req.close) .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) { - await chai.request(app) - .post('/artist') - .send({ - 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(200); - expect(res.body).to.deep.equal({ - id: 1 - }); - }); - } - - init() - .then(() => { createArtist('Artist1', 1); }) - .then(() => { createArtist('Artist2', 2); }) - .then(createSong) + var req = chai.request(app).keepOpen(); + helpers.createArtist(req, { name: "Artist1" }, 200, { id: 1 }) + .then(() => { helpers.createArtist(req, { name: "Artist2" }, 200, { id: 2 }) }) + .then(() => { helpers.createSong(req, { name: "MySong", artistIds: [1, 2] }, 200, { id: 1 }) }) + .then(req.close) .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) { - await chai.request(app) - .post('/artist') - .send({ - 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) + var req = chai.request(app).keepOpen(); + helpers.createArtist(req, { name: "Artist1" }, 200, { id: 1 }) + .then(() => { helpers.createSong(req, { name: "MySong", artistIds: [1, 2] }, 400) }) + .then(req.close) .then(done); }); }); diff --git a/server/test/integration/flows/ModifyArtistFlow.js b/server/test/integration/flows/ModifyArtistFlow.js index 8b65594..485a960 100644 --- a/server/test/integration/flows/ModifyArtistFlow.js +++ b/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(); - - createArtist(req) - .then(() => modifyArtist(req)) - .then(() => checkArtist(req)) - .then(() => req.close()) + helpers.createArtist(req, { name: "MyArtist" }, 200, { id: 1 }) + .then(() => { helpers.modifyArtist(req, 1, { name: "MyNewArtist" }, 200) }) + .then(() => { helpers.checkArtist(req, 1, 200, { name: "MyNewArtist" } )}) + .then(req.close) .then(done); }); }); diff --git a/server/test/integration/flows/QuerySongsFlow.js b/server/test/integration/flows/QuerySongsFlow.js index 8aef61f..b819ebd 100644 --- a/server/test/integration/flows/QuerySongsFlow.js +++ b/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', () => { - it('should give empty list', done => { +describe('POST /song/query with several songs and filters', () => { + 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) - .then(() => createSong(req)) - .then(() => createSong(req)) - .then(() => checkSongs(req)) - .then(() => req.close()) + helpers.createArtist(req, { name: "Artist1" }, 200) + .then(() => helpers.createArtist(req, { name: "Artist2" }, 200)) + .then(() => helpers.createSong(req, { title: "Song1", artistIds: [1] }, 200)) + .then(() => helpers.createSong(req, { title: "Song2", artistIds: [1] }, 200)) + .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) - }); + }) }); }); \ No newline at end of file diff --git a/server/test/integration/flows/helpers.js b/server/test/integration/flows/helpers.js new file mode 100644 index 0000000..8a217e0 --- /dev/null +++ b/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); + }) +} \ No newline at end of file