Merge pull request #178 from opendatakit/cxlt/misc-fixes

miscellaneous fixes

lib/http/endpoint.js

@@ -39,12 +39,22 @@ const ensurePromise = (x) => (((x != null) && (typeof x.then === 'function')) ?
 // allows Streams, response-mutating functions, and Problems to be directly returned
 // by a resource handler, and does sane thinsg with each.
 const finalize = (result, request, response) => {
-  // first disallow empty returns with an internal developer error.
-  if (result == null) throw Problem.internal.emptyResponse();
-
+  // first run a function if we got one; its result is what we want.
   if (typeof result === 'function')
-    return resolve(result(request, response)).then(finalize);
+    return resolve(result(request, response))
+      .then((x) => finalize(x, request, response));
+
+  if (result == null) {
+    // then bail out if there is no result but a response code has already been
+    // set, so that 204 and 301/302 (which also have no content) passthrough.
+    if (response.statusCode !== 200)
+      return '';
+
+    // (otherwise a null result is not allowed; return an internal developer error.)
+    throw Problem.internal.emptyResponse();
+  }
 
+  // make sure Problems are thrown so they are caught by the reporter.
   if (result.isProblem === true)
     throw result;
 
@@ -159,10 +169,8 @@ const defaultResultWriter = (result, request, response) => {
         result.end();
     });
     result.pipe(response);
-  } else if (response.statusCode === 204) {
-    response.send();
   } else {
-    response.status(200).send(serialize(result));
+    response.send(serialize(result));
   }
 };
 

lib/http/service.js

@@ -60,6 +60,8 @@ module.exports = (container) => {
   require('../resources/config')(service, endpoint);
   require('../resources/projects')(service, endpoint);
 
+  // TO BE REMOVED IN 0.5.0:
+  require('../resources/compat')(service, endpoint);
 
   ////////////////////////////////////////////////////////////////////////////////
   // POSTRESOURCE HANDLERS

lib/model/query/projects.js

@@ -41,10 +41,12 @@ module.exports = {
       .leftOuterJoin(
         db.select(db.raw('"projectId", count(forms.id)::integer as "forms", max("lastSubByForm") as "lastSubmission"'))
           .from('forms')
+          .where({ deletedAt: null })
           .groupBy('projectId')
           .leftOuterJoin(
             db.select(db.raw('"formId", max("createdAt") as "lastSubByForm"'))
               .from('submissions')
+              .where({ deletedAt: null })
               .groupBy('formId')
               .as('submission_stats'),
             'forms.id', 'submission_stats.formId'

lib/resources/compat.js

@@ -0,0 +1,75 @@
+// Copyright 2019 ODK Central Developers
+// See the NOTICE file at the top-level directory of this distribution and at
+// https://github.com/opendatakit/central-backend/blob/master/NOTICE.
+// This file is part of ODK Central. It is subject to the license terms in
+// the LICENSE file found in the top-level directory of this distribution and at
+// https://www.apache.org/licenses/LICENSE-2.0. No part of ODK Central,
+// including this file, may be copied, modified, propagated, or distributed
+// except according to the terms contained in the LICENSE file.
+
+const { redirect } = require('../util/http');
+const { createReadStream } = require('fs');
+const { always } = require('ramda');
+const { createdMessage } = require('../outbound/openrosa');
+const { getOrNotFound, getOrReject, rejectIf, reject, resolve, ignoringResult } = require('../util/promise');
+const Option = require('../util/option');
+const Problem = require('../util/problem');
+
+// COPYPASTED multipart things:
+const multer = require('multer');
+const tmpdir = require('tmp').dirSync();
+const multipart = multer({ dest: tmpdir.name });
+
+const legacyName = 'Forms you made before projects existed';
+
+// we are guaranteed to be authenticating with a session below, since these
+// endpoints are only ever hit by app users. everybody else: get outta here!
+
+module.exports = (service, endpoint) => {
+  service.get('/formList', endpoint(({ Project }, { auth }) =>
+    Project.getAll().then((projects) => {
+      const legacyProject = projects.find((project) => project.name === legacyName);
+      return (legacyProject == null)
+        ? Problem.user.notFound()
+        : redirect(302, `/v1/key/${auth.session().get().token}/projects/${legacyProject.id}/formList`);
+    })));
+
+  // TEMP TEMP TEMP: COPYPASTA FROM submissions.js => POST /projcets/:id/submission
+  // ANY CHANGES THERE SHOULD GO HERE
+  service.post('/submission', multipart.any(), endpoint.openRosa(({ Audit, Project, Submission }, { files, auth, query }) =>
+    Project.getAll().then((projects) => {
+      const project = projects.find((p) => p.name === legacyName);
+      if (project == null) return Problem.user.notFound();
+
+      return auth.canOrReject('submission.create', project)
+        // then locate the actual xml and parse it into a partial submission.
+        .then(() => Option.of(files).map((xs) => xs.find((file) => file.fieldname === 'xml_submission_file')))
+        .then(getOrReject(Problem.user.missingMultipartField({ field: 'xml_submission_file' })))
+        .then((file) => Submission.fromXml(createReadStream(file.path)))
+        // now that we know the target form, fetch it and make sure it's accepting submissions.
+        .then((partial) => project.getFormByXmlFormId(partial.xmlFormId)
+          .then(getOrNotFound) // TODO: detail why
+          .then(rejectIf(
+            (form) => !form.acceptsSubmissions(),
+            always(Problem.user.notAcceptingSubmissions())
+          ))
+          .then((form) => Submission.getById(form.id, partial.instanceId)
+            // we branch based on whether a submission already existed; in either case, we exit this
+            // branching promise path with a Promise[Submission] that is complete (eg with an id).
+            .then((maybeExtant) => maybeExtant
+              // if a submission already exists, first verify that the posted xml still matches
+              // (if it does not, reject). then, attach any new posted files.
+              .map((extant) => ((Buffer.compare(Buffer.from(extant.xml), Buffer.from(partial.xml)) !== 0)
+                ? reject(Problem.user.xmlConflict())
+                : resolve(extant).then(ignoringResult((submission) => submission.addAttachments(files)))))
+              // otherwise, this is the first POST for this submission. create the
+              // submission and the expected attachments:
+              .orElseGet(() => partial.complete(form, auth.actor(), query.deviceID).create()
+                .then(ignoringResult((submission) => submission.createExpectedAttachments(form, files)))))
+            // now we have a definite submission; we just need to do audit logging.
+            .then((submission) => Audit.log(auth.actor(), 'submission.create', form, { submissionId: submission.id }))
+            // TODO: perhaps actually decide between "full" and "partial"; aggregate does this.
+            .then(always(createdMessage({ message: 'full submission upload was successful!' })))));
+    })));
+};
+

lib/resources/submissions.js

@@ -43,12 +43,10 @@ module.exports = (service, endpoint) => {
   service.get('/projects/:projectId/submission', endpoint(({ Project }, { params }, request, response) =>
     Project.getById(params.projectId)
       .then(getOrNotFound)
-      .then(() => {
-        response.status(204);
-        return false; // TODO: we need to return something non-null but this is a lousy choice.
-      })));
+      .then(() => { response.status(204); })));
 
   // Nonstandard REST; OpenRosa-specific API.
+  // TEMP TEMP TEMP ANY CHANGES HERE SHOULD BE DUPLICATED TO COMPAT.JS REMOVE COMMENT AFTER 0.4 RELEASE
   service.post('/projects/:projectId/submission', multipart.any(), endpoint.openRosa(({ Audit, Project, Submission }, { params, files, auth, query }) =>
     // first, make sure the project exists, and that we have the right to submit to it.
     Project.getById(params.projectId)

lib/util/http.js

@@ -72,6 +72,11 @@ const withCookie = (key) => (value) => (result) => (_, response) => {
   return result;
 };
 
+const redirect = (code, path) => (_, response) => {
+  response.set('location', path);
+  response.status(code);
+};
+
 
 ////////////////////////////////////////////////////////////////////////////////
 // URL HELPERS
@@ -96,7 +101,7 @@ const urlWithQueryParams = (urlStr, set = {}) => {
 module.exports = {
   isTrue, urlPathname,
   serialize,
-  success, contentType, xml, atom, json, withCookie,
+  success, contentType, xml, atom, json, withCookie, redirect,
   urlWithQueryParams
 };
 

test/integration/api/projects.js

@@ -48,6 +48,20 @@ describe('api: /projects', () => {
             body[1].forms.should.equal(2);
             body[1].lastSubmission.should.be.a.recentIsoDate();
           })))));
+
+    it('should return extended metadata if requested', testService((service) =>
+      service.login('alice', (asAlice) =>
+        asAlice.delete('/v1/projects/1/forms/simple')
+          .expect(200)
+          .then(() => asAlice.get('/v1/projects')
+            .set('X-Extended-Metadata', 'true')
+            .expect(200)
+            .then(({ body }) => {
+              body.length.should.equal(1);
+              body[0].should.be.an.ExtendedProject();
+              body[0].name.should.equal('Default Project');
+              body[0].forms.should.equal(1);
+            })))));
   });
 
   describe('POST', () => {

test/unit/http/endpoint.js

@@ -416,6 +416,13 @@ describe('endpoints', () => {
       should.not.exist(response.body);
     });
 
+    it('should send nothing given a 3xx response', () => {
+      const response = createModernResponse();
+      response.status(302);
+      defaultResultWriter({}, createRequest(), response);
+      should.not.exist(response.body);
+    });
+
     it('should pipe through stream results', (done) => {
       let result, writeResult = (x) => { result = x };
       const requestTest = streamTest.fromChunks();