Merge branch 'master' into 1212-logging

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2.1
 jobs:
   build:
     docker:
-      - image: cimg/node:20.17.0
+      - image: cimg/node:22.12.0
       - image: cimg/postgres:14.10
         environment:
           POSTGRES_PASSWORD: odktest

.github/workflows/oidc-e2e.yml

@@ -1,6 +1,8 @@
 name: OIDC e2e tests
 
-on: push
+on:
+  push:
+  pull_request:
 
 env:
   DEBUG: pw:api
@@ -27,10 +29,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.17.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: make test-oidc-e2e
     - name: Archive playwright screenshots

.github/workflows/oidc-integration.yml

@@ -1,6 +1,8 @@
 name: OIDC integration tests
 
-on: push
+on:
+  push:
+  pull_request:
 
 jobs:
   oidc-integration-test:
@@ -23,10 +25,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.17.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: npm ci
     - run: FAKE_OIDC_ROOT_URL=http://localhost:9898 make fake-oidc-server-ci > fake-oidc-server.log &

.github/workflows/s3-e2e.yml

@@ -1,6 +1,8 @@
 name: S3 E2E Tests
 
-on: push
+on:
+  push:
+  pull_request:
 
 jobs:
   s3-e2e:
@@ -41,10 +43,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.17.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: npm ci
     - run: node lib/bin/create-docker-databases.js

.github/workflows/soak-test.yml

@@ -25,10 +25,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.17.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: npm ci
     - run: node lib/bin/create-docker-databases.js

.github/workflows/standard-e2e.yml

@@ -1,6 +1,8 @@
 name: Standard E2E Tests
 
-on: push
+on:
+  push:
+  pull_request:
 
 env:
   LOG_LEVEL: DEBUG
@@ -26,10 +28,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.10.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: npm ci
     - run: node lib/bin/create-docker-databases.js

.github/workflows/standard-suite.yml

@@ -1,6 +1,8 @@
 name: Full Standard Test Suite
 
-on: push
+on:
+  push:
+  pull_request:
 
 jobs:
   standard-tests:
@@ -23,10 +25,10 @@ jobs:
           --health-retries 5
     steps:
     - uses: actions/checkout@v4
-    - name: Use Node.js 20
+    - name: Set node version
       uses: actions/setup-node@v4
       with:
-        node-version: 20.17.0
+        node-version: 22.12.0
         cache: 'npm'
     - run: npm ci
     - run: node lib/bin/create-docker-databases.js

Makefile

@@ -101,27 +101,27 @@ debug: base
 
 .PHONY: test
 test: lint
-	BCRYPT=insecure npx mocha --recursive
+	BCRYPT=insecure npx --node-options="--no-deprecation" mocha --recursive
 
 .PHONY: test-ci
 test-ci: lint
-	BCRYPT=insecure npx mocha --recursive --reporter test/ci-mocha-reporter.js
+	BCRYPT=insecure npx --node-options="--no-deprecation" mocha --recursive --reporter test/ci-mocha-reporter.js
 
 .PHONY: test-fast
 test-fast: node_version
-	BCRYPT=insecure npx mocha --recursive --fgrep @slow --invert
+	BCRYPT=insecure npx --node-options="--no-deprecation" mocha --recursive --fgrep @slow --invert
 
 .PHONY: test-integration
 test-integration: node_version
-	BCRYPT=insecure npx mocha --recursive test/integration
+	BCRYPT=insecure npx --node-options="--no-deprecation" mocha --recursive test/integration
 
 .PHONY: test-unit
 test-unit: node_version
 	BCRYPT=insecure npx mocha --recursive test/unit
 
 .PHONY: test-coverage
 test-coverage: node_version
-	npx nyc -x "**/migrations/**" --reporter=lcov node_modules/.bin/_mocha --recursive test
+	npx --node-options="--no-deprecation" nyc -x "**/migrations/**" --reporter=lcov node_modules/.bin/_mocha --recursive test
 
 .PHONY: lint
 lint: node_version

docs/api.yaml

@@ -45,6 +45,7 @@ info:
     **Changed**:
 
     - [Submissions Odata]() now returns `__system/deletedAt`. It can also be used in $filter, $sort and $select query parameters.
+    - Dataset (Entity List) properties with the same name but different capitalization are not allowed.
 
     ## ODK Central v2024.2
 
@@ -868,7 +869,9 @@ tags:
     * `submission.delete` when a Submission is soft-deleted.
     * `submission.purge` when soft-deleted Submissions are purged.
     * `submission.restore` when a Submission is restored.
-    * `submission.reprocess` when an Entity Submission is held in a processing backlog and then removed.
+    * `submission.backlog.hold` when an Entity Submission is first held in processing backlog.
+    * `submission.backlog.reprocess` when an Entity Submission is reprocessed and removed from the backlog.
+    * `submission.backlog.force` when an Entity Submission is force-processed after being in backlog.
     * `dataset.create` when a Dataset is created.
     * `dataset.update` when a Dataset is updated.
     * `dataset.update.publish` when a Dataset is published.
@@ -8705,6 +8708,8 @@ paths:
       description: |-
         Creates a new Property with a specified name in the Dataset.
 
+        The name of a Property is case-sensitive in that it will keep the capitalization provided (e.g. "Firstname"). But Central will not allow another Property with the same name but different capitalization to be created (e.g. "FIRSTNAME" when "Firstname" already exists).
+
         Property names follow the same rules as form field names (valid XML identifiers) and cannot use the reserved names of `name` or `label`, or begin with the reserved prefix `__`. 
       operationId: Adding Properties
       parameters:
@@ -9642,7 +9647,7 @@ paths:
 
         * Central supports the `HEAD` request preflighting recommended by the specification, but does not require it. Because our supported authentication methods do not follow the try/retry pattern, only preflight your request if you want to read the `X-OpenRosa-Accept-Content-Length` header or are concerned about the other issues listed in the standards document, like proxies.
 
-        * As stated in the standards document, it is possible to submit multimedia attachments with the `Submission` across multiple `POST` requests to this API. _However_, we impose the additional restriction that the Submission XML (`xml_submission_file`) _may not change_ between requests. If Central sees a Submission with an `instanceId` it already knows about but the XML has changed in any way, it will respond with a `409 Conflict` error and reject the submission.
+        * As stated in the standards document, it is possible to submit multimedia attachments with the `Submission` across multiple `POST` requests to this API. _However_, we impose the additional restriction that the Submission XML (`xml_submission_file`) _may not change_ between requests. If Central sees a Submission with an `instanceId` it already knows about but the XML has changed in any way, it will respond with a `409 Conflict` error and reject the submission. Additionally, a `409 Conflict` error is returned if the Submission has been deleted in Central while attempting to send further POST requests.
 
         * Central will never return a `202` in any response from this API.
 

lib/data/entity.js

@@ -500,8 +500,12 @@ const getWithConflictDetails = (defs, audits, relevantToConflict) => {
     if (v.version > 1) { // v.root is false here - can use either
       const baseNotContiguousWithTrunk = v.branchId != null &&
         branches.get(v.branchId).lastContiguousWithTrunk < v.baseVersion;
+
+      // check if it's a create applied as an update, which is also a conflict
+      const createAsUpdate = def.aux.source?.details?.action === 'create';
+
       const conflict = v.version !== (v.baseVersion + 1) ||
-        baseNotContiguousWithTrunk;
+        baseNotContiguousWithTrunk || createAsUpdate;
 
       v.baseDiff = getDiffProp(v.dataReceived, { ...defs[v.baseVersion - 1].data, label: defs[v.baseVersion - 1].label });
 

lib/formats/odata.js

@@ -513,7 +513,9 @@ const singleRowToOData = (fields, row, domain, originalUrl, query) => {
     let { offset } = paging;
 
     if (skipToken) {
-      offset = filtered.findIndex(s => skipToken.repeatId === s.__id) + 1;
+      const { repeatId } = skipToken;
+      if (!repeatId) throw Problem.user.odataInvalidSkiptoken();
+      offset = filtered.findIndex(s => repeatId === s.__id) + 1;
       if (offset === 0) throw Problem.user.odataRepeatIdNotFound();
     }
 

lib/http/service.js

@@ -122,14 +122,18 @@ module.exports = (container) => {
   service.use((error, request, response, next) => {
     if (response.headersSent === true) {
       // In this case, we'll just let Express fail the request out.
-      next(error);
-    } else if (error?.type === 'entity.parse.failed') {
-      // catch body-parser middleware problems. we only ask it to parse JSON, which
-      // isn't part of OpenRosa, so we can assume a plain JSON response.
-      defaultErrorWriter(Problem.user.unparseable({ format: 'json', rawLength: error.body.length }), request, response);
-    } else {
-      defaultErrorWriter(error, request, response);
+      return next(error);
+    }
+
+    // catch body-parser middleware problems. we only ask it to parse JSON, which
+    // isn't part of OpenRosa, so we can assume a plain JSON response.
+    switch (error?.type) {
+      case 'encoding.unsupported': return defaultErrorWriter(Problem.user.encodingNotSupported(), request, response);
+      case 'entity.parse.failed': return defaultErrorWriter(Problem.user.unparseable({ format: 'json', rawLength: error.body.length }), request, response);
+      case 'entity.too.large': return defaultErrorWriter(Problem.user.requestTooLarge(), request, response);
+      default: return defaultErrorWriter(error, request, response);
     }
+
   });
 
   return service;

lib/model/query/analytics.js

@@ -525,7 +525,7 @@ FROM duplicateRuns;
 const countSubmissionReprocess = () => ({ oneFirst }) => oneFirst(sql`
   SELECT COUNT(*)
   FROM audits
-  WHERE "action" = 'submission.reprocess'
+  WHERE "action" = 'submission.backlog.reprocess'
 `);
 
 // Measure how much time entities whose source is a submission.create

lib/model/query/audits.js

@@ -38,7 +38,7 @@ const actionCondition = (action) => {
     // The backup action was logged by a backup script that has been removed.
     // Even though the script has been removed, the audit log entries it logged
     // have not, so we should continue to exclude those.
-    return sql`action not in ('entity.create', 'entity.bulk.create', 'entity.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'submission.reprocess', 'submission.delete', 'submission.restore', 'backup', 'analytics')`;
+    return sql`action not in ('entity.create', 'entity.bulk.create', 'entity.error', 'entity.update.version', 'entity.update.resolve', 'entity.delete', 'submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'submission.backlog.hold', 'submission.backlog.reprocess', 'submission.backlog.force', 'submission.delete', 'submission.restore', 'backup', 'analytics')`;
   else if (action === 'user')
     return sql`action in ('user.create', 'user.update', 'user.delete', 'user.assignment.create', 'user.assignment.delete', 'user.session.create')`;
   else if (action === 'field_key')
@@ -50,7 +50,7 @@ const actionCondition = (action) => {
   else if (action === 'form')
     return sql`action in ('form.create', 'form.update', 'form.delete', 'form.restore', 'form.purge', 'form.attachment.update', 'form.submission.export', 'form.update.draft.set', 'form.update.draft.delete', 'form.update.draft.replace', 'form.update.publish', 'upgrade.process.form.entities_version')`;
   else if (action === 'submission')
-    return sql`action in ('submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'submission.reprocess', 'submission.delete', 'submission.restore', 'submission.purge')`;
+    return sql`action in ('submission.create', 'submission.update', 'submission.update.version', 'submission.attachment.update', 'submission.backlog.hold', 'submission.backlog.reprocess', 'submission.backlog.force', 'submission.delete', 'submission.restore', 'submission.purge')`;
   else if (action === 'dataset')
     return sql`action in ('dataset.create', 'dataset.update')`;
   else if (action === 'entity')
@@ -114,8 +114,6 @@ ${extend|| sql`
   LEFT JOIN entity_defs AS current_entity_def ON current_entity_def."entityId" = entities.id AND current
 `}
   WHERE (audits.details->>'submissionId')::INTEGER = ${submissionId}
-    -- suppress this one event that is used for offline entity ordering/processing
-    AND audits.action != 'submission.reprocess'
   ORDER BY audits."loggedAt" DESC, audits.id DESC
   ${page(options)}`);