Merge pull request #1 from ubports/neothethird-dev

Improve coverage and code quality
ubports · Mar 7, 2018 · 11a18bf · 11a18bf
2 parents 58591af + b03da1c
commit 11a18bf
Show file tree

Hide file tree

Showing 9 changed files with 293 additions and 62 deletions.
diff --git a/.gitignore b/.gitignore
@@ -63,3 +63,7 @@ typings/
 
 
 # End of https://www.gitignore.io/api/node
+
+
+# ignore test output
+test
diff --git a/README.md b/README.md
@@ -1,21 +1,28 @@
-# System image nodejs module
+# System image node module
 
 [![Build Status](https://travis-ci.org/ubports/system-image-node-module.svg?branch=master)](https://travis-ci.org/ubports/system-image-node-module) [![Coverage Status](https://coveralls.io/repos/github/ubports/system-image-node-module/badge.svg?branch=master)](https://coveralls.io/github/ubports/system-image-node-module?branch=master)
 
 ## Client
 Access a system-image server http endpoint
 
+Example:
 
-examples:
-
-```
-const systemImageClient = require("ubports-system-image").Client;
-
+```javascript
+const systemImageClient = require("system-image-node-module").Client;
 const sic = new systemImageClient();
-
 sic.getDeviceChannels("bacon").then((channels) => console.log(channels));
 ```
 
+The constructor takes an object with optional properties as an argument. The default properties are listed below.
+
+```javascript
+{
+  host: "https://system-image.ubports.com/", // URL of the system-image server
+  path: "./test",                            // download path
+  allow_insecure: false                      // allow unencrypted URL
+  cache_time: 180                            // time to keep cached files
+}
+```
 
 ## Server
-Access and maintain a system-image server backend
+Maintain a system-image server backend (not implemented yet)
diff --git a/src/client.js b/src/client.js
@@ -22,20 +22,21 @@ const os = require("os");
 const fs = require("fs");
 const path = require("path");
 const mkdirp = require('mkdirp');
+const common = require("./common.js")
 
 const time = () => Math.floor(new Date() / 1000);
 
 const startCommands = "format system\n\
 load_keyring image-master.tar.xz image-master.tar.xz.asc\n\
 load_keyring image-signing.tar.xz image-signing.tar.xz.asc\n\
-mount system"
-const endCommands = "\nunmount system\n"
+mount system";
+const endCommands = "\nunmount system\n";
 const DEFAULT_HOST = "https://system-image.ubports.com/";
 const DEFAULT_CACHE_TIME = 180; // 3 minutes
 const downloadPath = "./test";
 const ubuntuCommandFile = "ubuntu_command";
-const ubuntuPushDir = "/cache/recovery/"
-const gpg = ["image-signing.tar.xz", "image-signing.tar.xz.asc", "image-master.tar.xz", "image-master.tar.xz.asc"]
+const ubuntuPushDir = "/cache/recovery/";
+const gpg = ["image-signing.tar.xz", "image-signing.tar.xz.asc", "image-master.tar.xz", "image-master.tar.xz.asc"];
 
 class Client {
   constructor(options) {
@@ -47,32 +48,45 @@ class Client {
     this.channelsIndex = {};
     this.channelsIndexCache = 0;
 
+    // accept options
     if (options) {
-      if (options.host)
-        this.host = options.host
-      if (options.port)
-        this.port = options.port
-      if (options.path)
-        this.path = options.path
-      if (options.cache_time)
+      if (options.host) {
+        // validate URL
+        if (options.host.match(/https?:\/\/(www\.)?[-a-z0-9@:%._\+~#=]{2,256}\.[a-z]{2,6}\b([-a-z0-9@:%_\+.~#?&//=]*)/i)) {
+          // ensure https
+          if (!options.allow_insecure && options.host.includes("http://")) {
+            throw new Error("Insecure URL! Call with allow_insecure to ignore.");
+          }
+          // ensure trailing slash
+          this.host = options.host + (options.host.slice(-1) != "/" ? "/" : "");
+        } else {
+          throw new Error("Host is not a valid URL!");
+        }
+      }
+      if (options.path) {
+        this.path = options.path;
+      }
+      if (options.cache_time) {
         this.cache_time = options.cache_time;
+      }
     }
   }
 
   // Install commands
+  // QUESTION Should this be public? Maybe call it from createInstallCommandsFile
   createInstallCommands(files, installerCheck, wipe, enable) {
     var cmd = startCommands;
-    if (wipe === true) cmd += "\nformat data"
+    if (wipe === true) cmd += "\nformat data";
     if (files.constructor !== Array)
       return false;
     files.forEach((file) => {
       cmd += "\nupdate " + path.basename(file.path) + " " + path.basename(file.signature);
-    })
+    });
     if (enable) {
       if (enable.constructor === Array) {
         enable.forEach((en) => {
           cmd += "\nenable " + en;
-        })
+        });
       }
     }
     cmd += endCommands;
@@ -84,12 +98,13 @@ class Client {
     if (!fs.existsSync(downloadPath + "/commandfile/")) {
       mkdirp.sync(downloadPath + "/commandfile/");
     }
-    var file = downloadPath + "/commandfile/" + ubuntuCommandFile + device + getRandomInt(1000, 9999);
+    var file = downloadPath + "/commandfile/" + ubuntuCommandFile + device + common.getRandomInt(1000, 9999);
     fs.writeFileSync(file, cmds);
     return file;
   }
 
   // HTTP functions
+  // QUESTION Should this be public?
   getChannelsIndex() {
     const _this = this;
     return new Promise(function(resolve, reject) {
@@ -111,12 +126,13 @@ class Client {
     });
   }
 
+  // QUESTION Should this be public?
   getDeviceIndex(device, channel) {
     var _this = this;
     return new Promise(function(resolve, reject) {
       var now=time();
       if (_this.deviceIndexCache > now)
-        return resolve()
+        return resolve();
       http.get({
         url: _this.host + channel + "/" + device + "/index.json",
         json: true
@@ -144,7 +160,6 @@ class Client {
     });
   }
 
-
   getDeviceChannels(device) {
     return this.getChannelsIndex().then((channels) => {
       var deviceChannels = [];
@@ -159,6 +174,7 @@ class Client {
     });
   }
 
+  // FIXME: Some people might perfer to get the latest *version* instead
   getLatestVesion(device, channel) {
     return this.getDeviceIndex(device, channel).then((index) => {
       //TODO optimize with searching in reverse, but foreach is safer
@@ -168,7 +184,7 @@ class Client {
         if (img.type === "full" && (!latest || latest.version < img.version)) {
           latest = img;
         }
-      })
+      });
       return latest;
     });
   }
@@ -177,10 +193,10 @@ class Client {
     var gpgUrls = [];
     gpg.forEach((g) => {
       gpgUrls.push({
-        url: this.host + "/gpg/" + g,
+        url: this.host + "gpg/" + g,
         path: downloadPath + "gpg"
-      })
-    })
+      });
+    });
     return gpgUrls;
   }
 
@@ -191,15 +207,16 @@ class Client {
         url: this.host + file.path,
         path: downloadPath + "pool",
         checksum: file.checksum
-      })
+      });
       ret.push({
         url: this.host + file.signature,
         path: downloadPath + "pool"
-      })
-    })
+      });
+    });
     return ret;
   }
 
+  // QUESTION Is this still needed?
   getFileBasenameArray(urls) {
     var files = [];
     urls.forEach((url) => {
@@ -208,6 +225,7 @@ class Client {
     return files;
   }
 
+  // QUESTION Is this still needed?
   getFilePathArray(urls) {
     var files = [];
     urls.forEach((url) => {
@@ -228,4 +246,4 @@ class Client {
   }
 }
 
-module.exports = Client
+module.exports = Client;
diff --git a/src/common.js b/src/common.js
@@ -20,3 +20,5 @@ function getRandomInt(min, max) {
     max = Math.floor(max);
     return Math.floor(Math.random() * (max - min)) + min;
 }
+
+module.exports = { getRandomInt: getRandomInt };
diff --git a/tests/test-data/commandfile.json b/tests/test-data/commandfile.json
@@ -0,0 +1 @@
+"format system\nload_keyring image-master.tar.xz image-master.tar.xz.asc\nload_keyring image-signing.tar.xz image-signing.tar.xz.asc\nmount system\nformat data\nupdate ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz.asc\nupdate device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz.asc\nupdate keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz.asc\nupdate version-261.tar.xz version-261.tar.xz.asc\nenable 1\nenable 2\nenable 3\nunmount system\n\ninstaller_check"
diff --git a/tests/test-data/file-push.json b/tests/test-data/file-push.json
@@ -0,0 +1,34 @@
+[
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz.asc"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz.asc"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz.asc"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/version-261.tar.xz"
+  },
+  {
+    "dest": "/cache/recovery/",
+    "src": "./testpool/version-261.tar.xz.asc"
+  }
+]
diff --git a/tests/test-data/files-urls.json b/tests/test-data/files-urls.json
@@ -0,0 +1,38 @@
+[
+  {
+    "checksum": "777b2d05b2a0f26b2a9a302eb7804e7eab9bf536f130ffae2f15455a80bd509b",
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz"
+  },
+  {
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/ubports-e5cf0885f061c9615931cf863a43f65d8590aa71027315ad71999252ce118868.tar.xz.asc"
+  },
+  {
+    "checksum": "234d719a32a01bc36d64dbb536fcf27d77957916f8c3851916e4d1e44d732364",
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz"
+  },
+  {
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/device-f6863fe2f1f8b47cd90f5721af05ccb0ab4d41f2e1cc318075458646eeda28ee.tar.xz.asc"
+  },
+  {
+    "checksum": "5b6e870acf8944330acb2a9dad6f5270bd06c377603a3115f805290e989ab421",
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz"
+  },
+  {
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//pool/keyring-4c4e7ef380ebcfa2c31084efa199138e93bfed8fc58aa3eb06bdf75a78af9b57.tar.xz.asc"
+  },
+  {
+    "checksum": "2624df95cbfc0c0dbdd0ff14a7cf6de78802bb38f5dd2a3fa75f0e99586dcba5",
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//ubports-touch/15.04/devel/bacon/version-261.tar.xz"
+  },
+  {
+    "path": "./testpool",
+    "url": "https://system-image.ubports.com//ubports-touch/15.04/devel/bacon/version-261.tar.xz.asc"
+  }
+]