Merge branch 'trs/grab-bag-ep2'

.gitignore

@@ -15,4 +15,7 @@ scratch
 /sessions/
 
 # generated data
-/data
+/data
+
+# test logs
+/test/server.log

README.md

@@ -193,15 +193,21 @@ See the [infrastructure documentation](./docs/infrastructure.md) for details.
 
 ---
 ## Testing
-Nextstrain.org currently implements limited smoke-testing to test requests originiating from the Auspice client to the nextstrain.org server (i.e. API GET requests starting with `/charon/`).
-These are defined [in this JSON file](./test/smoke-test/auspice_client_requests.json) and can be run via
+Nextstrain.org currently uses limited automated testing defined in `test/*.test.js` files.
+Run the tests with:
 
-```
-npm run test:ci
-```
+    npm run test:ci
+
+This will run a local server for the duration of the tests.
+Alternatively, you can run your own server in the background and run:
+
+    npm run test
+
+instead.
+
+To run a single test or small number of test files, run a local server and invoke Jest directly, for example:
 
-Which will run a local server for the duration of the tests.
-Alternatively, you can run your own server in the background and run `npm run smoke-test`.
+    npx jest --run-tests-by-path test/routing.test.js
 
 ---
 

docs/api.md

@@ -23,5 +23,5 @@ if (!source.visibleToUser(req.user)) {
 
 ## Tests
 
-There are a number of smoke-tests for these API calls.
-See `tests/smoke-test/` for details and run via `npm run smoke-test:ci`
+There are a number of tests for these API calls.
+See `test/auspice_client_requests.test.js` for details and run via `npm run test:ci`

docs/glossary.md

@@ -74,3 +74,29 @@ A named set of [users](#user) with access to see and update a set of builds.
 Each group has a related [source](#source), which typically authorizes access based on the group.
 Managed in an AWS Cognito User Pool called `nextstrain.org`.
 Not to be confused with AWS IAM groups.
+
+## Route
+
+A URL path (e.g. `/zika`), possibly parameterized (e.g. `/groups/:groupName/*`), at which nextstrain.org provides content.
+Routes map paths to resources (i.e. HTML pages, images, datasets, narratives, etc).
+
+The process of finding a matching route, if any exists, is called _routing_.
+
+The Express server performs server-side routing, which means it responds to HTTP requests for a matching path.
+Express route declarations include the stack of [request handlers](#request-handler) to use.
+
+The Gatsby app performs client-side routing, which means it displays content based on the browser's `location`.
+
+## Request handler
+
+Either an [endpoint](#endpoint) or [middleware](#middleware).
+Sometimes called an _Express handler_, _route handler_, or just _handler_.
+
+## Endpoint
+
+Function that sends the response to a request.
+Specific to an HTTP method and a resource or type of resource.
+
+## Middleware
+
+Function that either responds to a request or does some preparation and then hands processing to the next [handler](#request-handler) in the stack.

package.json

@@ -16,12 +16,8 @@
     "set-up": "npm run build",
     "server": "node server.js",
     "start": "npm run server",
-    "smoke-test": "NODE_ENV=test ENV=dev jest ./test/smoke-test/auspice_client_requests.test.js",
-    "e2e-test": "NODE_ENV=test ENV=dev jest ./test/end-to-end/*.test.js",
-    "e2e-test:ci": "start-server-and-test server http://localhost:5000 e2e-test",
-    "smoke-test:ci": "start-server-and-test server http://localhost:5000 smoke-test",
-    "test:ci": "npm run smoke-test:ci && npm run e2e-test:ci;",
-    "test:unit": "jest ./test/unit*",
+    "test": "jest --roots ./test/",
+    "test:ci": "start-server-and-test 'node server.js --verbose >./test/server.log 2>&1' http://localhost:5000 test",
     "dev": "./develop.sh"
   },
   "dependencies": {
@@ -54,7 +50,6 @@
     "query-string": "^4.2.3",
     "react-icons": "^3.11.0",
     "request": "^2.88.0",
-    "serve-favicon": "^2.5.0",
     "session-file-store": "^1.3.1",
     "yaml-front-matter": "^4.0.0"
   },
@@ -81,6 +76,9 @@
     "static-site/node_modules"
   ],
   "jest": {
+    "testMatch": [
+      "**/*.test.js"
+    ],
     "globals": {
       "BASE_URL": "http://localhost:5000"
     }

src/app.js

@@ -2,8 +2,6 @@
 const sslRedirect = require('heroku-ssl-redirect');
 const nakedRedirect = require('express-naked-redirect');
 const express = require("express");
-const expressStaticGzip = require("express-static-gzip");
-const favicon = require('serve-favicon');
 const compression = require('compression');
 const utils = require("./utils");
 const cors = require('cors');
@@ -14,7 +12,7 @@ const production = process.env.NODE_ENV === "production";
 
 const charon = require("./endpoints/charon");
 const users = require("./endpoints/users");
-const {assetPath, auspiceAssetPath, gatsbyAssetPath, sendAuspiceEntrypoint, sendGatsbyEntrypoint, sendGatsbyPage, sendGatsby404} = require("./endpoints/static");
+const {auspiceAssets, gatsbyAssets, sendAuspiceEntrypoint, sendGatsbyEntrypoint, sendGatsbyPage, sendGatsby404} = require("./endpoints/static");
 const {setSource, setGroupSource, setDataset, setNarrative, canonicalizeDataset, ifDatasetExists, ifNarrativeExists} = require("./endpoints/sources");
 const authn = require("./authn");
 const redirects = require("./redirects");
@@ -36,8 +34,6 @@ if (production) app.enable("trust proxy");
 app.use(sslRedirect()); // redirect HTTP to HTTPS
 app.use(compression()); // send files (e.g. res.json()) using compression (if possible)
 app.use(nakedRedirect({reverse: true})); // redirect www.nextstrain.org to nextstrain.org
-app.use(favicon(assetPath("favicon.ico")));
-app.use('/favicon.png', express.static(assetPath("favicon.png")));
 
 
 /* Setup a request-scoped context object for passing arbitrary request-local
@@ -181,7 +177,7 @@ app.routeAsync(["/community/:user/:repo", "/community/:user/:repo/*"])
 /* Datasets and narratives at ~arbitrary remote URLs.
  */
 app.use(["/fetch/narratives/:authority", "/fetch/:authority"],
-  setSource("fetch", req => req.params.authority));
+  setSource("fetch", req => [req.params.authority]));
 
 app.routeAsync("/fetch/narratives/:authority/*")
   .all(setNarrative(req => req.params[0]))
@@ -257,7 +253,7 @@ app.route(["/users", "/users/:name"])
  * so we must use that prefix here too.
  */
 app.route("/dist/*")
-  .all(expressStaticGzip(auspiceAssetPath(), {fallthrough: false, maxAge: '30d'}));
+  .all(auspiceAssets, (req, res, next) => next(new NotFound()));
 
 
 /* Gatsby static HTML pages and other assets.
@@ -281,7 +277,7 @@ if (app.locals.gatsbyDevUrl) {
    */
   app.use(createProxyMiddleware(app.locals.gatsbyDevUrl));
 } else {
-  app.use(express.static(gatsbyAssetPath()));
+  app.use(gatsbyAssets);
 }
 
 

src/endpoints/charon/getDataset.js

@@ -2,7 +2,7 @@ const queryString = require("query-string");
 
 const utils = require("../../utils");
 const {canonicalizePrefix, parsePrefix} = require("../../utils/prefix");
-const {NoDatasetPathError} = require("../../exceptions");
+const {NoResourcePathError} = require("../../exceptions");
 const auspice = require("auspice");
 const request = require('request');
 const {BadRequest, NotFound, InternalServerError} = require("http-errors");
@@ -60,7 +60,7 @@ const sendV1Dataset = async (res, metaJsonUrl, treeJsonUrl) => {
  * @throws {NotFound} Throws if the dataset didn't exist (or the streaming failed)
  */
 const streamMainV2Dataset = async (res, dataset) => {
-  const main = await dataset.urlFor("main");
+  const main = await dataset.subresource("main").url();
   try {
     await new Promise((resolve, reject) => {
       let statusCode;
@@ -126,7 +126,7 @@ const getDataset = async (req, res) => {
      * Note that this leaks the existence of private sources, but I think
      * broader discussions are leaning towards that anyhow.
      */
-    if (err instanceof NoDatasetPathError) {
+    if (err instanceof NoResourcePathError) {
       utils.verbose(err.message);
       return res.status(204).end();
     }
@@ -156,7 +156,7 @@ const getDataset = async (req, res) => {
   }
 
   if (query.type) {
-    const url = await dataset.urlFor(query.type);
+    const url = await dataset.subresource(query.type).url();
     return requestCertainFileType(res, req, url, query);
   }
 
@@ -166,7 +166,7 @@ const getDataset = async (req, res) => {
   } catch (errV2) {
     try {
       /* attempt to fetch the meta + tree JSONs, combine, and send */
-      return await sendV1Dataset(res, await dataset.urlFor("meta"), await dataset.urlFor("tree"));
+      return await sendV1Dataset(res, await dataset.subresource("meta").url(), await dataset.subresource("tree").url());
     } catch (errV1) {
       if (dataset.isRequestValidWithoutDataset) {
         utils.verbose("Request is valid, but no dataset available. Returning 204.");

src/endpoints/charon/getNarrative.js

@@ -41,7 +41,7 @@ const getNarrative = async (req, res) => {
 
   // Generate the narrative's origin URL for fetching.
   const narrative = source.narrative(prefixParts);
-  const fetchURL = await narrative.url();
+  const fetchURL = await narrative.subresource("md").url();
 
   try {
     utils.log(`Fetching narrative ${fetchURL} and streaming to client for parsing`);

src/endpoints/sources.js

@@ -101,7 +101,7 @@ const canonicalizeDataset = (pathBuilder) => (req, res, next) => {
  * Express middleware function that throws a {@link NotFound} error if {@link
  * Dataset#exists} returns false.
  *
- * @type {expressMiddleware}
+ * @type {expressMiddlewareAsync}
  */
 const ifDatasetExists = async (req, res, next) => {
   if (!(await req.context.dataset.exists())) throw new NotFound();
@@ -129,7 +129,7 @@ const setNarrative = (pathExtractor) => (req, res, next) => {
  * Express middleware function that throws a {@link NotFound} error if {@link
  * Narrative#exists} returns false.
  *
- * @type {expressMiddleware}
+ * @type {expressMiddlewareAsync}
  */
 const ifNarrativeExists = async (req, res, next) => {
   if (!(await req.context.narrative.exists())) throw new NotFound();
@@ -191,6 +191,14 @@ function pathParts(path = "") {
  * @param {Function} next
  */
 
+/**
+ * @callback expressMiddlewareAsync
+ * @async
+ * @param {express.request} req
+ * @param {express.response} res
+ * @param {Function} next
+ */
+
 
 module.exports = {
   setSource,

src/endpoints/static.js

@@ -1,10 +1,12 @@
+const express = require("express");
+const expressStaticGzip = require("express-static-gzip");
 const {InternalServerError} = require("http-errors");
 const path = require("path");
 
 const utils = require("../utils");
 
 
-/* Path helpers for static assets, to make routes more readable.
+/* Path helpers for our handlers below.
  */
 const assetPath = (...subpath) =>
   path.join(__dirname, "..", "..", ...subpath);
@@ -16,6 +18,13 @@ const gatsbyAssetPath = (...subpath) =>
   assetPath("static-site", "public", ...subpath);
 
 
+/* Handlers for static assets.
+ */
+const auspiceAssets = expressStaticGzip(auspiceAssetPath(), {maxAge: '30d'});
+
+const gatsbyAssets = express.static(gatsbyAssetPath());
+
+
 /**
  * Send the file at the given `filePath` as the response.
  *
@@ -148,9 +157,8 @@ const sendGatsby404 = async (req, res) => {
 
 
 module.exports = {
-  assetPath,
-  auspiceAssetPath,
-  gatsbyAssetPath,
+  auspiceAssets,
+  gatsbyAssets,
 
   sendFile,
   sendAuspiceEntrypoint,

src/exceptions.js

@@ -10,23 +10,13 @@ class NextstrainError extends Error {
 /* Thrown when a valid Source object is asked to create a new Dataset object
  * without a dataset path.
  */
-class NoDatasetPathError extends NextstrainError {
-  constructor(msg = "No dataset path provided", ...rest) {
-    super(msg, ...rest);
-  }
-}
-
-/* Thrown when a Source (sub-)class is improperly accessedobject is asked to create a new Dataset object
- * without a dataset path.
- */
-class InvalidSourceImplementation extends NextstrainError {
-  constructor(msg = "Invalid implementation of Source (sub-)class", ...rest) {
+class NoResourcePathError extends NextstrainError {
+  constructor(msg = "No resource path provided", ...rest) {
     super(msg, ...rest);
   }
 }
 
 
 module.exports = {
-  InvalidSourceImplementation,
-  NoDatasetPathError
+  NoResourcePathError
 };

src/negotiate.js

@@ -27,10 +27,9 @@ function contentTypesProvided(providers) {
       throw new NotAcceptable();
     }
 
-    const normalizedContentType = mime.getType(contentType);
-    if (normalizedContentType) {
-      res.set("Content-Type", normalizedContentType);
-    }
+    const normalizedContentType = mime.getType(contentType) || contentType;
+
+    res.set("Content-Type", normalizedContentType);
 
     return await handlers[contentType](req, res, next);
   };