(core) remove deprecated /download endpoint in favor of newer /api/docs/NNNN/download

Summary:
This endpoint has started to fail when called between a pair
of doc workers. The simplest fix is to simply remove it, it serves no
purpose.

Test Plan: added basic deployment test

Reviewers: dsagal

Reviewed By: dsagal

Subscribers: dsagal

Differential Revision: https://phab.getgrist.com/D3636

app/common/UserAPI.ts

@@ -755,8 +755,8 @@ export class DocWorkerAPIImpl extends BaseAPI implements DocWorkerAPI {
   }
 
   public async downloadDoc(docId: string, template: boolean = false): Promise<Response> {
-    const extra = template ? '&template=1' : '';
-    const result = await this.request(`${this.url}/download?doc=${docId}${extra}`, {
+    const extra = template ? '?template=1' : '';
+    const result = await this.request(`${this.url}/api/docs/${docId}/download${extra}`, {
       method: 'GET',
     });
     if (!result.ok) { throw new Error(await result.text()); }

app/server/lib/FlexServer.ts

@@ -46,7 +46,7 @@ import {IPermitStore} from 'app/server/lib/Permit';
 import {getAppPathTo, getAppRoot, getUnpackedAppRoot} from 'app/server/lib/places';
 import {addPluginEndpoints, limitToPlugins} from 'app/server/lib/PluginEndpoint';
 import {PluginManager} from 'app/server/lib/PluginManager';
-import {adaptServerUrl, addOrgToPath, getOrgUrl, getOriginUrl, getScope, optStringParam,
+import {adaptServerUrl, getOrgUrl, getOriginUrl, getScope, optStringParam,
         RequestWithGristInfo, stringParam, TEST_HTTPS_OFFSET, trustOrigin} from 'app/server/lib/requestUtils';
 import {ISendAppPageOptions, makeGristConfig, makeMessagePage, makeSendAppPage} from 'app/server/lib/sendAppPage';
 import {getDatabaseUrl, listenPromise} from 'app/server/lib/serverUtils';
@@ -1361,14 +1361,6 @@ export class FlexServer implements GristServer {
   private _addSupportPaths(docAccessMiddleware: express.RequestHandler[]) {
     if (!this._docWorker) { throw new Error("need DocWorker"); }
 
-    this.app.get('/download', ...docAccessMiddleware, expressWrap(async (req, res) => {
-      // Forward this endpoint to regular API.  This endpoint is now deprecated.
-      const docId = String(req.query.doc);
-      let url = await this.getHomeUrlByDocId(docId, addOrgToPath(req, `/api/docs/${docId}/download`));
-      if (req.query.template === '1') { url += '?template=1'; }
-      return res.redirect(url);
-    }));
-
     const basicMiddleware = [this._userIdMiddleware, this.tagChecker.requireTag];
 
     // Add the handling for the /upload route. Most uploads are meant for a DocWorker: they are put

app/server/lib/uploads.ts

@@ -417,7 +417,7 @@ async function fetchDoc(server: GristServer, docId: string, req: Request, access
   await _checkForError(response);
   const docWorkerUrl = getDocWorkerUrl(server.getOwnUrl(), await response.json());
   // Download the document, in full or as a template.
-  const url = new URL(`download?doc=${docId}&template=${Number(template)}`,
+  const url = new URL(`api/docs/${docId}/download?template=${Number(template)}`,
                       docWorkerUrl.replace(/\/*$/, '/'));
   return _fetchURL(url.href, accessId, {headers});
 }

test/nbrowser/DuplicateDocument.ts

@@ -1,5 +1,5 @@
 import * as gu from 'test/nbrowser/gristUtils';
-import { setupTestSuite } from 'test/nbrowser/testUtils';
+import { server, setupTestSuite } from 'test/nbrowser/testUtils';
 
 import { assert, driver, Key } from 'mocha-webdriver';
 
@@ -71,6 +71,9 @@ describe("DuplicateDocument", function() {
   });
 
   it("should offer a choice of orgs when user is owner", async function() {
+    if (server.isExternalServer()) {
+      this.skip();
+    }
     await driver.find('.test-tb-share').click();
     await driver.find('.test-save-copy').click();
     await driver.findWait('.test-modal-dialog', 1000);
@@ -123,6 +126,9 @@ describe("DuplicateDocument", function() {
   });
 
   it("should offer a choice of orgs when doc is public", async function() {
+    if (server.isExternalServer()) {
+      this.skip();
+    }
     const session = await gu.session().teamSite.login();
     const api = session.createHomeApi();
     // But if the doc is public, then users can copy it out.
@@ -173,6 +179,9 @@ describe("DuplicateDocument", function() {
   });
 
   it("should allow saving a public doc to the personal org", async function() {
+    if (server.isExternalServer()) {
+      this.skip();
+    }
     const session2 = gu.session().teamSite.user('user2');
     await session2.login();
     await session2.loadDoc(`/doc/${urlId}`);

test/server/lib/DocApi.ts

@@ -1767,6 +1767,13 @@ function testDocApi() {
     assert.notMatch(resp.data, /grist_Tables_column/);
   });
 
+  // A tiny test that /copy doesn't throw.
+  it("POST /docs/{did}/copy succeeds", async function() {
+    const docId = docIds.TestDoc;
+    const worker1 = await userApi.getWorkerAPI(docId);
+    await worker1.copyDoc(docId, undefined, 'copy');
+  });
+
   it("GET /docs/{did}/download/csv serves CSV-encoded document", async function() {
     const resp = await axios.get(`${serverUrl}/api/docs/${docIds.Timesheets}/download/csv?tableId=Table1`, chimpy);
     assert.equal(resp.status, 200);