Send 404 when env is invalid. Don't use default importmap file. Resolves

 #96. (#97)

src/cache-control.js

@@ -1,6 +1,7 @@
-const { config } = require("./config");
+const { getConfig } = require("./config");
 
 exports.getCacheControl = (hostSpecificCacheControl) => {
+  const config = getConfig();
   if (config.cacheControl) {
     return config.cacheControl;
   }

src/config.js

@@ -20,7 +20,5 @@ if (argv._.length === 0) {
   }
 }
 
-exports.config = config;
-
 exports.setConfig = (newConfig) => (config = newConfig);
 exports.getConfig = () => config;

src/io-methods/default.js

@@ -1,30 +1,26 @@
 "use strict";
 const _ = require("lodash");
-const fs = require("./filesystem");
+const filesystem = require("./filesystem");
 const s3 = require("./s3");
 const azure = require("./azure");
 const google = require("./google-cloud-storage");
-const config = require("../config").config;
+const { getConfig } = require("../config");
 const memory = require("./memory");
 
-const defaultFilePath =
-  config && config.manifestFormat === "importmap"
-    ? "import-map.json"
-    : "sofe-manifest.json";
-
 function getFilePath(env) {
+  const config = getConfig();
   if (_.has(config, ["locations", env])) {
     return config.locations[env];
   } else if (_.has(config, ["locations", "default"])) {
     return config.locations.default;
-  } else if (!_.isEmpty(config.locations)) {
+  } else if (!env && !_.isEmpty(config.locations)) {
     return config.locations[Object.keys(config.locations)[0]];
   } else {
-    return defaultFilePath;
+    throw Error(`No such environment '${env}'. Check your configuration file.`);
   }
 }
 
-exports.readManifest = function (env) {
+exports.readManifest = async function (env) {
   var filePath = getFilePath(env);
   if (usesAzure(filePath)) {
     //uses azure
@@ -38,11 +34,11 @@ exports.readManifest = function (env) {
     return memory.readManifest(filePath);
   } else {
     //use local file
-    return fs.readManifest(filePath);
+    return filesystem.readManifest(filePath);
   }
 };
 
-exports.writeManifest = function (data, env) {
+exports.writeManifest = async function (data, env) {
   var filePath = getFilePath(env);
 
   if (usesAzure(filePath)) {
@@ -57,7 +53,7 @@ exports.writeManifest = function (data, env) {
     return memory.writeManifest(filePath, data);
   } else {
     //use local file
-    return fs.writeManifest(filePath, data);
+    return filesystem.writeManifest(filePath, data);
   }
 };
 

src/io-methods/filesystem.js

@@ -1,47 +1,17 @@
 "use strict";
-const fs = require("fs");
-const config = require("../config").config;
+const fs = require("fs/promises");
+const { getConfig } = require("../config");
 const jsHelpers = require("./js-file-helpers.js");
 
 exports.readManifest = function (filePath) {
-  return new Promise((resolve, reject) => {
-    //create file if not already created
-    fs.open(filePath, "a", function (err, fd) {
-      if (err) reject(`Could not open file ${filePath}`);
-      else {
-        fs.readFile(filePath, "utf8", function (err2, data) {
-          if (err2) {
-            console.error(err2);
-            reject(`Could not read file ${filePath}`);
-          } else resolve(data);
-        });
-      }
-    });
-  });
+  return fs.readFile(filePath, "utf-8");
 };
 
 exports.writeManifest = function (filePath, data) {
-  const jsonPromise = new Promise((resolve, reject) => {
-    fs.writeFile(filePath, data, function (err) {
-      if (err) reject(`Could not write file ${filePath}`);
-      else resolve();
-    });
-  });
+  const config = getConfig();
+  const useJsHelpers = config && config.writeJsFile;
+  const finalFilePath = useJsHelpers ? jsHelpers.getJsPath(filePath) : filePath;
+  const finalData = useJsHelpers ? jsHelpers.createJsString(data) : data;
 
-  const jsPromise = new Promise((resolve, reject) => {
-    if (!config || !config.writeJsFile) {
-      resolve();
-    } else {
-      fs.writeFile(
-        jsHelpers.getJsPath(filePath),
-        jsHelpers.createJsString(data),
-        function (err) {
-          if (err) reject(`Could not write file ${filePath}`);
-          else resolve();
-        }
-      );
-    }
-  });
-
-  return Promise.all([jsonPromise, jsPromise]);
+  return fs.writeFile(finalFilePath, finalData, "utf-8");
 };

src/io-methods/s3.js

@@ -1,11 +1,11 @@
 "use strict";
 
 const aws = require("aws-sdk"),
-  config = require("../config").config,
+  getConfig = require("../config").getConfig,
   jsHelpers = require("./js-file-helpers.js");
 
-if (config && config.region) {
-  aws.config.update({ region: config.region });
+if (getConfig() && getConfig().region) {
+  aws.config.update({ region: getConfig().region });
 }
 const { getCacheControl } = require("../cache-control");
 
@@ -24,16 +24,16 @@ function parseFilePath(filePath) {
 
 let s3PutObjectCacheControl;
 let s3PutObjectConfigSansCacheControl = {};
-if (config && config.s3 && config.s3.putObject) {
-  const { CacheControl, ...rest } = config.s3.putObject;
+if (getConfig() && getConfig().s3 && getConfig().s3.putObject) {
+  const { CacheControl, ...rest } = getConfig().s3.putObject;
   s3PutObjectCacheControl = CacheControl;
   s3PutObjectConfigSansCacheControl = { ...rest };
 }
 
 const cacheControl = getCacheControl(s3PutObjectCacheControl);
 
 const s3 = new aws.S3({
-  endpoint: config.s3Endpoint,
+  endpoint: getConfig().s3Endpoint,
 });
 
 exports.readManifest = function (filePath) {
@@ -56,6 +56,7 @@ exports.readManifest = function (filePath) {
 };
 
 exports.writeManifest = function (filePath, data) {
+  const config = getConfig();
   const jsonPromise = new Promise(function (resolve, reject) {
     const file = parseFilePath(filePath);
     s3.putObject(

src/modify.js

@@ -2,7 +2,7 @@
 // File editing
 const lock = new (require("rwlock"))();
 const ioOperations = require("./io-operations.js");
-const getConfig = require("./config").getConfig;
+const { getConfig } = require("./config");
 
 const isImportMap = () => {
   const format = getConfig().manifestFormat;

src/trusted-urls.js

@@ -1,6 +1,7 @@
-const { config } = require("./config");
+const { getConfig } = require("./config");
 
 exports.checkUrlUnsafe = (urlStr) => {
+  const config = getConfig();
   if (!config.urlSafeList) {
     return null;
   } else {

src/web-server.js

@@ -58,11 +58,13 @@ app.use(auth);
 app.use(express.static(__dirname + "/public"));
 
 function getEnv(req) {
-  if (req.query.env === undefined) {
-    return "default";
-  } else {
-    return req.query.env;
-  }
+  return req.query.env;
+}
+
+function sendError(res, ex, prefix) {
+  const status =
+    ex && ex.message && /No such environment/.test(ex.message) ? 404 : 500;
+  res.status(status).send(`${prefix} -- ${ex.toString()}`);
 }
 
 // --------- //
@@ -119,7 +121,7 @@ function handleGetManifest(req, res) {
     })
     .catch((ex) => {
       console.error(ex);
-      res.status(500).send(`Could not read manifest file -- ${ex.toString()}`);
+      sendError(res, ex, "Could not read import map");
     });
 }
 
@@ -206,8 +208,7 @@ app.patch("/import-map.json", (req, res) => {
           res.status(200).send(newImportMap);
         })
         .catch((err) => {
-          console.error(err);
-          res.status(500).send(`Could not update import map`);
+          sendError(res, err, "Could not update import map");
         });
     })
     .catch((err) => {
@@ -266,10 +267,7 @@ app.patch("/services", function (req, res) {
           res.send(json);
         })
         .catch((ex) => {
-          console.error(ex);
-          res
-            .status(500)
-            .send(`Could not write manifest file -- ${ex.toString()}`);
+          sendError(res, ex, "Could not patch service");
         });
     })
     .catch((err) => {
@@ -285,10 +283,7 @@ app.delete("/services/:serviceName", function (req, res) {
       res.send(data);
     })
     .catch((ex) => {
-      console.error(ex);
-      res
-        .status(500)
-        .send(`Could not delete service ${req.params.serviceName}`);
+      sendError(res, ex, "Could not delete service");
     });
 });
 

test/import-map.test.js

@@ -188,4 +188,54 @@ describe(`/import-map.json`, () => {
     expect(response.body.imports.b).not.toBeDefined();
     expect(response.body.imports["b/"]).not.toBeDefined();
   });
+
+  it(`returns a 404 when you try to retrieve an import map for an environment that doesn't exist`, async () => {
+    await request(app)
+      .get("/import-map.json")
+      .query({
+        env: "envThatDoesntExist",
+      })
+      .expect(404);
+  });
+
+  it(`returns a 404 when you attempt to patch an import map environment that doesn't exist`, async () => {
+    const response = await request(app)
+      .patch("/import-map.json")
+      .query({
+        skip_url_check: true,
+        env: "envThatDoesntExist",
+      })
+      .set("accept", "json")
+      .send({
+        imports: {
+          a: "/a-1.mjs",
+        },
+      })
+      .expect(404);
+  });
+
+  it(`returns a 404 when you attempt to patch a service for an environment that doesn't exist`, async () => {
+    await request(app)
+      .patch("/services")
+      .query({
+        skip_url_check: true,
+        env: "envThatDoesntExist",
+      })
+      .set("accept", "json")
+      .send({
+        service: "a",
+        url: "/a-1-updated.mjs",
+      })
+      .expect(404);
+  });
+
+  it(`returns a 404 when you attempt to delete a service for an environment that doesn't exist`, async () => {
+    await request(app)
+      .delete("/services/b")
+      .query({
+        env: "envThatDoesntExist",
+      })
+      .set("accept", "json")
+      .expect(404);
+  });
 });