Refactor (#530)

* Big refactoring

Converted most of the code to standard classes and removed dependencies on service instances (like `Config`). This makes each class much cleaner, and consistent and above all more unit testable on its own.

Also split out the old Util class to separate classes (e.g. `Zip` and `Collator`) with their own unit tests.

There is a new `Application` class which is a singleton service which provides factory methods for things like `config`, `context`, `deviceList` and all the stuff which happens at run time.

This impacted logging which should only be used after deliberate initialisation, but which was being prematurely initialised - so these have had their `require()`s moved inline with method calls where necessary.

This also tidies up a lot of unit tests, file names, class names, linting and so on.

docs/config.md

@@ -240,9 +240,6 @@ module.exports = {
   afterConfig(config) {
     // Set your path here
     config.outputDirectory = '/home/me/scanned';
-
-    // By default paths with `..` or `/` are not allowed
-    config.allowUnsafePaths = true;
   }
 }
 ```

packages/server/.eslintrc.json

@@ -25,6 +25,13 @@
     ],
     "keyword-spacing": 1,
     "no-mixed-spaces-and-tabs": 1,
+    "no-multiple-empty-lines": [
+      1,
+      {
+        "max": 1
+      }
+    ],
+    "no-trailing-spaces": 1,
     "no-undef": 1,
     "no-unused-vars": 1,
     "no-var": 1,

packages/server/config/config.default.js

@@ -6,7 +6,7 @@
  */
 module.exports = {
   /**
-   * @param {Configuration} config 
+   * @param {Configuration} config
    */
   afterConfig(config) {
     /**
@@ -89,12 +89,12 @@ module.exports = {
    * disk) but before being returned to anything else. You can use this to
    * override default settings from the scanner, or resolution options or
    * anything else for that matter.
-   * 
+   *
    * Note that the devices parameter is an array. Most systems will likely just
    * have one scanner, but that's not always true. Therefore you will need to
    * identify the scanner by id or index. It's also possible that the list will
    * be empty if there's an upstream error.
-   * @param {ScanDevice[]} devices 
+   * @param {ScanDevice[]} devices
    */
   afterDevices(devices) {
     /**
@@ -115,7 +115,7 @@ module.exports = {
   /**
    * This method is called after a scan has completed with the resultant
    * FileInfo.
-   * @param {FileInfo} fileInfo 
+   * @param {FileInfo} fileInfo
    * @returns {Promise.<any>}
    */
   async afterScan(fileInfo) {

packages/server/gulpfile.js

@@ -22,15 +22,15 @@ const linter = () => {
 const app = {
   server: {
     lint: () => {
-      return src(['./src/*.js', './config/config.default.js', './test/**/*.js', 'gulpfile.js'])
+      return src(['./src/**/*.js', './config/config.default.js', './test/**/*.js', 'gulpfile.js'])
         .pipe(linter())
         .pipe(eslint.format())
         .pipe(eslint.failAfterError());
     },
-  
+
     build: () => {
       const shellFilter = filter('**/*.sh', {restore: true});
-  
+
       const common = src([
         './installer.sh',
         './scanservjs.service',
@@ -42,14 +42,14 @@ const app = {
         .pipe(chmod(0o755))
         .pipe(shellFilter.restore)
         .pipe(dest(DIST));
-    
+
       const source = src(['./src/**/*'])
         .pipe(dest(`${DIST}/server/`));
-    
-      return merge(common, source);  
+
+      return merge(common, source);
     },
   },
-  
+
   package: () => {
     const filename = `scanservjs_v${version}_${dayjs().format('YYYYMMDD.HHmmss')}.tar`;
     const shellFilter = filter('**/*.sh', {restore: true});

packages/server/src/api.js

@@ -1,23 +1,23 @@
 const log = require('loglevel').getLogger('Api');
-const Config = require('./config');
-const Context = require('./context');
-const Devices = require('./devices');
-const FileInfo = require('./file-info');
-const Filters = require('./filters');
-const Process = require('./process');
-const Request = require('./request');
-const Scanimage = require('./scanimage');
+
+const FileInfo = require('./classes/file-info');
+const Process = require('./classes/process');
+const Request = require('./classes/request');
 const ScanController = require('./scan-controller');
-const System = require('./system');
 
-class Api {
+const application = require('./application');
+const config = application.config();
+const system = application.system();
+const scanimageCommand = application.scanimageCommand();
+
+module.exports = new class Api {
 
   /**
    * @returns {Promise.<FileInfo[]>}
    */
-  static async fileList() {
+  async fileList() {
     log.trace('fileList()');
-    const dir = FileInfo.create(Config.outputDirectory);
+    const dir = FileInfo.create(config.outputDirectory);
     let files = await dir.list();
     files = files
       .filter(f => ['.tif', '.jpg', '.png', '.pdf', '.txt', '.zip'].includes(f.extension.toLowerCase()))
@@ -27,35 +27,35 @@ class Api {
   }
 
   /**
-   * @param {string} name 
+   * @param {string} name
    * @returns {FileInfo}
    */
-  static fileDelete(name) {
+  fileDelete(name) {
     log.trace('fileDelete()');
-    const file = FileInfo.unsafe(Config.outputDirectory, name);
+    const file = FileInfo.unsafe(config.outputDirectory, name);
     return file.delete();
   }
 
   /**
-   * @param {ScanRequest} req 
+   * @param {ScanRequest} req
    * @returns {Promise<any>}
    */
-  static async createPreview(req) {
-    const context = await Context.create();
+  async createPreview(req) {
+    const context = await application.context();
     const request = new Request(context).extend({
       params: {
         deviceId: req.params.deviceId,
         mode: req.params.mode,
         source: req.params.source,
-        resolution: Config.previewResolution,
+        resolution: config.previewResolution,
         brightness: req.params.brightness,
         contrast: req.params.contrast,
         dynamicLineart: req.params.dynamicLineart,
         isPreview: true
       }
     });
 
-    const cmd = `${Scanimage.scan(request)}`;
+    const cmd = `${scanimageCommand.scan(request)}`;
     log.trace('Executing cmd:', cmd);
     await Process.spawn(cmd);
     return {};
@@ -64,78 +64,78 @@ class Api {
   /**
    * @returns {FileInfo}
    */
-  static deletePreview() {
+  deletePreview() {
     log.trace('deletePreview()');
-    const file = FileInfo.create(`${Config.previewDirectory}/preview.tif`);
+    const file = FileInfo.create(`${config.previewDirectory}/preview.tif`);
     return file.delete();
   }
 
   /**
    * @param {string[]} filters
    * @returns {Promise.<Buffer>}
    */
-  static async readPreview(filters) {
+  async readPreview(filters) {
     log.trace('readPreview()', filters);
     // The UI relies on this image being the correct aspect ratio. If there is a
-    // preview image then just use it. 
-    const source = FileInfo.create(`${Config.previewDirectory}/preview.tif`);
+    // preview image then just use it.
+    const source = FileInfo.create(`${config.previewDirectory}/preview.tif`);
     if (source.exists()) {
       const buffer = source.toBuffer();
-      const cmds = [...Config.previewPipeline.commands];
+      const cmds = [...config.previewPipeline.commands];
       if (filters && filters.length) {
-        const params = Filters.build(filters, true);
+        const params = application.filterBuilder().build(filters, true);
         cmds.splice(0, 0, `convert - ${params} tif:-`);
       }
-    
+
       return await Process.chain(cmds, buffer, { ignoreErrors: true });
     }
 
     // If not then it's possible the default image is not quite the correct aspect ratio
-    const buffer = FileInfo.create(`${Config.previewDirectory}/default.jpg`).toBuffer();
+    const buffer = FileInfo.create(`${config.previewDirectory}/default.jpg`).toBuffer();
 
     try {
       // We need to know the correct aspect ratio from the device
-      const context = await Context.create();
+      const context = await application.context();
       const device = context.getDevice();
       const heightByWidth = device.features['-y'].limits[1] / device.features['-x'].limits[1];
       const width = 868;
       const height = Math.round(width * heightByWidth);
-      return await Process.spawn(`convert - -resize ${width}x${height}! jpg:-`, buffer);  
+      return await Process.spawn(`convert - -resize ${width}x${height}! jpg:-`, buffer);
     } catch (e) {
       return Promise.resolve(buffer);
     }
   }
 
   /**
-   * @param {string} name 
+   * @param {string} name
    * @returns {Promise.<Buffer>}
    */
-  static async readThumbnail(name) {
-    const source = FileInfo.unsafe(Config.outputDirectory, name);
+  async readThumbnail(name) {
+    const source = FileInfo.unsafe(config.outputDirectory, name);
     return await Process.spawn(`convert '${source.fullname}'[0] -resize 256 -quality 75 jpg:-`);
   }
 
   /**
-   * @param {ScanRequest} req 
+   * @param {ScanRequest} req
    * @returns {ScanResponse}
    */
-  static async scan(req) {
+  async scan(req) {
     return await ScanController.run(req);
   }
 
   /**
    * @returns {void}
    */
-  static deleteContext() {
-    Devices.reset();
+  deleteContext() {
+    application.deviceReset();
     this.deletePreview();
   }
 
   /**
    * @returns {Promise.<Context>}
    */
-  static async readContext() {
-    const context = await Context.create();
+  async readContext() {
+    const context = await application.context();
     context.filters = context.filters.map(f => f.description);
     context.pipelines = context.pipelines.map(p => p.description);
     return context;
@@ -144,9 +144,7 @@ class Api {
   /**
    * @returns {Promise.<SystemInfo>}
    */
-  static async readSystem() {
-    return System.info();
+  async readSystem() {
+    return system.info();
   }
-}
-
-module.exports = Api;
+};