Merge branch 'unify-errors' of github.com:keen-studio/payload

src/collections/registerRoutes.js

@@ -5,7 +5,6 @@ import authRoutes from '../auth/routes';
 import {
   query, create, findOne, destroy, update,
 } from '../mongoose/requestHandlers';
-import { upload as uploadMedia, update as updateMedia } from '../uploads/requestHandlers';
 import bindModelMiddleware from '../mongoose/bindModel';
 import setModelLocaleMiddleware from '../localization/setModelLocale';
 
@@ -28,18 +27,13 @@ const registerRoutes = ({ model, config }, router) => {
     bindModelMiddleware(model),
     setModelLocaleMiddleware());
 
-  // TODO: this feels sloppy, need to discuss media enabled collection handlers
-  const createHandler = config.media ? (req, res, next) => uploadMedia(req, res, next, config.media) : create;
-  const updateHandler = config.media ? (req, res, next) => updateMedia(req, res, next, config.media) : update;
-  // TODO: Do we need a delete?
-
   router.route(`/${config.slug}`)
     .get(config.policies.read, query)
-    .post(config.policies.create, createHandler);
+    .post(config.policies.create, create);
 
   router.route(`/${config.slug}/:id`)
     .get(config.policies.read, findOne)
-    .put(config.policies.update, updateHandler)
+    .put(config.policies.update, update)
     .delete(config.policies.destroy, destroy);
 };
 

src/errors/APIError.js

@@ -19,7 +19,7 @@ class ExtendableError extends Error {
  * Class representing an API error.
  * @extends ExtendableError
  */
-class APIError extends ExtendableError {
+export class APIError extends ExtendableError {
   /**
    * Creates an API error.
    * @param {string} message - Error message.
@@ -30,5 +30,3 @@ class APIError extends ExtendableError {
     super(message, status, isPublic);
   }
 }
-
-module.exports = APIError;

src/errors/DuplicateCollection.js

@@ -1,6 +1,6 @@
-import { PayloadError } from './PayloadError';
+import { APIError } from './APIError';
 
-export class DuplicateCollection extends PayloadError {
+export class DuplicateCollection extends APIError {
   constructor(config) {
     super(`Collection name "${config.labels.singular}" is already in use`);
   }

src/errors/DuplicateGlobal.js

@@ -1,6 +1,6 @@
-import { PayloadError } from './PayloadError';
+import { APIError } from './APIError';
 
-export class DuplicateGlobal extends PayloadError {
+export class DuplicateGlobal extends APIError {
   constructor(config) {
     super(`Global label "${config.label}" is already in use`);
   }

src/errors/ExistingUser.js

@@ -1,7 +1,7 @@
-import { PayloadError } from './PayloadError';
+import { APIError } from './APIError';
 
-export class ExistingUser extends PayloadError {
+export class ExistingUser extends APIError {
   constructor(config) {
-    super(`Error when registering ${config.labels.singular}: a user model already exists.`);
+    super(`Error when registering "${ config.labels.singular }": a user model already exists.`);
   }
 }

src/errors/MissingCollectionLabel.js

@@ -1,6 +1,6 @@
-import { PayloadError } from './PayloadError';
+import { APIError } from './APIError';
 
-export class MissingCollectionLabel extends PayloadError {
+export class MissingCollectionLabel extends APIError {
   constructor(config) {
     super('payload.config.collection object is missing label');
   }

src/errors/MissingGlobalLabel.js

@@ -1,6 +1,6 @@
-import { PayloadError } from './PayloadError';
+import { APIError } from './APIError';
 
-export class MissingGlobalLabel extends PayloadError {
+export class MissingGlobalLabel extends APIError {
   constructor(config) {
     super(`${config.globals} object is missing label`);
   }

src/errors/NotFound.js

@@ -1,7 +1,8 @@
-import { PayloadError } from './PayloadError';
+import httpStatus from 'http-status';
+import { APIError } from './APIError';
 
-export class NotFound extends PayloadError {
+export class NotFound extends APIError {
   constructor() {
-    super('Not found.');
+    super('Not found.', httpStatus.NOT_FOUND);
   }
 }

src/errors/PayloadError.js

@@ -1,6 +0,0 @@
-export class PayloadError extends Error {
-  constructor(message) {
-    super(message);
-    Error.captureStackTrace(this, this.constructor);
-  }
-}

src/errors/index.js

@@ -1,4 +1,4 @@
-export { PayloadError } from './PayloadError';
+export { APIError } from './APIError';
 export { DuplicateCollection } from './DuplicateCollection';
 export { DuplicateGlobal } from './DuplicateGlobal';
 export { MissingCollectionLabel } from './MissingCollectionLabel';

src/globals/requestHandlers.js

@@ -1,22 +1,24 @@
 import httpStatus from 'http-status';
 import { findOne } from '../mongoose/resolvers';
+import { NotFound } from '../errors';
 
 export const upsert = (req, res) => {
   if (!req.model.schema.tree[req.params.slug]) {
-    return res.status(httpStatus.NOT_FOUND).json({ error: 'not found' });
+    res.status(httpStatus.NOT_FOUND).json(new NotFound());
+    return;
   }
 
-  req.model.findOne({}, (err, doc) => {
+  req.model.findOne({}, (findErr, doc) => {
     let global = {};
-
     if (!doc) {
       if (req.params.slug) {
         global[req.params.slug] = req.body;
       } else {
         global = req.body;
       }
-      return req.model.create(global, (err, result) => {
-        if (err) return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+
+      return req.model.create(global, (createErr, result) => {
+        if (createErr) return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: createErr });
 
         return res.status(httpStatus.CREATED)
           .json({
@@ -27,14 +29,10 @@ export const upsert = (req, res) => {
     }
 
     if (!doc[req.params.slug]) {
-      // eslint-disable-next-line no-param-reassign
-      doc[req.params.slug] = {};
+      Object.assign(doc[req.params.slug], {});
     }
 
-    Object.keys(req.body).forEach((e) => {
-      // eslint-disable-next-line no-param-reassign
-      doc[req.params.slug][e] = req.body[e];
-    });
+    Object.assign(doc[req.params.slug], req.body);
 
     return doc.save((err) => {
       if (err) return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
@@ -62,8 +60,7 @@ export const fetch = (req, res) => {
       if (globals[req.params.key]) {
         return res.json(globals[req.params.key]);
       } if (req.params.key) {
-        return res.status(httpStatus.NOT_FOUND)
-          .json({ error: 'not found' });
+        return res.status(httpStatus.NOT_FOUND).json(new NotFound());
       }
       return res.json(globals);
     })

src/index.js

@@ -34,16 +34,15 @@ class Payload {
 
   registerCollection = (collection) => {
     validateCollection(collection, this.collections, this.User);
-    const registeredCollection = registerCollectionSchema(collection, this.config);
-    this.collections[collection.slug] = registeredCollection;
+    this.collections[collection.slug] = registerCollectionSchema(collection, this.config);
     registerCollectionRoutes(this.collections[collection.slug], this.router);
-  }
+  };
 
   registerGlobals = (globals) => {
     validateGlobals(globals);
     this.globals = registerGlobalSchema(globals, this.config);
     registerGlobalRoutes(this.globals.model, this.router);
-  }
+  };
 
   getCollections = () => this.collections;
 

src/init/registerExpressMiddleware.js

@@ -10,6 +10,6 @@ const registerExpressMiddleware = ({ app, config, router }) => {
   app.use(bodyParser.urlencoded({ extended: true }));
   app.use(localizationMiddleware(config.localization));
   app.use(router);
-}
+};
 
 export default registerExpressMiddleware;

src/init/uploads.js

@@ -8,12 +8,10 @@ const initUploads = ({ config, app, router }) => {
   app.use(fileUpload({}));
 
   if (config.uploads) {
-    app.use(fileUpload());
+    app.use(fileUpload({dubug: true}));
 
-    router.use('', uploadRoutes(config)
-    );
+    router.use('', uploadRoutes(config));
   }
-}
+};
 
 export default initUploads;
-

src/mongoose/requestHandlers/create.js

@@ -2,12 +2,16 @@ import httpStatus from 'http-status';
 
 const create = (req, res) => {
   req.model.create(req.body, (err, result) => {
-    if (err) return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+    if (err) {
+      res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json({ error: err });
+      return;
+    }
 
-    return res.status(httpStatus.CREATED)
+    res.status(httpStatus.CREATED)
       .json({
         message: 'success',
-        result: result.toJSON({ virtuals: true }),
+        result: result.toJSON({ virtuals: true })
       });
   });
 };

src/mongoose/requestHandlers/destroy.js

@@ -1,14 +1,22 @@
 import httpStatus from 'http-status';
+import { NotFound } from '../../errors';
 
 const destroy = (req, res) => {
   req.model.findOneAndDelete({ _id: req.params.id }, (err, doc) => {
-    if (err)
-      return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+    if (err) {
+      res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json({ error: err });
+      return;
+    }
 
-    if (!doc)
-      return res.status(httpStatus.NOT_FOUND).send('Not Found');
+    if (!doc) {
+      res.status(httpStatus.NOT_FOUND)
+        .json(new NotFound());
+      return;
+    }
 
-    return res.send();
+    res.status(httpStatus.OK)
+      .send({ result: 'success' });
   });
 };
 

src/mongoose/requestHandlers/findOne.js

@@ -1,4 +1,3 @@
-import httpStatus from 'http-status';
 import { modelById } from '../resolvers';
 
 const findOne = (req, res) => {
@@ -12,7 +11,7 @@ const findOne = (req, res) => {
 
   modelById(query)
     .then(doc => res.json(doc))
-    .catch(err => res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err }));
+    .catch(err => res.status(err.status).json({ error: err }));
 };
 
 export default findOne;

src/mongoose/requestHandlers/index.js

@@ -9,5 +9,5 @@ export {
   destroy,
   findOne,
   query,
-  update
-}
+  update,
+};

src/mongoose/requestHandlers/query.js

@@ -11,9 +11,10 @@ const query = (req, res) => {
 
   req.model.paginate(req.model.apiQuery(req.query, req.locale), { options: queryOptions }, (err, result) => {
     if (err) {
-      return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+      res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+      return;
     }
-    return res.json({
+    res.status(httpStatus.OK).json({
       ...result,
       docs: result.docs.map((doc) => {
         if (req.locale && doc.setLocale) {

src/mongoose/requestHandlers/update.js

@@ -1,21 +1,24 @@
 import httpStatus from 'http-status';
+import { NotFound } from '../../errors';
 
 const update = (req, res) => {
   req.model.findOne({ _id: req.params.id }, '', {}, (err, doc) => {
-    if (!doc)
-      return res.status(httpStatus.NOT_FOUND).send('Not Found');
+    if (!doc) {
+      res.status(httpStatus.NOT_FOUND).json(new NotFound());
+      return;
+    }
 
-    Object.keys(req.body).forEach(e => {
-      doc[e] = req.body[e];
-    });
+    Object.assign(doc, req.body);
 
-    doc.save((err) => {
-      if (err)
-        return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: err });
+    doc.save((saveError) => {
+      if (saveError) {
+        res.status(httpStatus.INTERNAL_SERVER_ERROR).json({ error: saveError });
+        return;
+      }
 
-      return res.json({
+      res.status(httpStatus.OK).json({
         message: 'success',
-        result: doc.toJSON({ virtuals: true })
+        result: doc.toJSON({ virtuals: true }),
       });
     });
   });

src/mongoose/resolvers/create.js

@@ -1,10 +1,12 @@
+import { APIError } from '../../errors';
 
-const create = query => {
+const create = (query) => {
   return new Promise((resolve, reject) => {
     query.Model.create(query.input, (err, doc) => {
       console.log(err, doc);
       if (err || !doc) {
-        return reject({message: err})
+        reject(new APIError(err));
+        return;
       }
       resolve(doc);
     });

src/mongoose/resolvers/find.js

@@ -1,16 +1,17 @@
+import { NotFound } from '../../errors';
+
 const find = (query) => {
   return new Promise((resolve, reject) => {
     query.Model.find({}, (err, docs) => {
       if (err || !docs) {
-        return reject({ message: 'not found' });
+        reject(new NotFound());
       }
 
       let result = docs;
 
       if (query.locale) {
         docs.setLocale(query.locale, query.fallback);
-        const json = docs.toJSON({ virtuals: true });
-        result = json;
+        result = docs.toJSON({ virtuals: true });
       }
 
       resolve(result);

src/mongoose/resolvers/findOne.js

@@ -1,3 +1,5 @@
+import { NotFound } from '../../errors';
+
 const find = ({
   Model, locale, fallback, depth,
 }) => {
@@ -12,15 +14,15 @@ const find = ({
   return new Promise((resolve, reject) => {
     Model.findOne(null, null, options, (err, doc) => {
       if (err || !doc) {
-        return reject({ message: 'not found' });
+        reject(new NotFound());
+        return;
       }
 
       let result = doc;
 
       if (locale) {
         doc.setLocale(locale, fallback);
-        const json = doc.toJSON({ virtuals: true });
-        result = json;
+        result = doc.toJSON({ virtuals: true });
       }
 
       resolve(result);

src/mongoose/resolvers/index.js

@@ -7,5 +7,5 @@ export {
   modelById,
   find,
   findOne,
-  create
-}
+  create,
+};

src/mongoose/resolvers/modelById.js

@@ -1,3 +1,5 @@
+import { NotFound } from '../../errors';
+
 const modelById = (query) => {
   const options = {};
   const { depth } = query;
@@ -10,8 +12,10 @@ const modelById = (query) => {
 
   return new Promise((resolve, reject) => {
     query.Model.findOne({ _id: query.id }, {}, options, (err, doc) => {
+
       if (err || !doc) {
-        return reject({ message: 'not found' });
+        reject(new NotFound());
+        return;
       }
 
       let result = doc;

src/mongoose/schema/fieldToSchemaMap.js

@@ -46,6 +46,7 @@ const fieldToSchemaMap = {
   repeater: (field) => {
     const schema = {};
     if (field.id === false) {
+      // eslint-disable-next-line no-underscore-dangle
       schema._id = false;
     }
     field.fields.forEach((subField) => {

src/uploads/images/imageResizer.js

@@ -1,39 +1,42 @@
 import sharp from 'sharp';
 import { promisify } from 'util';
 import imageSize from 'image-size';
+
 const sizeOf = promisify(imageSize);
 
 function getOutputImageName(sourceImage, size) {
-  let extension = sourceImage.split('.').pop();
-  let filenameWithoutExtension = sourceImage.substr(0, sourceImage.lastIndexOf('.')) || sourceImage;
+  const extension = sourceImage.split('.')
+    .pop();
+  const filenameWithoutExtension = sourceImage.substr(0, sourceImage.lastIndexOf('.')) || sourceImage;
   return `${filenameWithoutExtension}-${size.width}x${size.height}.${extension}`;
 }
 
 export async function resizeAndSave(config, uploadConfig, file) {
-  let sourceImage = `${config.staticDir}/${file.name}`;
+  const sourceImage = `${config.staticDir}/${file.name}`;
 
-  let outputSizes = [];
+  const outputSizes = [];
   try {
     const dimensions = await sizeOf(sourceImage);
-    for (let desiredSize of uploadConfig.imageSizes) {
+    uploadConfig.imageSizes.map(async (desiredSize) => {
       if (desiredSize.width > dimensions.width) {
-        continue;
+        return;
       }
-      let outputImageName = getOutputImageName(sourceImage, desiredSize);
+      const outputImageName = getOutputImageName(sourceImage, desiredSize);
+      // eslint-disable-next-line no-await-in-loop
       await sharp(sourceImage)
         .resize(desiredSize.width, desiredSize.height, {
-          position: desiredSize.crop || 'centre' // would it make sense for this to be set by the uploader?
+          position: desiredSize.crop || 'centre', // would it make sense for this to be set by the uploader?
         })
         .toFile(outputImageName);
       outputSizes.push({
         name: desiredSize.name,
         height: desiredSize.height,
-        width: desiredSize.width
+        width: desiredSize.width,
       });
-    }
+    });
   } catch (e) {
     console.log('error in resize and save', e.message);
   }
-
+  console.log(outputSizes);
   return outputSizes;
 }

src/uploads/images/model.js

@@ -2,22 +2,20 @@ import mongoose from 'mongoose';
 import localizationPlugin from '../../localization/plugin';
 
 const imageUploadModelLoader = (Upload, config) => {
-
   const ImageSchema = new mongoose.Schema(
     {
       sizes: [{
         name: { type: String },
         height: { type: Number },
         width: { type: Number },
-        _id: false
-      }]
-    }
+        _id: false,
+      }],
+    },
   );
 
   ImageSchema.plugin(localizationPlugin, config.localization);
 
   return Upload.discriminator('image', ImageSchema);
-
 };
 
 export default imageUploadModelLoader;

src/uploads/requestHandlers.js

@@ -1,72 +1,8 @@
 import mkdirp from 'mkdirp';
-import { resizeAndSave } from './images/imageResizer';
 import httpStatus from 'http-status';
+import { resizeAndSave } from './images/imageResizer';
 import modelById from '../mongoose/resolvers/modelById';
-
-const update = async (req, res, next, config) => {
-    const query = {
-      Model: req.model,
-      id: req.params.id,
-      locale: req.locale,
-    };
-    let doc = await modelById(query, {returnRawDoc: true});
-    if (!doc)
-      return res.status(httpStatus.NOT_FOUND).send('Not Found');
-
-    if (req.files && req.files.file) {
-      doc['filename'] = req.files.file.name;
-      let outputFilepath = `${config.staticDir}/${req.files.file.name}`;
-      let moveError = await req.files.file.mv(outputFilepath);
-      if (moveError) return res.status(500).send(moveError);
-      let handlerData = await fileTypeHandler(config, req.uploadConfig, req.files.file);
-      Object.keys(handlerData).forEach(e => {
-        doc[e] = handlerData[e];
-      });
-    }
-
-    doc.save((saveError) => {
-      if (saveError)
-        return res.status(httpStatus.INTERNAL_SERVER_ERROR).json({error: saveError});
-
-      return res.json({
-        message: 'success',
-        result: doc.toJSON({virtuals: true})
-      });
-    });
-  };
-
-const upload = async (req, res, next, config) => {
-  if (!req.files || Object.keys(req.files).length === 0) {
-    return res.status(400).send('No files were uploaded.');
-  }
-
-  mkdirp(config.staticDir, (err) => {
-    if (err) {
-      console.error(err);
-      return res.status(500).send('Upload failed.');
-    }
-  });
-
-  let outputFilepath = `${config.staticDir}/${req.files.file.name}`;
-  let moveError = await req.files.file.mv(outputFilepath);
-  if (moveError) return res.status(500).send(moveError);
-
-  let handlerData = await fileTypeHandler(config, req.uploadConfig, req.files.file);
-
-  req.model.create({
-    filename: req.files.file.name,
-    ...handlerData
-  }, (mediaCreateError, result) => {
-    if (mediaCreateError)
-      return res.status(500).json({error: mediaCreateError});
-
-    return res.status(201)
-      .json({
-        message: 'success',
-        result: result.toJSON({virtuals: true})
-      });
-  });
-};
+import { NotFound } from '../errors';
 
 async function fileTypeHandler(config, uploadConfig, file) {
   const data = {};
@@ -77,4 +13,88 @@ async function fileTypeHandler(config, uploadConfig, file) {
   return data;
 }
 
-export {update, upload};
+const update = async (req, res, next, config) => {
+  const query = {
+    Model: req.model,
+    id: req.params.id,
+    locale: req.locale,
+  };
+  const doc = await modelById(query, { returnRawDoc: true });
+  if (!doc) {
+    res.status(httpStatus.NOT_FOUND)
+      .json(new NotFound());
+    return;
+  }
+
+  if (req.files && req.files.file) {
+    doc.filename = req.files.file.name;
+    const outputFilepath = `${config.staticDir}/${req.files.file.name}`;
+    const moveError = await req.files.file.mv(outputFilepath);
+    if (moveError) {
+      res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json(moveError);
+      return;
+    }
+    const handlerData = await fileTypeHandler(config, req.uploadConfig, req.files.file);
+    Object.assign(doc, handlerData);
+  }
+
+  doc.save((saveError) => {
+    if (saveError) {
+      res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json({ error: saveError });
+      return;
+    }
+
+    res.status(httpStatus.OK)
+      .json({
+        message: 'success',
+        result: doc.toJSON({ virtuals: true }),
+      });
+  });
+};
+
+const upload = async (req, res, next, config) => {
+  console.log(req.files);
+  if (!req.files || Object.keys(req.files).length === 0) {
+    res.status(httpStatus.BAD_REQUEST)
+      .json({ error: 'No files were uploaded.' });
+    return;
+  }
+
+  mkdirp(config.staticDir, (err) => {
+    if (err) {
+      console.error(err);
+      res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json({ error: 'Upload failed.' });
+    }
+  });
+
+  const outputFilepath = `${config.staticDir}/${req.files.file.name}`;
+  const moveError = await req.files.file.mv(outputFilepath);
+  if (moveError) {
+    res.status(httpStatus.INTERNAL_SERVER_ERROR)
+      .send(moveError);
+    return;
+  }
+
+  const handlerData = await fileTypeHandler(config, req.uploadConfig, req.files.file);
+
+  req.model.create({
+    filename: req.files.file.name,
+    ...handlerData,
+  }, (mediaCreateError, result) => {
+    if (mediaCreateError) {
+      return res.status(httpStatus.INTERNAL_SERVER_ERROR)
+        .json({ error: mediaCreateError });
+    }
+
+    return res.status(httpStatus.CREATED)
+      .json({
+        message: 'success',
+        result: result.toJSON({ virtuals: true }),
+      });
+  });
+};
+
+export { update, upload };

src/uploads/routes.js

@@ -8,7 +8,7 @@ import passport from 'passport';
 
 const router = express.Router();
 
-const uploadRoutes = config => {
+const uploadRoutes = (config) => {
 
   const Upload = uploadModelLoader(config);
   const UploadModels = {
@@ -19,17 +19,16 @@ const uploadRoutes = config => {
   router.all('/upload*',
     passport.authenticate('jwt', { session: false }),
     uploadMiddleware(config, UploadModels),
-    setModelLocaleMiddleware(),
-  );
+    setModelLocaleMiddleware());
 
   router.route('/upload')
     .post(
-      (req, res, next) => upload(req, res, next, config)
+      (req, res, next) => upload(req, res, next, config),
     );
 
   router.route('/upload/:id')
     .put(
-      (req, res, next) => update(req, res, next, config)
+      (req, res, next) => update(req, res, next, config),
     );
 
   return router;