feat: Preserve original format of images, change interpolation kernel

Closes #349

client/src/sagas/core/services/project-managers.js

@@ -24,6 +24,7 @@ export function* createProjectManager(projectId, data) {
     ({ item: projectManager } = yield call(request, api.createProjectManager, projectId, data));
   } catch (error) {
     yield put(actions.createProjectManager.failure(localId, error));
+    return;
   }
 
   yield put(actions.createProjectManager.success(localId, projectManager));

client/src/sagas/core/services/projects.js

@@ -53,6 +53,7 @@ export function* updateProject(id, data) {
     ({ item: project } = yield call(request, api.updateProject, id, data));
   } catch (error) {
     yield put(actions.updateProject.failure(id, error));
+    return;
   }
 
   yield put(actions.updateProject.success(project));
@@ -76,6 +77,7 @@ export function* updateProjectBackgroundImage(id, data) {
     ({ item: project } = yield call(request, api.updateProjectBackgroundImage, id, data));
   } catch (error) {
     yield put(actions.updateProjectBackgroundImage.failure(id, error));
+    return;
   }
 
   yield put(actions.updateProjectBackgroundImage.success(project));
@@ -101,6 +103,7 @@ export function* deleteProject(id) {
     ({ item: project } = yield call(request, api.deleteProject, id));
   } catch (error) {
     yield put(actions.deleteProject.failure(id, error));
+    return;
   }
 
   yield put(actions.deleteProject.success(project));

server/api/controllers/attachments/download-thumbnail.js

@@ -50,7 +50,7 @@ module.exports = {
       sails.config.custom.attachmentsPath,
       attachment.dirname,
       'thumbnails',
-      'cover-256.jpg',
+      `cover-256.${attachment.image.thumbnailsExtension}`,
     );
 
     if (!fs.existsSync(filePath)) {

server/api/controllers/projects/update-background-image.js

@@ -88,7 +88,7 @@ module.exports = {
     project = await sails.helpers.projects.updateOne(
       project,
       {
-        backgroundImageDirname: fileData.dirname,
+        backgroundImage: fileData,
       },
       this.req,
     );

server/api/controllers/users/update-avatar.js

@@ -89,7 +89,7 @@ module.exports = {
     user = await sails.helpers.users.updateOne(
       user,
       {
-        avatarDirname: fileData.dirname,
+        avatar: fileData,
       },
       currentUser,
       this.req,

server/api/controllers/users/update.js

@@ -65,15 +65,17 @@ module.exports = {
       throw Errors.USER_NOT_FOUND;
     }
 
-    const values = _.pick(inputs, [
+    const values = {
+      ..._.pick(inputs, [
         'isAdmin',
         'name',
-      'avatarUrl',
         'phone',
         'organization',
         'language',
         'subscribeToOwnCards',
-    ]);
+      ]),
+      avatar: inputs.avatarUrl,
+    };
 
     user = await sails.helpers.users.updateOne(user, values, currentUser, this.req);
 

server/api/helpers/attachments/process-uploaded-file.js

@@ -26,9 +26,11 @@ module.exports = {
     fs.mkdirSync(rootPath);
     await moveFile(inputs.file.fd, filePath);
 
-    const image = sharp(filePath);
+    const image = sharp(filePath, {
-    let metadata;
+      animated: true,
+    });
 
+    let metadata;
     try {
       metadata = await image.metadata();
     } catch (error) {} // eslint-disable-line no-empty
@@ -44,25 +46,19 @@ module.exports = {
       const thumbnailsPath = path.join(rootPath, 'thumbnails');
       fs.mkdirSync(thumbnailsPath);
 
+      const extension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
+
       try {
         await image
-          .resize(
+          .resize(256, metadata.height > metadata.width ? 320 : undefined, {
-            metadata.height > metadata.width
+            kernel: sharp.kernel.nearest,
-              ? {
-                  width: 256,
-                  height: 320,
-                }
-              : {
-                  width: 256,
-                },
-          )
-          .jpeg({
-            quality: 100,
-            chromaSubsampling: '4:4:4',
           })
-          .toFile(path.join(thumbnailsPath, 'cover-256.jpg'));
+          .toFile(path.join(thumbnailsPath, `cover-256.${extension}`));
 
-        fileData.image = _.pick(metadata, ['width', 'height']);
+        fileData.image = {
+          ..._.pick(metadata, ['width', 'height']),
+          thumbnailsExtension: extension,
+        };
       } catch (error1) {
         try {
           rimraf.sync(thumbnailsPath);

server/api/helpers/projects/process-uploaded-background-image-file.js

@@ -17,29 +17,36 @@ module.exports = {
   },
 
   async fn(inputs) {
-    const image = sharp(inputs.file.fd);
+    const image = sharp(inputs.file.fd, {
+      animated: true,
+    });
 
+    let metadata;
     try {
-      await image.metadata();
+      metadata = await image.metadata();
     } catch (error) {
       throw 'fileIsNotImage';
     }
 
+    if (['svg', 'pdf'].includes(metadata.format)) {
+      throw 'fileIsNotImage';
+    }
+
     const dirname = uuid();
     const rootPath = path.join(sails.config.custom.projectBackgroundImagesPath, dirname);
 
     fs.mkdirSync(rootPath);
 
+    const extension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
+
     try {
-      await image.jpeg().toFile(path.join(rootPath, 'original.jpg'));
+      await image.toFile(path.join(rootPath, `original.${extension}`));
 
       await image
-        .resize(336, 200)
+        .resize(336, 200, {
-        .jpeg({
+          kernel: sharp.kernel.nearest,
-          quality: 100,
-          chromaSubsampling: '4:4:4',
         })
-        .toFile(path.join(rootPath, 'cover-336.jpg'));
+        .toFile(path.join(rootPath, `cover-336.${extension}`));
     } catch (error1) {
       try {
         rimraf.sync(rootPath);
@@ -58,6 +65,7 @@ module.exports = {
 
     return {
       dirname,
+      extension,
     };
   },
 };

server/api/helpers/projects/update-one.js

@@ -14,15 +14,11 @@ module.exports = {
           return false;
         }
 
-        if (
+        if (value.background && !_.isPlainObject(value.background)) {
-          !_.isUndefined(value.background) &&
-          !_.isNull(value.background) &&
-          !_.isPlainObject(value.background)
-        ) {
           return false;
         }
 
-        if (!_.isUndefined(value.backgroundImage) && !_.isNull(value.backgroundImage)) {
+        if (value.backgroundImage && !_.isPlainObject(value.backgroundImage)) {
           return false;
         }
 
@@ -36,24 +32,17 @@ module.exports = {
   },
 
   exits: {
-    backgroundImageDirnameMustBeNotNullInValues: {},
+    backgroundImageMustBeNotNullInValues: {},
   },
 
   async fn(inputs) {
-    if (!_.isUndefined(inputs.values.backgroundImage)) {
+    if (inputs.values.backgroundImage) {
-      /* eslint-disable no-param-reassign */
-      inputs.values.backgroundImageDirname = null;
-      delete inputs.values.backgroundImage;
-      /* eslint-enable no-param-reassign */
-    }
-
-    if (inputs.values.backgroundImageDirname) {
       // eslint-disable-next-line no-param-reassign
       inputs.values.background = {
         type: 'image',
       };
     } else if (
-      _.isNull(inputs.values.backgroundImageDirname) &&
+      _.isNull(inputs.values.backgroundImage) &&
       inputs.record.background &&
       inputs.record.background.type === 'image'
     ) {
@@ -62,14 +51,14 @@ module.exports = {
 
     let project;
     if (inputs.values.background && inputs.values.background.type === 'image') {
-      if (_.isNull(inputs.values.backgroundImageDirname)) {
+      if (_.isNull(inputs.values.backgroundImage)) {
-        throw 'backgroundImageDirnameMustBeNotNullInValues';
+        throw 'backgroundImageMustBeNotNullInValues';
       }
 
-      if (_.isUndefined(inputs.values.backgroundImageDirname)) {
+      if (_.isUndefined(inputs.values.backgroundImage)) {
         project = await Project.updateOne({
           id: inputs.record.id,
-          backgroundImageDirname: {
+          backgroundImage: {
             '!=': null,
           },
         }).set(inputs.values);
@@ -86,14 +75,15 @@ module.exports = {
 
     if (project) {
       if (
-        inputs.record.backgroundImageDirname &&
+        inputs.record.backgroundImage &&
-        project.backgroundImageDirname !== inputs.record.backgroundImageDirname
+        (!project.backgroundImage ||
+          project.backgroundImage.dirname !== inputs.record.backgroundImage.dirname)
       ) {
         try {
           rimraf.sync(
             path.join(
               sails.config.custom.projectBackgroundImagesPath,
-              inputs.record.backgroundImageDirname,
+              inputs.record.backgroundImage.dirname,
             ),
           );
         } catch (error) {

server/api/helpers/users/process-uploaded-avatar-file.js

@@ -17,34 +17,36 @@ module.exports = {
   },
 
   async fn(inputs) {
-    const image = sharp(inputs.file.fd);
+    const image = sharp(inputs.file.fd, {
+      animated: true,
+    });
 
+    let metadata;
     try {
-      await image.metadata();
+      metadata = await image.metadata();
     } catch (error) {
       throw 'fileIsNotImage';
     }
 
+    if (['svg', 'pdf'].includes(metadata.format)) {
+      throw 'fileIsNotImage';
+    }
+
     const dirname = uuid();
     const rootPath = path.join(sails.config.custom.userAvatarsPath, dirname);
 
     fs.mkdirSync(rootPath);
 
+    const extension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
+
     try {
-      await image
+      await image.toFile(path.join(rootPath, `original.${extension}`));
-        .jpeg({
-          quality: 100,
-          chromaSubsampling: '4:4:4',
-        })
-        .toFile(path.join(rootPath, 'original.jpg'));
 
       await image
-        .resize(100, 100)
+        .resize(100, 100, {
-        .jpeg({
+          kernel: sharp.kernel.nearest,
-          quality: 100,
-          chromaSubsampling: '4:4:4',
         })
-        .toFile(path.join(rootPath, 'square-100.jpg'));
+        .toFile(path.join(rootPath, `square-100.${extension}`));
     } catch (error1) {
       try {
         rimraf.sync(rootPath);
@@ -63,6 +65,7 @@ module.exports = {
 
     return {
       dirname,
+      extension,
     };
   },
 };

server/api/helpers/users/update-one.js

@@ -28,7 +28,7 @@ module.exports = {
           return false;
         }
 
-        if (!_.isUndefined(value.avatarUrl) && !_.isNull(value.avatarUrl)) {
+        if (value.avatar && !_.isPlainObject(value.avatar)) {
           return false;
         }
 
@@ -74,13 +74,6 @@ module.exports = {
       inputs.values.username = inputs.values.username.toLowerCase();
     }
 
-    if (!_.isUndefined(inputs.values.avatarUrl)) {
-      /* eslint-disable no-param-reassign */
-      inputs.values.avatarDirname = null;
-      delete inputs.values.avatarUrl;
-      /* eslint-enable no-param-reassign */
-    }
-
     const user = await User.updateOne({
       id: inputs.record.id,
       deletedAt: null,
@@ -102,9 +95,12 @@ module.exports = {
       );
 
     if (user) {
-      if (inputs.record.avatarDirname && user.avatarDirname !== inputs.record.avatarDirname) {
+      if (
+        inputs.record.avatar &&
+        (!user.avatar || user.avatar.dirname !== inputs.record.avatar.dirname)
+      ) {
         try {
-          rimraf.sync(path.join(sails.config.custom.userAvatarsPath, inputs.record.avatarDirname));
+          rimraf.sync(path.join(sails.config.custom.userAvatarsPath, inputs.record.avatar.dirname));
         } catch (error) {
           console.warn(error.stack); // eslint-disable-line no-console
         }

server/api/models/Attachment.js

@@ -49,10 +49,10 @@ module.exports = {
 
   customToJSON() {
     return {
-      ..._.omit(this, ['dirname', 'filename']),
+      ..._.omit(this, ['dirname', 'filename', 'image.thumbnailsExtension']),
       url: `${sails.config.custom.attachmentsUrl}/${this.id}/download/${this.filename}`,
       coverUrl: this.image
-        ? `${sails.config.custom.attachmentsUrl}/${this.id}/download/thumbnails/cover-256.jpg`
+        ? `${sails.config.custom.attachmentsUrl}/${this.id}/download/thumbnails/cover-256.${this.image.thumbnailsExtension}`
         : null,
     };
   },

server/api/models/Project.js

@@ -54,11 +54,9 @@ module.exports = {
     background: {
       type: 'json',
     },
-    backgroundImageDirname: {
+    backgroundImage: {
-      type: 'string',
+      type: 'json',
-      isNotEmptyString: true,
+      columnName: 'background_image',
-      allowNull: true,
-      columnName: 'background_image_dirname',
     },
 
     //  ╔═╗╔╦╗╔╗ ╔═╗╔╦╗╔═╗
@@ -82,10 +80,10 @@ module.exports = {
 
   customToJSON() {
     return {
-      ..._.omit(this, ['backgroundImageDirname']),
+      ..._.omit(this, ['backgroundImage']),
-      backgroundImage: this.backgroundImageDirname && {
+      backgroundImage: this.backgroundImage && {
-        url: `${sails.config.custom.projectBackgroundImagesUrl}/${this.backgroundImageDirname}/original.jpg`,
+        url: `${sails.config.custom.projectBackgroundImagesUrl}/${this.backgroundImage.dirname}/original.${this.backgroundImage.extension}`,
-        coverUrl: `${sails.config.custom.projectBackgroundImagesUrl}/${this.backgroundImageDirname}/cover-336.jpg`,
+        coverUrl: `${sails.config.custom.projectBackgroundImagesUrl}/${this.backgroundImage.dirname}/cover-336.${this.backgroundImage.extension}`,
       },
     };
   },

server/api/models/User.js

@@ -37,11 +37,8 @@ module.exports = {
       regex: /^[a-zA-Z0-9]+((_|\.)?[a-zA-Z0-9])*$/,
       allowNull: true,
     },
-    avatarDirname: {
+    avatar: {
-      type: 'string',
+      type: 'json',
-      isNotEmptyString: true,
-      allowNull: true,
-      columnName: 'avatar_dirname',
     },
     phone: {
       type: 'string',
@@ -106,10 +103,10 @@ module.exports = {
 
   customToJSON() {
     return {
-      ..._.omit(this, ['password', 'avatarDirname', 'passwordChangedAt']),
+      ..._.omit(this, ['password', 'avatar', 'passwordChangedAt']),
       avatarUrl:
-        this.avatarDirname &&
+        this.avatar &&
-        `${sails.config.custom.userAvatarsUrl}/${this.avatarDirname}/square-100.jpg`,
+        `${sails.config.custom.userAvatarsUrl}/${this.avatar.dirname}/square-100.${this.avatar.extension}`,
     };
   },
 };

server/config/routes.js

@@ -82,7 +82,7 @@ module.exports.routes = {
     skipAssets: false,
   },
 
-  'GET /attachments/:id/download/thumbnails/cover-256.jpg': {
+  'GET /attachments/:id/download/thumbnails/cover-256.:extension': {
     action: 'attachments/download-thumbnail',
     skipAssets: false,
   },

server/db/migrations/20221003140000_@.js

@@ -0,0 +1,4 @@
+/* Move to new naming by feature */
+
+module.exports.up = () => Promise.resolve();
+module.exports.down = () => Promise.resolve();

server/db/migrations/20221223131625_preserve_original_format_of_images.js

@@ -0,0 +1,111 @@
+const path = require('path');
+const rimraf = require('rimraf');
+const sharp = require('sharp');
+
+const getConfig = require('../../get-config');
+
+const migrateImage = async (knex, tableName, fieldName, prevFieldName) => {
+  await knex.schema.table(tableName, (table) => {
+    /* Columns */
+
+    table.jsonb(fieldName);
+  });
+
+  await knex(tableName)
+    .update({
+      [fieldName]: knex.raw('format(\'{"dirname":"%s","extension":"jpg"}\', ??)::jsonb', [
+        prevFieldName,
+      ]),
+    })
+    .whereNotNull(prevFieldName);
+
+  await knex.schema.table(tableName, (table) => {
+    table.dropColumn(prevFieldName);
+  });
+};
+
+const rollbackImage = async (knex, tableName, fieldName, prevFieldName) => {
+  await knex.schema.table(tableName, (table) => {
+    /* Columns */
+
+    table.text(prevFieldName);
+  });
+
+  await knex(tableName)
+    .update({
+      [prevFieldName]: knex.raw("??->>'dirname'", [fieldName]),
+    })
+    .whereNotNull(fieldName);
+
+  await knex.schema.table(tableName, (table) => {
+    table.dropColumn(fieldName);
+  });
+};
+
+module.exports.up = async (knex) => {
+  await migrateImage(knex, 'user_account', 'avatar', 'avatar_dirname');
+  await migrateImage(knex, 'project', 'background_image', 'background_image_dirname');
+
+  const config = await getConfig();
+  const attachments = await knex('attachment').whereNotNull('image');
+
+  // eslint-disable-next-line no-restricted-syntax
+  for (attachment of attachments) {
+    const rootPath = path.join(config.custom.attachmentsPath, attachment.dirname);
+    const thumbnailsPath = path.join(rootPath, 'thumbnails');
+
+    const image = sharp(path.join(rootPath, attachment.filename), {
+      animated: true,
+    });
+
+    let metadata;
+    try {
+      metadata = await image.metadata(); // eslint-disable-line no-await-in-loop
+    } catch (error) {
+      continue; // eslint-disable-line no-continue
+    }
+
+    const extension = metadata.format === 'jpeg' ? 'jpg' : metadata.format;
+
+    try {
+      // eslint-disable-next-line no-await-in-loop
+      await image
+        .resize(256, metadata.height > metadata.width ? 320 : undefined, {
+          kernel: sharp.kernel.nearest,
+        })
+        .toFile(path.join(thumbnailsPath, `cover-256.${extension}`));
+    } catch (error) {
+      continue; // eslint-disable-line no-continue
+    }
+
+    if (extension !== 'jpg') {
+      try {
+        rimraf.sync(path.join(thumbnailsPath, 'cover-256.jpg'));
+      } catch (error) {
+        console.warn(error.stack); // eslint-disable-line no-console
+      }
+    }
+
+    // eslint-disable-next-line no-await-in-loop
+    await knex('attachment')
+      .update({
+        image: {
+          width: metadata.width,
+          height: metadata.pageHeight || metadata.height,
+          thumbnailsExtension: extension,
+        },
+      })
+      .where('id', attachment.id);
+  }
+};
+
+module.exports.down = async (knex) => {
+  await rollbackImage(knex, 'user_account', 'avatar', 'avatar_dirname');
+  await rollbackImage(knex, 'project', 'background_image', 'background_image_dirname');
+
+  return knex('attachment')
+    .update({
+      image: knex.raw("?? - 'thumbnailsExtension'", ['image']),
+    })
+    .whereNotNull('image');
+};