@oddnetworks/oddworks - npm Package Compare versions

Comparing version 3.3.1 to 3.4.0

CHANGELOG.md

 # Changelog
 
-## 3.3.0
+## 3.4.0 Unstable
 
+- Resource specification objects must be [fetched with full channel object](https://github.com/oddnetworks/oddworks/pull/109) rather than the channel id string
+- HTTP [DELETE handlers in catalog controllers](https://github.com/oddnetworks/oddworks/pull/111).
+_ More compliant [JSON API responses](https://github.com/oddnetworks/oddworks/pull/112).
+
+## 3.3.0 Unstable
+
 - Handle special case of [JSON API links in config](https://github.com/oddnetworks/oddworks/commit/f67ae7f61010d740099d26909e1b78bd6449b218) response
@@ -6,0 +12,0 @@ - Fixes JSON API response middleware bugs

lib/middleware/error-handler.js

@@ -29,14 +29,22 @@ 'use strict';
 
-	return function errorHandler(err, req, res, next) {
-		if (err) {
+	return function errorHandler(errors, req, res, next) {
+		if (errors) {
+			errors = Array.isArray(errors) ? errors : [errors];
+
 			// We treat Boom errors specially
 			// https://github.com/hapijs/boom
 
-			if (shouldReportError(err)) {
-				handler(err, req);
-			}
+			errors.forEach(err => {
+				if (shouldReportError(err)) {
+					handler(err, req);
+				}
+			});
 
+			const err = errors[0];
+
 			res
 				.status(err.isBoom ? err.output.statusCode : 500)
-				.send({error: formatError(err)});
+				.send({
+					errors: errors.map(formatError)
+				});
 		} else {
@@ -43,0 +51,0 @@ next();

lib/middleware/request-authorize.js

@@ -36,3 +36,3 @@ 'use strict';
 		debug('audience: %s', audience.join());
-		debug('allowed: %s', allowed.join());
+		debug('allowed: %s', allowed ? allowed.join() : null);
 		if (!allowed) {
@@ -39,0 +39,0 @@ return next(Boom.methodNotAllowed());

lib/middleware/response-json-api.js

@@ -42,5 +42,6 @@ 'use strict';
 
-	function composeIncluded(data, baseUrl) {
+	function composeIncluded(include, data, baseUrl) {
+		include = include.split(',');
+		const included = _.compact(data.included || []);
 		const relationships = data.relationships || {};
-		const included = data.included || [];
 
@@ -55,22 +56,38 @@ function inIncluded(item, key) {
 			}
-			return true;
+			return included[foundIndex];
 		}
 
-		// Remove relationship references that were not found.
-		Object.keys(relationships).forEach(key => {
+		const finalIncluded = Object.keys(relationships).reduce((finalIncluded, key) => {
+			// If the request did not ?include= this relationship, don't include it.
+			if (include.indexOf(key) < 0) {
+				return finalIncluded;
+			}
+
 			const rel = relationships[key].data || {};
 
+			// Only include relationships actually referenced.
+			// Remove relationship references that were not found.
 			if (Array.isArray(rel)) {
 				rel.slice().forEach((item, i) => {
-					if (!inIncluded(item, key)) {
+					const res = inIncluded(item, key);
+					if (res) {
+						finalIncluded.push(res);
+					} else {
 						rel.splice(i, 1);
 					}
 				});
-			} else if (!inIncluded(rel, key)) {
-				delete relationships[key];
+			} else {
+				const res = inIncluded(rel, key);
+				if (res) {
+					finalIncluded.push(res);
+				} else {
+					delete relationships[key];
+				}
 			}
-		});
 
-		if (included.length) {
-			return included.map(entity => {
+			return finalIncluded;
+		}, []);
+
+		if (finalIncluded.length) {
+			return finalIncluded.map(entity => {
 				return format(entity, baseUrl);
@@ -89,4 +106,11 @@ });
 		let baseUrl = `${req.protocol}://${req.hostname}`;
-		const port = req.socket.address().port;
-		if (port >= 1024) {
+		const port = req.socket.address().port.toString();
+
+		let defaultPort = false;
+		if ((req.protocol === 'http' && port === '80') ||
+			(req.protocol === 'https' && port === '443')) {
+			defaultPort = true;
+		}
+
+		if (defaultPort) {
 			baseUrl = `${baseUrl}:${port}`;
@@ -112,3 +136,3 @@ }
 			if (_.isString(req.query.include)) {
-				res.body.included = composeIncluded(data, baseUrl);
+				res.body.included = composeIncluded(req.query.include, data, baseUrl);
 			}
@@ -115,0 +139,0 @@ delete data.included;

lib/services/catalog/commands/index.js

@@ -120,3 +120,3 @@ 'use strict';
 					service.bus.sendCommand(
-						{role: 'catalog', cmd: 'removeSpec'},
+						{role: 'catalog', cmd: 'removeItemSpec'},
 						spec
@@ -123,0 +123,0 @@ );

lib/services/catalog/controllers/catalog-spec-controller.js

 'use strict';
 
+const Promise = require('bluebird');
 const _ = require('lodash');
@@ -16,18 +17,36 @@ const Boom = require('boom');
 		const type = this.type;
-		const channel = req.query.channel || (req.identity.channel || {}).id;
+		const id = req.params.id;
+		const channel = req.identity.channel;
+		const channelId = req.query.channel;
 
-		if (!channel) {
+		if (req.query.include) {
+			return next(Boom.badRequest(
+				'The "include" parmeter is not supported for spec resources'
+			));
+		}
+
+		let promise;
+		if (channel) {
+			promise = Promise.resolve(channel);
+		} else if (channelId) {
+			promise = this.bus.query(
+				{role: 'store', cmd: 'get', type: 'channel'},
+				{type: 'channel', id: channelId}
+			);
+		} else {
 			return next(Boom.badRequest('channel parameter is required'));
 		}
 
-		const args = {
-			channel,
-			type,
-			id: req.params.id
-		};
+		return promise
+			.then(channel => {
+				if (!channel) {
+					return next(Boom.badData(`channel ${channelId} does not exist`));
+				}
 
-		return this.bus
-			.query({role: 'catalog', cmd: 'fetchItemSpec'}, args)
+				const args = {channel, type, id};
+
+				return this.bus
+					.query({role: 'catalog', cmd: 'fetchItemSpec'}, args);
+			})
 			.then(resource => {
-				res.status(200);
 				res.body = resource;
@@ -41,31 +60,68 @@ return next();
 		const type = this.type;
-		const channel = req.query.channel || (req.identity.channel || {}).id;
+		const channel = req.identity.channel;
+		const channelId = req.query.channel;
+		const payload = req.body;
+		const id = req.params.id;
 
-		if (!channel) {
+		let promise;
+		if (channel) {
+			promise = Promise.resolve(channel);
+		} else if (channelId) {
+			promise = this.bus.query(
+				{role: 'store', cmd: 'get', type: 'channel'},
+				{type: 'channel', id: channelId}
+			);
+		} else {
 			return next(Boom.badRequest('channel parameter is required'));
 		}
 
+		return promise
+			.then(channel => {
+				const args = {channel, type, id};
+
+				return this.bus
+					.query({role: 'catalog', cmd: 'fetchItemSpec'}, args)
+					.then(resource => {
+						if (!resource) {
+							return next(Boom.notFound(`cannot find ${type} ${id}`));
+						}
+
+						resource = _.merge({}, resource, payload);
+						resource.channel = channel.id;
+						resource.type = this.type;
+						resource.id = id;
+
+						this.bus.sendCommand({role: 'catalog', cmd: 'setItemSpec'}, resource);
+						res.body = {};
+						res.status(200);
+						return next();
+					});
+			})
+			.catch(next);
+	}
+
+	delete(req, res, next) {
+		const channel = req.identity.channel;
+		const channelId = channel ? channel.id : req.query.channel;
+
+		if (!channelId) {
+			return next(Boom.badRequest('channel parameter is required'));
+		}
+
 		const args = {
-			channel,
-			type,
+			channel: channelId,
+			type: this.type,
 			id: req.params.id
 		};
 
-		const payload = req.body;
-
 		return this.bus
-			.query({role: 'catalog', cmd: 'fetchItemSpec'}, args)
-			.then(resource => {
-				if (!resource) {
-					return next(Boom.notFound(`cannot find ${type} ${args.id}`));
+			.sendCommand({role: 'catalog', cmd: 'removeItemSpec'}, args)
+			.then(result => {
+				if (result) {
+					res.status(200);
+				} else {
+					res.status(404);
 				}
 
-				resource = _.merge({}, resource, payload);
-				resource.channel = args.channel;
-				resource.type = this.type;
-				resource.id = args.id;
-
-				this.bus.sendCommand({role: 'catalog', cmd: 'setItemSpec'}, resource);
 				res.body = {};
-				res.status(200);
 				return next();
@@ -72,0 +128,0 @@ })

lib/services/catalog/controllers/catalog-spec-list-controller.js

@@ -20,10 +20,6 @@ 'use strict';
 
-		if (!channelId) {
-			return next(Boom.badRequest('channel parameter is required'));
-		}
-
 		let promise;
 		if (req.identity.channel) {
 			promise = Promise.resolve(req.identity.channel);
-		} else {
+		} else if (channelId) {
 			promise = this.bus.query(
@@ -33,19 +29,23 @@ {role: 'store', cmd: 'get', type: 'channel'},
 			);
+		} else {
+			return next(Boom.badRequest('channel parameter is required'));
 		}
 
-		return promise.then(channel => {
-			if (!channel) {
-				return next(Boom.badData(`channel ${channelId} does not exist`));
-			}
-			const args = {channel, type, limit};
+		return promise
+			.then(channel => {
+				if (!channel) {
+					return next(Boom.badData(`channel ${channelId} does not exist`));
+				}
 
-			return this.bus
-				.query({role: 'catalog', cmd: 'fetchItemSpecList'}, args)
-				.then(resources => {
-					res.status(200);
-					res.body = resources;
-					return next();
-				})
-				.catch(next);
-		});
+				const args = {channel, type, limit};
+
+				return this.bus
+					.query({role: 'catalog', cmd: 'fetchItemSpecList'}, args);
+			})
+			.then(resources => {
+				res.status(200);
+				res.body = resources;
+				return next();
+			})
+			.catch(next);
 	}
@@ -58,10 +58,6 @@
 
-		if (!channelId) {
-			return next(Boom.badData('"channel" is required'));
-		}
-
 		let promise;
 		if (req.identity.channel) {
 			promise = Promise.resolve(req.identity.channel);
-		} else {
+		} else if (channelId) {
 			promise = this.bus.query(
@@ -71,23 +67,26 @@ {role: 'store', cmd: 'get', type: 'channel'},
 			);
+		} else {
+			return next(Boom.badData('"channel" is required'));
 		}
 
-		return promise.then(channel => {
-			if (!channel) {
-				return next(Boom.conflict(`Channel ${channelId} does not exist.`));
-			}
+		return promise
+			.then(channel => {
+				if (!channel) {
+					return next(Boom.conflict(`Channel ${channelId} does not exist.`));
+				}
 
-			payload.channel = channel.id;
-			payload.type = type;
+				payload.channel = channel.id;
+				payload.type = type;
 
-			return this.bus.sendCommand(
-				{role: 'catalog', cmd: 'setItemSpec'},
-				payload
-			);
-		})
-		.then(resource => {
-			res.body = resource;
-			res.status(201);
-			return next();
-		})
-		.catch(next);
+				return this.bus.sendCommand(
+					{role: 'catalog', cmd: 'setItemSpec'},
+					payload
+				);
+			})
+			.then(resource => {
+				res.body = resource;
+				res.status(201);
+				return next();
+			})
+			.catch(next);
 	}
@@ -94,0 +93,0 @@

lib/services/catalog/queries/index.js

@@ -78,3 +78,3 @@ 'use strict';
 
-						return checkCache(object).then(resource => {
+						return checkCache(channel, object).then(resource => {
 							// Update the meta again.
@@ -110,3 +110,3 @@ // TODO: Bring back string interpolation
 
-	function checkCache(object) {
+	function checkCache(channel, object) {
 		const now = _.now();
@@ -129,3 +129,3 @@
 			const promise = (now > (expiration + staleWindow)) ?
-				queries.fetchItemSource(object) : null;
+				fetchItemSource(channel, object) : null;
 
@@ -144,3 +144,3 @@ if (promise) {
 				// Fetch resource in the background
-				queries.fetchItemSource(object);
+				fetchItemSource(channel, object);
 			}
@@ -152,14 +152,3 @@ }
 
-	queries.fetchItemSource = function fetchItemSource(object) {
-		if (!object.channel || !_.isString(object.channel)) {
-			throw new Error('expects object.channel to be present');
-		}
-		if (!object.type || !_.isString(object.type)) {
-			throw new Error('expects object.type to be present');
-		}
-		if (!object.id || !_.isString(object.id)) {
-			throw new Error('expects object.id to be present');
-		}
-
-		const channel = object.channel;
+	function fetchItemSource(channel, object) {
 		const type = object.type;
@@ -184,3 +173,3 @@ const specId = object.id.replace(/^res/, 'spec');
 					newObject = _.cloneDeep(newObject);
-					newObject.channel = channel;
+					newObject.channel = channel.id;
 					newObject.type = type;
@@ -201,3 +190,3 @@ newObject.id = object.id;
 			});
-	};
+	}
 
@@ -249,3 +238,3 @@ // args.channel - Object *required*
 
-	// args.channel - String *required*
+	// args.channel - Object *required*
 	// args.type - String *required*
@@ -256,3 +245,3 @@ // args.id - String *required*
 
-		if (!args.channel || !_.isString(args.channel)) {
+		if (!args.channel || !_.isObject(args.channel)) {
 			throw new Error('args.channel is required');
@@ -267,3 +256,3 @@ }
 
-		const channel = args.channel;
+		const channel = args.channel.id;
 		const type = args.type;
@@ -276,3 +265,3 @@ const id = args.id;
 
-	// args.channel - String *required*
+	// args.channel - Object *required*
 	// args.type - String *required*
@@ -283,3 +272,3 @@ // args.limit - Number *default is 10*
 
-		if (!args.channel || !_.isString(args.channel)) {
+		if (!args.channel || !_.isObject(args.channel)) {
 			throw new Error('args.channel is required');
@@ -291,14 +280,8 @@ }
 
-		const channel = args.channel;
+		const channel = args.channel.id;
 		const type = `${args.type}Spec`;
 		const limit = parseInt(args.limit, 10) || 10;
 
-		const payload = {
-			channel: channel.id,
-			type,
-			limit
-		};
-
 		return service.bus
-			.query({role: 'store', cmd: 'scan', type}, payload);
+			.query({role: 'store', cmd: 'scan', type}, {channel, type, limit});
 	};
@@ -305,0 +288,0 @@

package.json

 {
   "name": "@oddnetworks/oddworks",
-  "version": "3.3.1",
+  "version": "3.4.0",
   "description": "An extensible media platform for OTT devices.",
@@ -5,0 +5,0 @@ "main": "./lib/oddworks.js",

No alert changes

Improved metrics

Total package byte prevSize

137042

increased by1.63%

Lines of code

3046

increased by2.04%

No dependency changes