[version descriptors] redirect versioned URLs

The desired behaviour is that (e.g.) dengue/denv1@version redirects to
dengue/denv1/genome@version, similarly to how the non-versioned URL
currently behaves. This failed because of three inter-related aspects:

1. The previous commit removed (e.g.) "dengue/denv1" from the index,
   choosing instead to place that data under the "dengue/denv1/genome"
   ID.
2. The server instantiates the Resource (Dataset) class for the request
   before resolving/redirecting the URL. During this instantiation the
   requested version was checked against the index and since the
   resource wasn't present in the index this failed (NotFound error).
   This was solved by deferring the version lookup until after the URL
   resolving/redirecting stage.
3. The redirect URL didn't include the version descriptor.

src/endpoints/sources.js

@@ -43,7 +43,8 @@ export const setDataset = (pathExtractor) => (req, res, next) => {
 
 /**
  * Generate Express middleware that redirects to the canonical path for the
- * current {@link Dataset} if it is not fully resolved.
+ * current {@link Dataset} if it is not fully resolved. Any provided version
+ * descriptor is included in the redirect.
  *
  * @param {pathBuilder} pathBuilder - Function to build a fully-specified path
  * @returns {expressMiddleware}
@@ -54,9 +55,11 @@ export const canonicalizeDataset = (pathBuilder) => (req, res, next) => {
 
   if (dataset === resolvedDataset) return next();
 
+  const version = dataset.versionDescriptor ? `@${dataset.versionDescriptor}` : '';
+
   const canonicalPath = pathBuilder.length >= 2
-    ? pathBuilder(req, resolvedDataset.pathParts.join("/"))
-    : pathBuilder(resolvedDataset.pathParts.join("/"));
+    ? pathBuilder(req, resolvedDataset.pathParts.join("/") + version)
+    : pathBuilder(resolvedDataset.pathParts.join("/") + version);
 
   /* 307 Temporary Redirect preserves request method, unlike 302 Found, which
    * is important since this middleware function may be used in non-GET routes.

src/sources/models.js

@@ -144,7 +144,6 @@ export class Resource {
     this.source = source;
     this.pathParts = pathParts;
     this.versionDescriptor = versionDescriptor;
-    [this.versionDate, this.versionUrls] = this.versionInfo(versionDescriptor);
 
     // Require baseParts, otherwise we have no actual dataset/narrative path.
     // This inspects baseParts because some of the pathParts (above) may not
@@ -241,12 +240,15 @@ export class Subresource {
      * Note that the URL is not signed, and changes will be required to support
      * this as needed.
      */
-    if (this.resource.versionUrls) {
+
+    const versionUrls = this.resource.versionInfo(this.resource.versionDescriptor)[1];
+
+    if (versionUrls) {
       if (!['HEAD', 'GET'].includes(method)) {
         throw new InternalServerError(`Only the GET and HEAD methods are available for previous resource versions`);
       }
-      if (this.resource.versionUrls[this.type]) {
-        return this.resource.versionUrls[this.type];
+      if (versionUrls[this.type]) {
+        return versionUrls[this.type];
       }
       throw new NotFound(`This version of the resource does not have a subresource for ${this.type}`);
     }

test/date_descriptor.test.js

@@ -301,6 +301,54 @@ describe("Valid narratives", () => {
   }
 })
 
+const redirects = [
+  ["WNV", "WNV/NA"],
+  /* Note that WNV is not in the index, so this versioned test ensures we are
+  not checking the existence/validity of any version descriptor against the WNV
+  resource, rather we are redirecting and deferring those checks */
+  ["WNV@2020-01-01", "WNV/NA@2020-01-01"],
+]
+
+describe("Paths redirect with version descriptors", () => {
+  /* See <https://github.com/node-fetch/node-fetch#manual-redirect> for how
+  fetch stores the redirect location when {redirect: 'manual'} */
+
+  redirects.forEach(([fromUrl, toUrl]) => {
+    /* test RESTful API */
+    (['html', 'json']).forEach((type) => {
+      it(`REST API: ${fromUrl} goes to ${toUrl} (${type})`, async () => {
+        const res = await fetchType(`${BASE_URL}/${fromUrl}`, type);
+        expect(res.status).toEqual(307);
+        expect(res.headers.get('location')).toEqual(`${BASE_URL}/${toUrl}`);
+      })
+    })
+
+
+    /* test Charon API */
+    const charonUrl = (path, sidecar) =>
+      `${BASE_URL}/charon/getDataset?prefix=${path}${sidecar ? `&type=root-sequence` : ''}`;
+
+    /* note that the redirect URL of charon requests is constructed by creating
+    a URL object (the RESTful API does not) which results in '/' and '@' being
+    escaped in the query. This isn't necessary, but it's also fine to do so. */
+
+    it(`Charon main dataset: ${fromUrl} goes to ${toUrl} `, async () => {
+      const res = await fetch(charonUrl(fromUrl, false), {redirect: 'manual'});
+      expect(res.status).toEqual(307);
+      expect(decodeURIComponent(res.headers.get('location'))).toEqual(charonUrl(toUrl, false));
+    })
+
+    it(`Charon sidecar: ${fromUrl} goes to ${toUrl} `, async () => {
+      const res = await fetch(charonUrl(fromUrl, true), {redirect: 'manual'});
+      expect(res.status).toEqual(307);
+      expect(decodeURIComponent(res.headers.get('location'))).toEqual(charonUrl(toUrl, true));
+    })
+
+  })
+
+})
+
+
 /**
  * Map the sidecar names we use day-to-day to their content types
  * used within the server