resources/odata: don't crash with surprising instanceId

.github/workflows/standard-e2e.yml

@@ -0,0 +1,38 @@
+name: Standard E2E Tests
+
+on: push
+
+env:
+  LOG_LEVEL: DEBUG
+
+jobs:
+  standard-e2e:
+    timeout-minutes: 15
+    # TODO should we use the same container as circle & central?
+    runs-on: ubuntu-latest
+    services:
+      # see: https://docs.github.com/en/enterprise-server@3.5/actions/using-containerized-services/creating-postgresql-service-containers
+      postgres:
+        image: postgres:14.10
+        env:
+          POSTGRES_PASSWORD: odktest
+        ports:
+          - 5432:5432
+        # Set health checks to wait until postgres has started
+        options: >-
+          --health-cmd pg_isready
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+    steps:
+    - uses: actions/checkout@v4
+    - name: Use Node.js 20
+      uses: actions/setup-node@v4
+      with:
+        node-version: 20.10.0
+        cache: 'npm'
+    - run: npm ci --legacy-peer-deps
+    - run: node lib/bin/create-docker-databases.js
+    - name: E2E Test
+      timeout-minutes: 10
+      run: ./test/e2e/standard/run-tests.sh

lib/resources/odata.js

@@ -46,13 +46,13 @@ module.exports = (service, endpoint) => {
       return matches[1];
     };
 
-    const singleRecord = endpoint.odata.json(({ Forms, Submissions, env }, { auth, params, query, originalUrl }) =>
-      getForm(Forms, auth, params)
-        .then((form) => Promise.all([
-          Forms.getFields(form.def.id).then(selectFields(query, getTableFromOriginalUrl(originalUrl))),
-          Submissions.getForExport(form.id, getUuid(params.uuid), draft).then(getOrNotFound), // may require s3 blob handling
-        ])
-          .then(([fields, row]) => singleRowToOData(fields, row, env.domain, originalUrl, query))));
+    const singleRecord = endpoint.odata.json(async ({ Forms, Submissions, env }, { auth, params, query, originalUrl }) => {
+      const form = await getForm(Forms, auth, params);
+      const allFields = await Forms.getFields(form.def.id);
+      const selectedFields = await selectFields(query, getTableFromOriginalUrl(originalUrl))(allFields);
+      const row = await Submissions.getForExport(form.id, getUuid(params.uuid), draft).then(getOrNotFound);
+      return singleRowToOData(selectedFields, row, env.domain, originalUrl, query);
+    });
 
     // TODO: because of the way express compiles the *, we have to register this twice.
     service.get(`${base}/Submissions\\((:uuid)\\)`, singleRecord);

test/e2e/standard/run-tests.sh

@@ -0,0 +1,35 @@
+#!/bin/bash -eu
+set -o pipefail
+
+serverUrl="http://localhost:8383"
+userEmail="x@example.com"
+userPassword="secret1234"
+
+log() { echo "[test/e2e/standard/run-tests] $*"; }
+
+make base
+
+if [[ "${CI-}" = '' ]]; then
+  set +e
+fi
+
+log "Attempting to create user..."
+echo "$userPassword" | node ./lib/bin/cli.js user-create  -u "$userEmail" && log "User created."
+log "Attempting to promote user..."
+node ./lib/bin/cli.js user-promote -u "$userEmail" && log "User promoted."
+
+make run &
+
+log 'Waiting for backend to start...'
+timeout 30 bash -c "while ! curl -s -o /dev/null $serverUrl; do sleep 1; done"
+log 'Backend started!'
+
+cd test/e2e/standard
+npx mocha test.js
+
+if ! curl -s -o /dev/null "$serverUrl"; then
+  log 'Backend died.'
+  exit 1
+fi
+
+log "Test completed OK."

test/e2e/standard/submission.xml

@@ -0,0 +1,3 @@
+<data id="{{formId}}" version="{{formVersion}}">
+  <orx:meta><orx:instanceID>{{submissionId}}</orx:instanceID></orx:meta>
+</data>

test/e2e/standard/test-form.xml

@@ -0,0 +1,22 @@
+<?xml version="1.0"?>
+<h:html xmlns="http://www.w3.org/2002/xforms" xmlns:h="http://www.w3.org/1999/xhtml" xmlns:ev="http://www.w3.org/2001/xml-events" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:jr="http://openrosa.org/javarosa" xmlns:orx="http://openrosa.org/xforms" xmlns:odk="http://www.opendatakit.org/xforms">
+  <h:head>
+    <h:title>instanceId Test</h:title>
+    <model odk:xforms-version="1.0.0">
+      <instance>
+        <data id="test-form" version="20240506130956">
+          <meta>
+            <instanceID/>
+          </meta>
+        </data>
+      </instance>
+      <instance id="image">
+        <root>
+        </root>
+      </instance>
+      <bind nodeset="/data/meta/instanceID" type="string" readonly="true()" jr:preload="uid"/>
+    </model>
+  </h:head>
+  <h:body>
+  </h:body>
+</h:html>

test/e2e/standard/test.js

@@ -0,0 +1,82 @@
+// Copyright 2024 ODK Central Developers
+// See the NOTICE file at the top-level directory of this distribution and at
+// https://github.com/getodk/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 assert = require('node:assert');
+const fs = require('node:fs');
+
+const SUITE_NAME = 'test/e2e/standard';
+const { apiClient } = require('../util/api');
+
+const serverUrl = 'http://localhost:8383';
+const userEmail = 'x@example.com';
+const userPassword = 'secret1234';
+
+describe('standard', () => {
+  let api, projectId, xmlFormId, xmlFormVersion; // eslint-disable-line one-var, one-var-declaration-per-line
+
+  it('should handle weird submission instanceId gracefully', async () => {
+    // given
+    api = await apiClient(SUITE_NAME, { serverUrl, userEmail, userPassword });
+    projectId = await createProject();
+    await uploadForm('test-form.xml');
+    // and
+    const goodSubmissionId = 'good-id';
+    await uploadSubmission(goodSubmissionId);
+
+    // expect 200:
+    await api.apiGet(`projects/${projectId}/forms/${encodeURIComponent(xmlFormId)}.svc/Submissions('${goodSubmissionId}')`);
+
+    // given
+    const badSubmissionId = 'bad-id:';
+    await uploadSubmission(badSubmissionId);
+    // when
+    await assert.rejects(
+      () => api.apiGet(`projects/${projectId}/forms/${encodeURIComponent(xmlFormId)}.svc/Submissions('${badSubmissionId}')?%24select=__id%2C__system%2Cmeta`),
+      (err) => {
+        // then
+        assert.strictEqual(err.responseStatus, 404);
+        assert.deepStrictEqual(JSON.parse(err.responseText), {
+          message: 'Could not find the resource you were looking for.',
+          code: 404.1,
+        });
+        return true;
+      },
+    );
+
+    // and service has not crashed:
+    const rootRes = await fetch(serverUrl);
+    assert.strictEqual(rootRes.status, 404);
+    assert.strictEqual(await rootRes.text(), '{"message":"Expected an API version (eg /v1) at the start of the request URL.","code":404.2}');
+  });
+
+  async function createProject() {
+    const project = await api.apiPostJson(
+      'projects',
+      { name:`standard-test-${new Date().toISOString().replace(/\..*/, '')}` },
+    );
+    return project.id;
+  }
+
+  async function uploadForm(xmlFilePath) {
+    const res = await api.apiPostFile(`projects/${projectId}/forms?publish=true`, xmlFilePath);
+    xmlFormId = res.xmlFormId;
+    xmlFormVersion = res.version;
+  }
+
+  function uploadSubmission(submissionId) {
+    const xmlTemplate = fs.readFileSync('submission.xml', { encoding: 'utf8' });
+    const tempFile = 'TODO-generate-proper-tempfile-name.xml';
+    const formXml = xmlTemplate
+      .replace('{{submissionId}}', submissionId)
+      .replace('{{formId}}', xmlFormId)
+      .replace('{{formVersion}}', xmlFormVersion);
+    fs.writeFileSync(tempFile, formXml);
+    return api.apiPostFile(`projects/${projectId}/forms/${encodeURIComponent(xmlFormId)}/submissions?deviceID=testid`, tempFile);
+  }
+});

test/e2e/util/api.js

@@ -91,7 +91,16 @@ async function apiClient(suiteName, { serverUrl, userEmail, userPassword, logPat
 
     // eslint-disable-next-line no-use-before-define
     if(isRedirected(res)) return new Redirect(res);
-    if(!res.ok) throw new Error(`${res.status}: ${await res.text()}`);
+    if(!res.ok) {
+      const responseStatus = res.status;
+      const responseText = await res.text();
+
+      const err = new Error(`${responseStatus}: ${responseText}`);
+      err.responseStatus = responseStatus;
+      err.responseText = responseText;
+
+      throw err;
+    }
     return res;
   }
 }