From 97e42ddfd0cb2f1ac7dda2380696e9198b2303b1 Mon Sep 17 00:00:00 2001 From: Alex Kulikov Date: Tue, 18 Aug 2026 00:57:40 +0100 Subject: [PATCH 1/4] feat(offline-demo): add exporter and drop Microsoft dependency OfflineDemoTransferExtension.getExporter() returned a literal null, so the one extension in the repo that bypasses OAuth entirely was import-only and could not drive a transfer. Implement it, so offline-demo -> offline-demo becomes a complete, credential-free path. Replace MicrosoftOfflineData with a local DemoOfflineData. That type is a 20-line String wrapper, and depending on it pulled the entire Microsoft adapter -- okhttp plus every Graph exporter and importer -- into a demo module. Exporter bounds T on DataModel rather than ContainerResource, so a plain value class is type-legal here. Also gate both getExporter and getImporter on OFFLINE_DATA. getImporter previously returned the importer for any vertical; unreachable today, since the auth extension advertises only OFFLINE_DATA, but TransferCompatibilityProvider probes extensions with MEDIA, PHOTOS and VIDEOS and would take the answer. Adds the module's first tests. BREAKING CHANGE: OfflineDemoImporter's type parameter changes from MicrosoftOfflineData to DemoOfflineData. This module is published to Maven Central, so it is a binary-compatibility break for any external consumer -- unlikely for a demo importer that prints to stdout, but worth a release note. --- .../build.gradle | 3 - .../transfer/offline/DemoOfflineData.java | 40 +++++++++ .../transfer/offline/OfflineDemoExporter.java | 43 ++++++++++ .../transfer/offline/OfflineDemoImporter.java | 7 +- .../offline/OfflineDemoTransferExtension.java | 11 ++- .../offline/OfflineDemoTransferTest.java | 86 +++++++++++++++++++ 6 files changed, 178 insertions(+), 12 deletions(-) create mode 100644 extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/DemoOfflineData.java create mode 100644 extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoExporter.java create mode 100644 extensions/data-transfer/portability-data-transfer-offline-demo/src/test/java/org/datatransferproject/transfer/offline/OfflineDemoTransferTest.java diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/build.gradle b/extensions/data-transfer/portability-data-transfer-offline-demo/build.gradle index 6e6611f46..b54dd0202 100644 --- a/extensions/data-transfer/portability-data-transfer-offline-demo/build.gradle +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/build.gradle @@ -21,9 +21,6 @@ plugins { dependencies { compile project(':portability-spi-cloud') compile project(':portability-spi-transfer') - compile project(':extensions:data-transfer:portability-data-transfer-microsoft') - - } configurePublication(project) \ No newline at end of file diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/DemoOfflineData.java b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/DemoOfflineData.java new file mode 100644 index 000000000..314a070b1 --- /dev/null +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/DemoOfflineData.java @@ -0,0 +1,40 @@ +/* + * Copyright 2026 The Data Transfer Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.datatransferproject.transfer.offline; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonTypeName; +import org.datatransferproject.types.common.models.DataModel; + +/** + * Encapsulates offline data for the demo extension. Note the format of the contents is opaque; they + * may change without notice. + */ +@JsonTypeName("org.dataportability:DemoOfflineData") +public class DemoOfflineData extends DataModel { + + private final String contents; + + @JsonCreator + public DemoOfflineData(@JsonProperty("contents") String contents) { + this.contents = contents; + } + + public String getContents() { + return contents; + } +} diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoExporter.java b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoExporter.java new file mode 100644 index 000000000..abe6ec193 --- /dev/null +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoExporter.java @@ -0,0 +1,43 @@ +/* + * Copyright 2026 The Data Transfer Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.datatransferproject.transfer.offline; + +import java.util.Optional; +import java.util.UUID; +import org.datatransferproject.spi.transfer.provider.ExportResult; +import org.datatransferproject.spi.transfer.provider.Exporter; +import org.datatransferproject.types.common.ExportInformation; +import org.datatransferproject.types.transfer.auth.TokenAuthData; + +/** + * Simulates exporting offline data. For demo purposes only! + * + *

Returns a fixed payload without contacting any service, so a transfer can be run end to end + * without provider credentials. The contents are deterministic so callers may assert on them. + */ +public class OfflineDemoExporter implements Exporter { + + /** The payload every export returns. */ + static final String CONTENTS = "offline-demo data"; + + @Override + public ExportResult export( + UUID jobId, TokenAuthData authData, Optional exportInformation) { + // Continuation data is left null: that, rather than the ResultType, is what stops + // PortabilityInMemoryDataCopier#copyHelper from recursing. + return new ExportResult<>(ExportResult.ResultType.END, new DemoOfflineData(CONTENTS)); + } +} diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoImporter.java b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoImporter.java index 64ead77a4..90aef3d8f 100644 --- a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoImporter.java +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoImporter.java @@ -18,23 +18,20 @@ import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor; import org.datatransferproject.spi.transfer.provider.ImportResult; import org.datatransferproject.spi.transfer.provider.Importer; -import org.datatransferproject.transfer.microsoft.spi.types.MicrosoftOfflineData; import org.datatransferproject.types.transfer.auth.TokenAuthData; import java.util.UUID; /** * Simulates importing offline data. For demo purposes only! - * - *

Microsoft offline data is used since that is the only form currently supported. */ -public class OfflineDemoImporter implements Importer { +public class OfflineDemoImporter implements Importer { @Override public ImportResult importItem(UUID jobId, IdempotentImportExecutor idempotentExecutor, TokenAuthData authData, - MicrosoftOfflineData data) { + DemoOfflineData data) { // Print to the console to simulate an import System.out.println("Received offline data:\n" + data.getContents()); return ImportResult.OK; diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoTransferExtension.java b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoTransferExtension.java index b2c2d3596..3a4ec489d 100644 --- a/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoTransferExtension.java +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/src/main/java/org/datatransferproject/transfer/offline/OfflineDemoTransferExtension.java @@ -1,5 +1,7 @@ package org.datatransferproject.transfer.offline; +import static org.datatransferproject.types.common.models.DataVertical.OFFLINE_DATA; + import org.datatransferproject.api.launcher.ExtensionContext; import org.datatransferproject.types.common.models.DataVertical; import org.datatransferproject.spi.transfer.extension.TransferExtension; @@ -7,9 +9,10 @@ import org.datatransferproject.spi.transfer.provider.Importer; /** - * Simulates importing offline data. For demo purposes only! + * Simulates transferring offline data. For demo purposes only! * - *

Microsoft offline data is used since that is the only form currently supported. + *

Both sides are credential-free, so this is the one extension pair that can run a complete + * transfer without provider API keys. */ public class OfflineDemoTransferExtension implements TransferExtension { private static final String SERVICE_ID = "offline-demo"; @@ -21,12 +24,12 @@ public String getServiceId() { @Override public Exporter getExporter(DataVertical transferDataType) { - return null; + return OFFLINE_DATA.equals(transferDataType) ? new OfflineDemoExporter() : null; } @Override public Importer getImporter(DataVertical transferDataType) { - return new OfflineDemoImporter(); + return OFFLINE_DATA.equals(transferDataType) ? new OfflineDemoImporter() : null; } @Override diff --git a/extensions/data-transfer/portability-data-transfer-offline-demo/src/test/java/org/datatransferproject/transfer/offline/OfflineDemoTransferTest.java b/extensions/data-transfer/portability-data-transfer-offline-demo/src/test/java/org/datatransferproject/transfer/offline/OfflineDemoTransferTest.java new file mode 100644 index 000000000..1abfd9cd3 --- /dev/null +++ b/extensions/data-transfer/portability-data-transfer-offline-demo/src/test/java/org/datatransferproject/transfer/offline/OfflineDemoTransferTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2026 The Data Transfer Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.datatransferproject.transfer.offline; + +import static com.google.common.truth.Truth.assertThat; +import static org.datatransferproject.types.common.models.DataVertical.OFFLINE_DATA; +import static org.datatransferproject.types.common.models.DataVertical.PHOTOS; +import static org.mockito.Mockito.mock; + +import java.util.Optional; +import java.util.UUID; +import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor; +import org.datatransferproject.spi.transfer.provider.ExportResult; +import org.datatransferproject.spi.transfer.provider.ImportResult; +import org.datatransferproject.types.transfer.auth.TokenAuthData; +import org.junit.jupiter.api.Test; + +public class OfflineDemoTransferTest { + + private static final UUID JOB_ID = UUID.randomUUID(); + private static final TokenAuthData AUTH_DATA = new TokenAuthData("123"); + + @Test + public void exportsFixedContents() { + ExportResult result = + new OfflineDemoExporter().export(JOB_ID, AUTH_DATA, Optional.empty()); + + assertThat(result.getType()).isEqualTo(ExportResult.ResultType.END); + assertThat(result.getExportedData().getContents()).isEqualTo(OfflineDemoExporter.CONTENTS); + } + + @Test + public void exportReturnsNoContinuationData() { + // PortabilityInMemoryDataCopier#copyHelper recurses on continuation data, not on ResultType, + // so null here is what actually ends the copy. + ExportResult result = + new OfflineDemoExporter().export(JOB_ID, AUTH_DATA, Optional.empty()); + + assertThat(result.getContinuationData()).isNull(); + } + + @Test + public void extensionSuppliesBothSidesOfOfflineData() { + OfflineDemoTransferExtension extension = new OfflineDemoTransferExtension(); + + assertThat(extension.getExporter(OFFLINE_DATA)).isInstanceOf(OfflineDemoExporter.class); + assertThat(extension.getImporter(OFFLINE_DATA)).isInstanceOf(OfflineDemoImporter.class); + } + + @Test + public void extensionSuppliesNothingForOtherVerticals() { + OfflineDemoTransferExtension extension = new OfflineDemoTransferExtension(); + + assertThat(extension.getExporter(PHOTOS)).isNull(); + assertThat(extension.getImporter(PHOTOS)).isNull(); + } + + @Test + public void importsExportedData() { + ExportResult exported = + new OfflineDemoExporter().export(JOB_ID, AUTH_DATA, Optional.empty()); + + ImportResult result = + new OfflineDemoImporter() + .importItem( + JOB_ID, + mock(IdempotentImportExecutor.class), + AUTH_DATA, + exported.getExportedData()); + + assertThat(result.getType()).isEqualTo(ImportResult.ResultType.OK); + } +} From 25ebd500b46c1a6281db7ee5acc26a9a41cb5df5 Mon Sep 17 00:00:00 2001 From: Alex Kulikov Date: Tue, 1 Sep 2026 01:29:05 +0100 Subject: [PATCH 2/4] test(e2e): add black-box offline-demo transfer harness Runs a complete offline-demo -> offline-demo transfer against the packaged demo-server jar and asserts that the exported payload actually arrives. One command, no provider credentials, no local JDK and no local Python: ./e2e/run.sh Three services in the root compose file, two of which share the repo's single Dockerfile. No new Dockerfile, no build.gradle, no settings.gradle entry, no Java. Also renames the existing `test` service to `gradle`. It was confusingly named: it runs the Gradle build, while the service that actually runs tests is a different one. Developer.md had the tell -- `docker compose run --rm test test --tests SomeTest`, service name and Gradle task colliding on one word. `gradle` is exact, since the service's ENTRYPOINT is ./gradlew. The name `test` is deliberately left unused rather than reassigned to the pytest service: it is documented and hardcoded in docker-build.yml, so a `test` that survived but meant something else would silently run the e2e suite for anyone with the old command in their fingers, and pass. It now fails with "no such service". Developer.md and docker-build.yml are updated to match, and a stale claim that the root Dockerfile pins gradle:8.10.2-jdk11 is corrected -- it pins eclipse-temurin:11-jdk-jammy. The jar is built through the `gradle` service, whose ENTRYPOINT is already ./gradlew, so arguments pass straight through. That keeps the pinned Gradle 6.9.2 / JDK 11 toolchain single-sourced and reuses the warm gradle-cache volume, which a `docker build` stage could not mount. shadowJar has no path to copyWebApp, so no Node or Angular toolchain is involved. `dtp` overrides the image entrypoint to run `java -jar` on that shadowJar, so the harness exercises the real packaged artifact including its mergeServiceFiles() META-INF/services merge. It deliberately defines no image of its own: the repo publishes no server image today, so the harness pins a contract -- an API on 8080 plus a readable log stream -- that a future image can satisfy by replacing three keys on one service. Completion is read from the server log because there is no alternative. TransferJob.state is a hardcoded CREATED with no getter, so Jackson never emits it and GET /api/transfer/{id} reports no progress at all. Note that "Finished processing ... with 0 error(s)" is necessary but not sufficient: the count is copier.getErrors().size(), so a job that never copied anything reports zero too. Failure is therefore detected from SEVERE lines naming the job -- a clean run emits none -- and the delivered payload is asserted separately from job completion. Both assertions were verified by deliberately breaking them. CI runs the harness non-blocking to begin with, uploading the server log as an artifact whether the run passes or fails. --- .github/workflows/docker-build.yml | 2 +- .github/workflows/e2e.yml | 37 ++++ Documentation/Developer.md | 36 +++- docker-compose.yml | 78 ++++++++- e2e/README.md | 142 +++++++++++++++ e2e/driver/conftest.py | 30 ++++ e2e/driver/dtp.py | 267 +++++++++++++++++++++++++++++ e2e/driver/pytest.ini | 6 + e2e/driver/requirements.txt | 2 + e2e/driver/test_offline_demo.py | 102 +++++++++++ e2e/run.sh | 50 ++++++ 11 files changed, 746 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/e2e.yml create mode 100644 e2e/README.md create mode 100644 e2e/driver/conftest.py create mode 100644 e2e/driver/dtp.py create mode 100644 e2e/driver/pytest.ini create mode 100644 e2e/driver/requirements.txt create mode 100644 e2e/driver/test_offline_demo.py create mode 100755 e2e/run.sh diff --git a/.github/workflows/docker-build.yml b/.github/workflows/docker-build.yml index 97dd8f7aa..05aa083eb 100644 --- a/.github/workflows/docker-build.yml +++ b/.github/workflows/docker-build.yml @@ -19,4 +19,4 @@ jobs: steps: - uses: actions/checkout@v4 - name: Build and test in Docker - run: docker compose run --rm test + run: docker compose run --rm gradle diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml new file mode 100644 index 000000000..a8277688e --- /dev/null +++ b/.github/workflows/e2e.yml @@ -0,0 +1,37 @@ +# Runs a complete offline-demo -> offline-demo transfer against the packaged +# demo-server jar. See e2e/README.md. +# +# continue-on-error is deliberate and temporary: a flaky end-to-end job that +# blocks merges is worse than no end-to-end job. Drop it once this has run +# green on master for a while. + +name: End-to-end transfer + +on: + push: + branches: [ master ] + pull_request: + branches: [ master ] + +jobs: + transfer: + + runs-on: ubuntu-latest + continue-on-error: true + + steps: + - uses: actions/checkout@v4 + + - name: Run the end-to-end transfer + run: ./e2e/run.sh + + # The server log is the only record of what the transfer actually did, so + # keep it whether the run passed or failed -- a failure should be + # diagnosable without re-running it. + - name: Upload server log + if: always() + uses: actions/upload-artifact@v4 + with: + name: e2e-server-log + path: e2e/.logs/ + if-no-files-found: warn diff --git a/Documentation/Developer.md b/Documentation/Developer.md index cf42a115d..1d1409505 100644 --- a/Documentation/Developer.md +++ b/Documentation/Developer.md @@ -86,17 +86,20 @@ See [Running Locally](RunningLocally.md) for instructions. This is unrelated to the demo image built by `dockerize` above -- it's a separate, additive way to run the Java test suite without installing a JDK locally. The `Dockerfile` at the repo root pins -`gradle:8.10.2-jdk11`; the source is bind-mounted at run time rather than baked into the image, so a -rebuild isn't needed after every code change. +`eclipse-temurin:11-jdk-jammy` and resolves the wrapper-pinned Gradle 6.9.2 into an image layer at +build time; the source is bind-mounted at run time rather than baked into the image, so a rebuild +isn't needed after every code change. + +The `gradle` service's `ENTRYPOINT` is `./gradlew`, so anything you pass it is a Gradle argument. Run the full check task against the current working tree: ```bash -docker compose run --rm test +docker compose run --rm gradle ``` To run a specific Gradle task or test, override the default command: ```bash -docker compose run --rm test test --tests SomeTest +docker compose run --rm gradle test --tests SomeTest ``` The `gradle-cache` named volume (defined in `docker-compose.yml`) persists resolved dependencies across @@ -105,6 +108,31 @@ runs, so only the first run pays the full resolution cost. The JDK is pinned to 11 because the Gradle wrapper (6.9.2) can't parse Java 17 bytecode when compiling `build.gradle`/`settings.gradle` -- a `gradle:*-jdk17` image fails outright for this reason. +## Running the end-to-end transfer test + +The `gradle` service above runs unit tests. To run a real transfer end to end -- job creation, auth, +worker claim, export, import -- against the packaged `demo-server` jar: + +```bash +./e2e/run.sh +``` + +No provider credentials, no local JDK and no local Python; about 35 seconds with a warm Gradle cache. +It uses the `dtp` and `e2e` services in `docker-compose.yml`, and leaves the server log in +`e2e/.logs/dtp.log` whether the run passes or fails. + +The same services are the shortest way to watch DTP actually do something without acquiring a single +API key: + +```bash +docker compose run --rm gradle --no-daemon \ + :distributions:demo-server:shadowJar -PofflineData=true -PencryptionScheme=cleartext +docker compose up dtp # API on https://localhost:8080 (self-signed cert) +``` + +See [e2e/README.md](../e2e/README.md) for what a green run does and does not prove, and for how to +add an adapter. + ## Deploying in production A demo distribution for Google Cloud Platform is available at diff --git a/docker-compose.yml b/docker-compose.yml index 8eccd7eb5..8fc15c316 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,9 +1,85 @@ +# Three services; `gradle` and `dtp` share this repo's one Dockerfile. +# +# `gradle` runs the Gradle build (see Dockerfile) -- its ENTRYPOINT is +# ./gradlew, so arguments passed to it are Gradle arguments. `dtp` runs the +# packaged demo-server jar that build produces, and `e2e` drives a transfer +# against it. +# +# `dtp` deliberately reuses `build: .` rather than defining an image of its +# own -- the repo publishes no server image today, so the harness pins a +# *contract* (an API on 8080 plus a log stream) rather than an artifact. When a +# real server image exists, replace `build`/`entrypoint`/`command` on `dtp` +# with `image:` and nothing else here changes. +# +# docker compose run --rm gradle # the build +# docker compose up dtp # the server, on https://localhost:8080 +# ./e2e/run.sh # the end-to-end transfer services: - test: + gradle: build: . volumes: - .:/workspace - gradle-cache:/home/gradle/.gradle + # The system under test: API + transfer worker in one JVM, as SingleVMMain + # requires (LocalJobStore shares state through private static maps, so the + # two only ever see the same job when co-located). + dtp: + build: . + volumes: + - .:/workspace + - dtp-logs:/var/log/dtp + environment: + # LocalAppCredentialStore reads secrets from the environment and ApiMain + # builds a JWTTokenManager unconditionally, so it will not boot without + # these. The values are never checked against anything -- offline-demo + # bypasses OAuth entirely. + JWT_KEY: e2e-key + JWT_SECRET: e2e-secret + # Published so `docker compose up dtp` is also the credential-free way to + # poke at a running DTP by hand. JettyTransport hardcodes 8080. + ports: + - "8080:8080" + # The image's ENTRYPOINT is ./gradlew; override it to run the jar that + # `gradle` built. Not `:distributions:demo-server:run` -- that task reads + # build/resources/main but nothing makes it depend on createApiFile, so + # api.yaml is missing and ApiExtensionContext fails on a null baseUrl. + # shadowJar.dependsOn(createApiFile), so the jar is self-contained. + # + # Globbed because the jar name depends on the version, and root + # build.gradle sets `version = System.getenv('RELEASE_VERSION')` -- unset + # locally, so the artifact is plain `demo-server-all.jar`, but versioned in + # a release build. A glob needs a shell, hence bash -c. + # + # tee, not `>`: the e2e service reads the file, humans read `docker compose + # logs dtp`, and run.sh captures the same stream to e2e/.logs/ afterwards. + # Merging stderr matters -- ConsoleMonitor writes the server log to stderr + # while OfflineDemoImporter prints the delivered payload to stdout. + entrypoint: ["bash", "-c"] + command: + - "java -jar distributions/demo-server/build/libs/*-all.jar 2>&1 | tee /var/log/dtp/dtp.log" + + # Stock python image, no Dockerfile. Deps install into a cached volume. + e2e: + image: python:3.12-slim + working_dir: /workspace + depends_on: + - dtp + volumes: + - .:/workspace + - dtp-logs:/var/log/dtp:ro + - pip-cache:/root/.cache/pip + environment: + DTP_BASE_URL: https://dtp:8080 + DTP_LOG: /var/log/dtp/dtp.log + # Keep the container from littering the bind-mounted repo with + # root-owned __pycache__ directories. + PYTHONDONTWRITEBYTECODE: "1" + entrypoint: ["bash", "-c"] + command: + - "pip install --quiet --root-user-action=ignore -r e2e/driver/requirements.txt && pytest e2e/driver -v -p no:cacheprovider" + volumes: gradle-cache: + dtp-logs: + pip-cache: diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 000000000..4b366965c --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,142 @@ +# End-to-end transfer harness + +Runs a complete DTP transfer — job creation, auth, worker claim, export, import — +and asserts that the exported payload actually arrives. + +```bash +./e2e/run.sh +``` + +No provider credentials, no local JDK, no local Python. About 35 seconds with a +warm Gradle cache. The server log lands in `e2e/.logs/dtp.log` afterwards +whether the run passed or failed. + +This is also the shortest path to watching DTP do something real: + +```bash +docker compose run --rm gradle --no-daemon \ + :distributions:demo-server:shadowJar -PofflineData=true -PencryptionScheme=cleartext +docker compose up dtp # API on https://localhost:8080 (self-signed) +``` + +## How it fits together + +Three services in the root `docker-compose.yml`, two of them sharing the repo's +single `Dockerfile`: + +| Service | What it is | +|---|---| +| `gradle` | the Gradle build. Its `ENTRYPOINT` is `./gradlew`, so anything you pass it is a Gradle argument. | +| `dtp` | the same image, with the entrypoint overridden to `java -jar` the shadowJar that `gradle` produced. | +| `e2e` | stock `python:3.12-slim`, pytest, no Dockerfile. | + +`dtp` tees its output to a shared volume; `e2e` reads that file. Both also +bind-mount the repo, so no image needs rebuilding when a test changes. + +The transfer runs as a single JVM because it has to: `LocalJobStore` shares +state through `private static` maps and `JobMetadata` is a static singleton, so +the API and the worker only ever see the same job when co-located. That is what +`SingleVMMain` is for. + +### Why there is no dedicated server image + +The repository publishes no container image today — `release.yml` pushes Maven +artifacts, and `docker-build.yml` pushes nothing. The one image *definition* +that exists, `datatransferproject/demo:latest`, is generated at build time by +`:distributions:demo-server:dockerize`, which pulls in an Angular toolchain the +repo's own pinned dev image does not have, and never exposes 8080. + +So this harness pins a **contract**, not an artifact: *a service that answers +the DTP API on 8080 and writes its log where the driver can read it*. When a +real server image exists, replace `build`/`entrypoint`/`command` on the `dtp` +service with `image:` and nothing else here changes. + +It does already run the real packaged jar, including the `mergeServiceFiles()` +`META-INF/services` merge that `ServiceLoader` extension discovery depends on. + +## What a green run does and does not prove + +**Does:** a job was created and authorized over HTTP, a worker claimed it, the +exporter produced data, the copier moved it, and the importer received the +exact expected payload. + +**Does not:** + +- **Prove much from the job's own "success".** `Finished processing jobId: … with + 0 error(s).` is *necessary but not sufficient*. That count is the size of + `copier.getErrors()`, so a job that never copied anything reports zero too. + Build the jar with `-PencryptionScheme=jwe` while the driver still posts + `cleartext` and you get exactly that: `JobProcessor` logs `No auth decrypter + found for scheme …`, returns without transferring, and its `finally` block + still prints the clean-looking line. This is why the delivered payload is + asserted separately, and why failure is detected from `SEVERE` lines rather + than from the absence of a success line. +- **Cover any real provider adapter.** `offline-demo` has no HTTP surface, no + pagination and no sub-resources. It exercises the machinery, not a data path. +- **Cover the published image**, since there isn't one. + +### Why the completion signal is a log line + +Because there is no other one. `TransferJob.state` is +`private final State state = State.CREATED` with no getter, so Jackson never +emits it and `GET /api/transfer/{id}` reports no progress at all — +`GetTransferJobAction` fetches the real `PortabilityJob` and discards +everything but the ids (`TODO(#553)`). + +Giving `TransferJob` a real, nullable `state` and passing `job.state()` is +roughly fifteen lines; the route is already wired and the action already has +the job. It would let this harness poll HTTP instead of grepping, and would let +the Angular client show transfer progress, which it currently cannot. That is a +change to shipped code, so it is deliberately not bundled here. + +## Adding an adapter + +The driver is adapter-agnostic — service ids, vertical and encryption scheme +are parameters, and `dtp.py` names no provider. Adding one should cost a compose +service and a directory of JSON, not a driver change: + +1. Add a **WireMock standalone** service to `docker-compose.yml` with the + adapter's endpoints as `mappings/` (and `__files/` for any binary payloads). + No code — WireMock is configured entirely by JSON. +2. Make the adapter's base URL configurable. Most are a single + `private static final` constant, e.g. `ImgurTransferExtension.BASE_URL`; + they should read from `TransferServiceConfig`, defaulting to today's value. +3. Add a test module beside `test_offline_demo.py` with that adapter's ids and + fixtures. + +For the import side, WireMock's `/__admin/requests` admin API returns every +request it received with bodies intact — that is the assertion surface for +"what actually arrived", and a far better one than a log line. + +Derive mock request and response shapes from the adapter's existing +`MockWebServer` tests rather than from the client code, and seed **more than one +page** of data — otherwise pagination never fires and its absence passes. + +Note the fidelity limit that comes with all of this: a mock built from adapter +code tests DTP against *our reading* of a provider's API. It catches DTP +regressions. It does not catch the provider changing under us. + +## Traps encoded in the driver + +Each of these is a real inconsistency in the API, and each cost a debugging +session: + +- **Service ids differ in case.** `OfflineDemoAuthServiceExtension` declares + `OFFLINE-DEMO`; `OfflineDemoTransferExtension` declares `offline-demo`. + `PortabilityAuthServiceProviderRegistry` does an exact-match lookup while + `TransferExtension.supportsService` lowercases both sides, so only the auth + extension's spelling satisfies both. +- **The vertical is spelled two ways.** `OFFLINE-DATA` (the `@JsonValue`) in a + JSON body; `OFFLINE_DATA` in a path segment, where JAX-RS resolves the enum + with `Enum.valueOf`. +- **`encryptedAuthData` is triple-encoded.** It is a *string* holding JSON whose + two members are themselves *strings* holding serialized `AuthData` — + `JobProcessor` re-parses each with `readValue(…, AuthData.class)`. +- **The `{id}` path parameter is never read** on the POSTs. The id comes from the + body; the segment only matters for routing. +- **Job ids are base64url of the UUID's 36-character string**, not of its 16 + bytes. +- **A failed job can take the whole JVM with it.** + `JobCancelWatchingService` calls `System.exit(0)` on `ERROR`, and + `SingleVMMain`'s worker loop means that kills the API too. The driver reports + a server that stopped answering as a job failure rather than a flake. diff --git a/e2e/driver/conftest.py b/e2e/driver/conftest.py new file mode 100644 index 000000000..6e0796c50 --- /dev/null +++ b/e2e/driver/conftest.py @@ -0,0 +1,30 @@ +"""Fixtures shared by the e2e tests. + +Both the API base URL and the log path come from the environment so the driver +stays independent of how the server is deployed -- see docker-compose.yml. +""" + +import os + +import pytest + +from dtp import DtpClient, ServerLog + +BASE_URL = os.environ.get("DTP_BASE_URL", "https://localhost:8080") +LOG_PATH = os.environ.get("DTP_LOG", "/var/log/dtp/dtp.log") + +# Boot covers a JVM start plus every unconfigured provider adapter logging +# "Did you set X_KEY and X_SECRET?" on the way past. +READY_TIMEOUT = float(os.environ.get("DTP_READY_TIMEOUT", "180")) + + +@pytest.fixture(scope="session") +def client() -> DtpClient: + dtp = DtpClient(BASE_URL) + dtp.await_ready(READY_TIMEOUT) + return dtp + + +@pytest.fixture(scope="session") +def server_log() -> ServerLog: + return ServerLog(LOG_PATH) diff --git a/e2e/driver/dtp.py b/e2e/driver/dtp.py new file mode 100644 index 000000000..5e5e49779 --- /dev/null +++ b/e2e/driver/dtp.py @@ -0,0 +1,267 @@ +"""Black-box client for the DTP transfer API, plus a tail-follower for the +server log. + +Nothing here is adapter-specific: service ids, data type and encryption scheme +are all parameters. Adding an adapter should cost a compose service and some +fixtures, not a change to this file. + +The request sequence is the one implemented in +``client-rest/src/app/transfer/initiate-transfer.component.ts``, the only other +place it exists. +""" + +from __future__ import annotations + +import base64 +import json +import re +import time + +import requests +import urllib3 + +# JettyRestExtension defaults useHttps to true and demo-server's generated +# api.yaml never emits an override, so the server serves TLS on 8080 with the +# bundled self-signed keystore. Nothing here is testing certificate handling. +urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) + + +class TransferFailed(AssertionError): + """The job failed, or never finished. Carries log context.""" + + +def decode_job_id(encoded_job_id: str) -> str: + """Recover the job UUID from the id the API hands back. + + ``ActionUtils.encodeJobId`` is + ``BaseEncoding.base64Url().encode(uuid.toString().getBytes(UTF_8))`` -- so + it encodes the 36-character *string*, not the 16 raw bytes. 36 is divisible + by 3, so there is never any padding to add back. + """ + return base64.urlsafe_b64decode(encoded_job_id).decode("utf-8") + + +class DtpClient: + """Speaks to the API under its /api/* servlet prefix.""" + + def __init__(self, base_url: str, timeout: float = 30.0): + self.base_url = base_url.rstrip("/") + self.timeout = timeout + self.session = requests.Session() + self.session.verify = False + + # -- plumbing --------------------------------------------------------- + + def get(self, path: str) -> dict: + return self._send("GET", path) + + def post(self, path: str, body: dict) -> dict: + return self._send("POST", path, body) + + def _send(self, method: str, path: str, body: dict | None = None) -> dict: + response = self.session.request( + method, + f"{self.base_url}{path}", + json=body, + timeout=self.timeout, + ) + if not response.ok: + raise AssertionError( + f"{method} {path} returned {response.status_code}: {response.text}" + ) + return response.json() + + def await_ready(self, timeout: float = 120.0) -> None: + """Block until the API answers. This is the readiness gate; the compose + file deliberately has no healthcheck.""" + deadline = time.monotonic() + timeout + last = None + while time.monotonic() < deadline: + try: + self.get("/api/datatypes") + return + except Exception as exc: # noqa: BLE001 - any failure means not ready + last = exc + time.sleep(1) + raise TimeoutError(f"API not ready within {timeout}s; last error: {last}") + + def is_alive(self) -> bool: + try: + self.get("/api/datatypes") + return True + except Exception: # noqa: BLE001 + return False + + # -- the transfer sequence -------------------------------------------- + + def create_job( + self, + export_service: str, + import_service: str, + data_type: str, + encryption_scheme: str, + callback_url: str, + ) -> str: + """Returns the base64url-encoded job id, which every later call reuses + verbatim. + + ``data_type`` here is the JSON form -- the DataVertical's @JsonValue, + e.g. "OFFLINE-DATA". In a path segment it is the enum constant instead + (see :meth:`services_for`). + """ + job = self.post( + "/api/transfer", + { + "exportService": export_service, + "importService": import_service, + "exportCallbackUrl": callback_url, + "importCallbackUrl": callback_url, + "dataType": data_type, + "encryptionScheme": encryption_scheme, + }, + ) + return job["id"] + + def services_for(self, data_type_enum: str) -> dict: + """``data_type_enum`` is the enum *constant* (e.g. "OFFLINE_DATA"). + + This is a JAX-RS enum path param resolved by Enum.valueOf, so it does + not accept the "OFFLINE-DATA" spelling a JSON body requires. + """ + return self.get(f"/api/transfer/services/{data_type_enum}") + + def generate_auth(self, encoded_job_id: str, mode: str, callback_url: str) -> str: + """Returns an already-serialized AuthData *string*, not an object. + + The @PathParam is never read -- the id comes from the body -- but the + path segment is still needed for routing. + """ + response = self.post( + f"/api/transfer/{encoded_job_id}/generate", + { + "id": encoded_job_id, + "authToken": "unused-without-a-real-oauth-flow", + "mode": mode, + "callbackUrl": callback_url, + }, + ) + return response["authData"] + + def reserve_worker(self, encoded_job_id: str) -> None: + """Moves the job to CREDS_AVAILABLE so a worker can pick it up.""" + self.post(f"/api/transfer/worker/{encoded_job_id}", {"id": encoded_job_id}) + + def await_worker_claim(self, encoded_job_id: str, timeout: float = 60.0) -> str: + """Poll until the worker has claimed the job and published its key. + + ReserveWorkerAction returns an empty-string key immediately; the real + key only appears once the worker's JobPollingService has moved the job + to CREDS_ENCRYPTION_KEY_GENERATED. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + worker = self.get(f"/api/transfer/worker/{encoded_job_id}") + key = worker.get("publicKey") + if key: + return key + time.sleep(0.1) + raise TimeoutError(f"No worker claimed job {encoded_job_id} within {timeout}s") + + def start_job(self, encoded_job_id: str, export_auth: str, import_auth: str) -> None: + """Hand the worker its credentials. + + Under the cleartext scheme ``ClearTextAuthDataDecryptService.decrypt`` + is a plain ``readValue(encrypted, AuthDataPair.class)`` and the private + key is ignored, so "encrypted" auth data is just the serialized pair. + + Note the double encoding: ``encryptedAuthData`` is a *string* holding + JSON, and both members of that JSON are themselves *strings* holding + serialized AuthData -- JobProcessor re-parses each one with + ``readValue(..., AuthData.class)``. + """ + self.post( + f"/api/transfer/{encoded_job_id}/start", + { + "id": encoded_job_id, + "encryptedAuthData": json.dumps( + {"exportAuthData": export_auth, "importAuthData": import_auth} + ), + }, + ) + + +class ServerLog: + """Reads the log the dtp container tees onto a shared volume. + + This is the completion signal because there is no other one: TransferJob's + `state` field is hardcoded to CREATED with no getter, so Jackson never + emits it and GET /api/transfer/{id} reports no progress at all. + """ + + def __init__(self, path: str): + self.path = path + + def read(self) -> str: + try: + with open(self.path, "r", encoding="utf-8", errors="replace") as handle: + return handle.read() + except FileNotFoundError: + return "" + + def tail(self, lines: int = 40) -> str: + return "\n".join(self.read().splitlines()[-lines:]) + + def assert_contains(self, needle: str, why: str) -> None: + """Assert on the log without pytest dumping all of it into the report. + + A plain ``assert needle in log.read()`` prints the entire ~30KB server + log as the assertion's left-hand side, which buries the actual failure. + """ + if needle not in self.read(): + raise TransferFailed( + f"{why}: {needle!r} never appeared in the server log.\n\n" + f"--- server log (tail) ---\n{self.tail()}" + ) + + def wait_for( + self, + pattern: str, + timeout: float = 60.0, + fail_on: str | None = None, + client: DtpClient | None = None, + ) -> re.Match: + """Wait for ``pattern``, failing immediately if ``fail_on`` shows up. + + Racing the two is what makes a broken transfer fail in milliseconds + with the server's own error, rather than at timeout. It matters here: + JobCancelWatchingService calls System.exit(0) on ERROR, and because + SingleVMMain's WorkerRunner loops forever that takes the whole JVM -- + API included -- with it. The severe log line is written before the job + is marked ERROR, so this sees it first. + """ + deadline = time.monotonic() + timeout + while time.monotonic() < deadline: + contents = self.read() + if fail_on: + failure = re.search(fail_on, contents) + if failure: + raise TransferFailed( + f"Server reported failure: {failure.group(0)}\n\n" + f"--- server log (tail) ---\n{self.tail()}" + ) + match = re.search(pattern, contents) + if match: + return match + time.sleep(0.1) + + died = client is not None and not client.is_alive() + note = ( + "\nThe server is no longer answering -- the JVM exited, which is " + "what JobCancelWatchingService does on a failed job." + if died + else "" + ) + raise TransferFailed( + f"Timed out after {timeout}s waiting for /{pattern}/.{note}\n\n" + f"--- server log (tail) ---\n{self.tail()}" + ) diff --git a/e2e/driver/pytest.ini b/e2e/driver/pytest.ini new file mode 100644 index 000000000..f902736c0 --- /dev/null +++ b/e2e/driver/pytest.ini @@ -0,0 +1,6 @@ +[pytest] +# The server presents demo-server's bundled self-signed certificate, and the +# driver deliberately does not verify it -- see dtp.py. urllib3.disable_warnings +# does not survive pytest's own warnings filter, so silence it here. +filterwarnings = + ignore::urllib3.exceptions.InsecureRequestWarning diff --git a/e2e/driver/requirements.txt b/e2e/driver/requirements.txt new file mode 100644 index 000000000..000f8692c --- /dev/null +++ b/e2e/driver/requirements.txt @@ -0,0 +1,2 @@ +pytest==8.3.4 +requests==2.32.3 diff --git a/e2e/driver/test_offline_demo.py b/e2e/driver/test_offline_demo.py new file mode 100644 index 000000000..28212ac3a --- /dev/null +++ b/e2e/driver/test_offline_demo.py @@ -0,0 +1,102 @@ +"""A complete offline-demo -> offline-demo transfer, driven over HTTP. + +This is the credential-free end-to-end path: OfflineDemoAuthServiceExtension +bypasses OAuth entirely -- its "authorization URL" points straight back at the +local callback with a hardcoded code, and generateAuthData returns a fixed +token -- so no provider API keys are involved at any step. +""" + +import re + +from dtp import decode_job_id + +# OfflineDemoAuthServiceExtension declares "OFFLINE-DEMO" while +# OfflineDemoTransferExtension declares "offline-demo". +# PortabilityAuthServiceProviderRegistry does an exact-match MapBinder lookup +# with no normalisation, whereas TransferExtension.supportsService lowercases +# both sides -- so only the auth extension's spelling satisfies both. +SERVICE = "OFFLINE-DEMO" + +# The same vertical, spelled two ways. In a JSON body Jackson wants the +# DataVertical's @JsonValue; in a path segment JAX-RS resolves the enum +# constant with Enum.valueOf. +DATA_TYPE_JSON = "OFFLINE-DATA" +DATA_TYPE_ENUM = "OFFLINE_DATA" + +# Must match the security extension baked into the jar (see e2e/run.sh). +# JobProcessor.getAuthDecryptService compares with String.equals and, on a +# mismatch, returns *without transferring* -- the finally block still marks the +# job ERROR, so it is a silent no-transfer rather than a silent success. +ENCRYPTION_SCHEME = "cleartext" + +# offline-demo never dereferences this; it exists because the API requires it. +CALLBACK_URL = "http://localhost:3000/callback/offline-demo" + +# Set by OfflineDemoExporter. Asserting on the value is what distinguishes +# "a job ran" from "the data arrived". +EXPECTED_PAYLOAD = "offline-demo data" + +TRANSFER_TIMEOUT = 120.0 + + +def test_api_advertises_the_credential_free_vertical(client): + services = client.services_for(DATA_TYPE_ENUM) + + assert SERVICE in services["exportServices"] + assert SERVICE in services["importServices"] + + +def test_transfer_completes_and_delivers_the_payload(client, server_log): + encoded_job_id = client.create_job( + export_service=SERVICE, + import_service=SERVICE, + data_type=DATA_TYPE_JSON, + encryption_scheme=ENCRYPTION_SCHEME, + callback_url=CALLBACK_URL, + ) + assert encoded_job_id + + export_auth = client.generate_auth(encoded_job_id, "EXPORT", CALLBACK_URL) + import_auth = client.generate_auth(encoded_job_id, "IMPORT", CALLBACK_URL) + assert export_auth and import_auth + + client.reserve_worker(encoded_job_id) + client.await_worker_claim(encoded_job_id) + + client.start_job(encoded_job_id, export_auth, import_auth) + + job_id = re.escape(decode_job_id(encoded_job_id)) + + # Wait for the job to finish, and bail out the moment anything is logged at + # SEVERE against this job id. A clean run emits no SEVERE lines at all, so + # the pattern needs no list of known failures and stays adapter-agnostic. + # + # "with 0 error(s)" is necessary but NOT sufficient, which is worth being + # explicit about because it is a trap. That count is the size of + # copier.getErrors() -- the errors the idempotent import executor logged -- + # so a job that never copied anything also reports zero. Pointing the jar + # at a different encryptionScheme than the driver posts reproduces it + # exactly: JobProcessor logs "No auth decrypter found for scheme ...", + # returns without transferring, and its finally block still prints + # "Finished processing ... with 0 error(s)". That is why the delivered + # payload is asserted separately below, and why failure is detected from + # the SEVERE line rather than from the absence of a success line. + server_log.wait_for( + rf"Finished processing jobId: {job_id} with 0 error\(s\)\.", + timeout=TRANSFER_TIMEOUT, + fail_on=rf"SEVERE[^\n]*{job_id}", + client=client, + ) + + # Asserted separately, and on purpose: a job can report success without + # having moved anything. OfflineDemoImporter's println is the only artifact + # a successful import leaves behind. + # + # Phrased through assert_contains rather than `in server_log.read()` so a + # failure prints the tail of the log instead of all 30KB of it. + server_log.assert_contains( + "Received offline data:", "the importer never received anything" + ) + server_log.assert_contains( + EXPECTED_PAYLOAD, "the importer ran but the exported payload did not arrive" + ) diff --git a/e2e/run.sh b/e2e/run.sh new file mode 100755 index 000000000..34c97b192 --- /dev/null +++ b/e2e/run.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# +# Runs a complete offline-demo -> offline-demo transfer and exits non-zero if it +# does not arrive. No provider credentials, no local JDK, no local Python. +# +# ./e2e/run.sh +# +# The server log lands in e2e/.logs/dtp.log afterwards, whether the run passed +# or failed. +set -euo pipefail + +cd "$(dirname "${BASH_SOURCE[0]}")/.." + +LOG_DIR="e2e/.logs" + +# Capture the server log before tearing anything down, so a failure is +# diagnosable without re-running. The redirect happens here on the host, which +# is also what keeps the file owned by you rather than by root. +capture_and_clean() { + local rc=$? + mkdir -p "$LOG_DIR" + docker compose logs --no-color --no-log-prefix dtp > "$LOG_DIR/dtp.log" 2>/dev/null || true + # No -v: that would also drop gradle-cache and make every run a cold build. + docker compose down --remove-orphans >/dev/null 2>&1 || true + if [[ $rc -ne 0 ]]; then + echo + echo "FAILED. Server log: $LOG_DIR/dtp.log" + fi + exit "$rc" +} +trap capture_and_clean EXIT + +docker compose down --remove-orphans >/dev/null 2>&1 || true + +# Built through the `gradle` service, whose ENTRYPOINT is already ./gradlew, so +# these arguments pass straight through. That single-sources the pinned Gradle +# 6.9.2 / JDK 11 toolchain from the root Dockerfile and reuses the warm +# gradle-cache volume, which a `docker build` stage could not mount. +# +# shadowJar has no path to copyWebApp, so no Node or Angular toolchain is +# involved -- only `dockerize` drags those in. +echo "==> Building the demo-server jar (offline-demo, cleartext)" +docker compose run --rm gradle --no-daemon \ + :distributions:demo-server:shadowJar \ + -PofflineData=true \ + -PencryptionScheme=cleartext + +echo +echo "==> Running the transfer" +docker compose run --rm e2e From c6d5b845fa3f4d67d4e397837b79474eaebdf790 Mon Sep 17 00:00:00 2001 From: Alex Kulikov Date: Tue, 8 Sep 2026 01:54:46 +0100 Subject: [PATCH 3/4] feat(imgur): read base and auth URLs from service config ImgurTransferExtension hardcoded the API root and ImgurOAuthConfig hardcoded the authorization and token endpoints, so pointing the adapter at a staging environment or a test double meant rebuilding it. Both now read from config/imgur.yaml on the classpath, falling back to Imgur's own endpoints when no file is present: serviceConfig: baseUrl: https://api.imgur.com/3 authUrl: https://api.imgur.com/oauth2/authorize tokenUrl: https://api.imgur.com/oauth2/token This follows the convention Flickr, Deezer and Synology already use for other per-service settings; Imgur had simply not adopted it. Both the exporter and the importer already took baseUrl as a constructor parameter, so only the extension-level constants needed to change. One asymmetry worth noting for review: ImgurOAuthConfig calls the static TransferServiceConfig.getForService rather than taking an injected instance, because OAuth2ServiceExtension is never handed a service-scoped one the way WorkerModule hands one to a TransferExtension. Behaviour is unchanged when no config/imgur.yaml is on the classpath, which is the case for every distribution in this repo today. Adds the first tests for either class. --- .../ImgurOAuthConfig.java | 58 ++++++++++++- .../auth/imgur/ImgurOAuthConfigTest.java | 87 +++++++++++++++++++ .../imgur/ImgurTransferExtension.java | 28 +++++- .../imgur/ImgurTransferExtensionTest.java | 62 +++++++++++++ 4 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 extensions/auth/portability-auth-imgur/src/test/java/org/datatransferproject/auth/imgur/ImgurOAuthConfigTest.java create mode 100644 extensions/data-transfer/portability-data-transfer-imgur/src/test/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtensionTest.java diff --git a/extensions/auth/portability-auth-imgur/src/main/java/org.datatransferproject.auth.imgur/ImgurOAuthConfig.java b/extensions/auth/portability-auth-imgur/src/main/java/org.datatransferproject.auth.imgur/ImgurOAuthConfig.java index 94a761806..9ef692105 100644 --- a/extensions/auth/portability-auth-imgur/src/main/java/org.datatransferproject.auth.imgur/ImgurOAuthConfig.java +++ b/extensions/auth/portability-auth-imgur/src/main/java/org.datatransferproject.auth.imgur/ImgurOAuthConfig.java @@ -18,32 +18,84 @@ import static org.datatransferproject.types.common.models.DataVertical.PHOTOS; +import com.fasterxml.jackson.databind.JsonNode; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; +import java.io.IOException; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.datatransferproject.auth.OAuth2Config; import org.datatransferproject.types.common.models.DataVertical; +import org.datatransferproject.types.transfer.serviceconfig.TransferServiceConfig; /** * Class that provides Imgur-specific information for OAuth2 * See https://apidocs.imgur.com/#authorization-and-oauth + * + *

The authorization and token endpoints default to Imgur's, and may be overridden from + * {@code config/imgur.yaml} on the classpath -- the same file {@code ImgurTransferExtension} reads + * its {@code baseUrl} from. That exists so a deployer can point the adapter at a staging or test + * double without rebuilding; nothing else changes behaviour. */ public class ImgurOAuthConfig implements OAuth2Config { + private static final String SERVICE_NAME = "Imgur"; + + @VisibleForTesting + static final String DEFAULT_AUTH_URL = "https://api.imgur.com/oauth2/authorize"; + + @VisibleForTesting + static final String DEFAULT_TOKEN_URL = "https://api.imgur.com/oauth2/token"; + + private final String authUrl; + private final String tokenUrl; + + public ImgurOAuthConfig() { + this(readServiceConfig()); + } + + @VisibleForTesting + ImgurOAuthConfig(Optional serviceConfig) { + this.authUrl = configuredOrDefault(serviceConfig, "authUrl", DEFAULT_AUTH_URL); + this.tokenUrl = configuredOrDefault(serviceConfig, "tokenUrl", DEFAULT_TOKEN_URL); + } + + /** + * Reads {@code config/imgur.yaml} if one is on the classpath. + * + *

Unlike the transfer extension, an {@link + * org.datatransferproject.auth.OAuth2ServiceExtension} is handed no service-scoped {@code + * TransferServiceConfig}, so this reads it directly. A missing or unreadable file is not an + * error -- it just means the defaults apply. + */ + private static Optional readServiceConfig() { + try { + return TransferServiceConfig.getForService(SERVICE_NAME).getServiceConfig(); + } catch (IOException e) { + return Optional.empty(); + } + } + + private static String configuredOrDefault( + Optional serviceConfig, String field, String fallback) { + return serviceConfig.map(node -> node.path(field).asText(fallback)).orElse(fallback); + } + @Override public String getServiceName() { - return "Imgur"; + return SERVICE_NAME; } @Override public String getAuthUrl() { - return "https://api.imgur.com/oauth2/authorize"; + return authUrl; } @Override public String getTokenUrl() { - return "https://api.imgur.com/oauth2/token"; + return tokenUrl; } // Imgur doesn't require scopes diff --git a/extensions/auth/portability-auth-imgur/src/test/java/org/datatransferproject/auth/imgur/ImgurOAuthConfigTest.java b/extensions/auth/portability-auth-imgur/src/test/java/org/datatransferproject/auth/imgur/ImgurOAuthConfigTest.java new file mode 100644 index 000000000..ec858d5a4 --- /dev/null +++ b/extensions/auth/portability-auth-imgur/src/test/java/org/datatransferproject/auth/imgur/ImgurOAuthConfigTest.java @@ -0,0 +1,87 @@ +/* + * Copyright 2026 The Data Transfer Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.datatransferproject.auth.imgur; + +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; + +import com.fasterxml.jackson.databind.JsonNode; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.util.Optional; +import org.datatransferproject.types.transfer.serviceconfig.TransferServiceConfig; +import org.junit.jupiter.api.Test; + +public class ImgurOAuthConfigTest { + + private static Optional serviceConfig(String yaml) throws IOException { + return TransferServiceConfig.create(new ByteArrayInputStream(yaml.getBytes(UTF_8))) + .getServiceConfig(); + } + + @Test + public void defaultsToImgursOwnEndpoints() { + ImgurOAuthConfig config = new ImgurOAuthConfig(Optional.empty()); + + assertThat(config.getAuthUrl()).isEqualTo(ImgurOAuthConfig.DEFAULT_AUTH_URL); + assertThat(config.getTokenUrl()).isEqualTo(ImgurOAuthConfig.DEFAULT_TOKEN_URL); + } + + @Test + public void readsBothEndpointsFromServiceConfig() throws IOException { + ImgurOAuthConfig config = + new ImgurOAuthConfig( + serviceConfig( + "serviceConfig:\n" + + " authUrl: \"https://imgur.example/oauth2/authorize\"\n" + + " tokenUrl: \"https://imgur.example/oauth2/token\"\n")); + + assertThat(config.getAuthUrl()).isEqualTo("https://imgur.example/oauth2/authorize"); + assertThat(config.getTokenUrl()).isEqualTo("https://imgur.example/oauth2/token"); + } + + @Test + public void overridesEachEndpointIndependently() throws IOException { + // Only tokenUrl is strictly load-bearing -- generateAuthData dereferences it, + // while the auth URL is only ever handed to a browser -- so overriding one + // without the other has to leave the other at its default rather than empty. + ImgurOAuthConfig config = + serviceConfigured("serviceConfig:\n tokenUrl: \"https://imgur.example/oauth2/token\"\n"); + + assertThat(config.getTokenUrl()).isEqualTo("https://imgur.example/oauth2/token"); + assertThat(config.getAuthUrl()).isEqualTo(ImgurOAuthConfig.DEFAULT_AUTH_URL); + } + + @Test + public void keepsDefaultsWhenTheConfigHasNoServiceSection() throws IOException { + ImgurOAuthConfig config = serviceConfigured("perUserRateLimit: 10"); + + assertThat(config.getAuthUrl()).isEqualTo(ImgurOAuthConfig.DEFAULT_AUTH_URL); + assertThat(config.getTokenUrl()).isEqualTo(ImgurOAuthConfig.DEFAULT_TOKEN_URL); + } + + @Test + public void keepsTheServiceNameTheRegistryKeysOn() { + // PortabilityAuthServiceProviderRegistry does an exact-match lookup on this + // string, so a change here silently breaks every Imgur job at creation time. + assertThat(new ImgurOAuthConfig(Optional.empty()).getServiceName()).isEqualTo("Imgur"); + } + + private static ImgurOAuthConfig serviceConfigured(String yaml) throws IOException { + return new ImgurOAuthConfig(serviceConfig(yaml)); + } +} diff --git a/extensions/data-transfer/portability-data-transfer-imgur/src/main/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtension.java b/extensions/data-transfer/portability-data-transfer-imgur/src/main/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtension.java index 1b514e7f0..de250593b 100644 --- a/extensions/data-transfer/portability-data-transfer-imgur/src/main/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtension.java +++ b/extensions/data-transfer/portability-data-transfer-imgur/src/main/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtension.java @@ -19,9 +19,12 @@ import static org.datatransferproject.types.common.models.DataVertical.PHOTOS; import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import java.util.Optional; import okhttp3.OkHttpClient; import org.datatransferproject.api.launcher.ExtensionContext; import org.datatransferproject.api.launcher.Monitor; @@ -32,11 +35,14 @@ import org.datatransferproject.spi.transfer.extension.TransferExtension; import org.datatransferproject.spi.transfer.provider.Exporter; import org.datatransferproject.spi.transfer.provider.Importer; +import org.datatransferproject.types.transfer.serviceconfig.TransferServiceConfig; /** Extension for transferring Imgur data */ public class ImgurTransferExtension implements TransferExtension { private static final String SERVICE_ID = "Imgur"; - private static final String BASE_URL = "https://api.imgur.com/3"; + + @VisibleForTesting + static final String DEFAULT_BASE_URL = "https://api.imgur.com/3"; private boolean initialized = false; @@ -58,12 +64,28 @@ public void initialize(ExtensionContext context) { OkHttpClient client = context.getService(OkHttpClient.class); TemporaryPerJobDataStore jobStore = context.getService(TemporaryPerJobDataStore.class); - exporter = new ImgurPhotosExporter(monitor, client, mapper, jobStore, BASE_URL); - importer = new ImgurPhotosImporter(monitor, client, mapper, jobStore, BASE_URL); + String baseUrl = baseUrl(context.getService(TransferServiceConfig.class)); + + exporter = new ImgurPhotosExporter(monitor, client, mapper, jobStore, baseUrl); + importer = new ImgurPhotosImporter(monitor, client, mapper, jobStore, baseUrl); initialized = true; } + /** + * The API root, from {@code config/imgur.yaml} if one is on the classpath. + * + *

Follows the convention Flickr and Deezer already use, so a deployer can point the adapter at + * a staging endpoint or a test double without rebuilding. Defaults to Imgur's own. + */ + @VisibleForTesting + static String baseUrl(TransferServiceConfig serviceConfig) { + Optional config = serviceConfig.getServiceConfig(); + return config + .map(node -> node.path("baseUrl").asText(DEFAULT_BASE_URL)) + .orElse(DEFAULT_BASE_URL); + } + @Override public String getServiceId() { return SERVICE_ID; diff --git a/extensions/data-transfer/portability-data-transfer-imgur/src/test/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtensionTest.java b/extensions/data-transfer/portability-data-transfer-imgur/src/test/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtensionTest.java new file mode 100644 index 000000000..bbb505daa --- /dev/null +++ b/extensions/data-transfer/portability-data-transfer-imgur/src/test/java/org/datatransferproject/datatransfer/imgur/ImgurTransferExtensionTest.java @@ -0,0 +1,62 @@ +/* + * Copyright 2026 The Data Transfer Project Authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.datatransferproject.datatransfer.imgur; + +import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import org.datatransferproject.types.transfer.serviceconfig.TransferServiceConfig; +import org.junit.jupiter.api.Test; + +public class ImgurTransferExtensionTest { + + private static TransferServiceConfig configFrom(String yaml) throws IOException { + return TransferServiceConfig.create(new ByteArrayInputStream(yaml.getBytes(UTF_8))); + } + + @Test + public void usesImgursOwnApiWhenNothingIsConfigured() { + assertThat(ImgurTransferExtension.baseUrl(TransferServiceConfig.getDefaultInstance())) + .isEqualTo(ImgurTransferExtension.DEFAULT_BASE_URL); + } + + @Test + public void usesImgursOwnApiWhenTheConfigHasNoServiceSection() throws IOException { + // A config file that only sets a rate limit is the shape Flickr and Deezer + // ship, so it must not be read as "override the base URL with nothing". + assertThat(ImgurTransferExtension.baseUrl(configFrom("perUserRateLimit: 10"))) + .isEqualTo(ImgurTransferExtension.DEFAULT_BASE_URL); + } + + @Test + public void readsTheBaseUrlFromServiceConfig() throws IOException { + TransferServiceConfig config = + configFrom("serviceConfig:\n baseUrl: \"https://imgur.example/3\"\n"); + + assertThat(ImgurTransferExtension.baseUrl(config)).isEqualTo("https://imgur.example/3"); + } + + @Test + public void ignoresAServiceConfigThatSetsOtherKeys() throws IOException { + TransferServiceConfig config = configFrom("serviceConfig:\n tokenUrl: \"https://x/token\"\n"); + + assertThat(ImgurTransferExtension.baseUrl(config)) + .isEqualTo(ImgurTransferExtension.DEFAULT_BASE_URL); + } +} From 5af95a59facea3d9c53641559238ddf3a79d6159 Mon Sep 17 00:00:00 2001 From: Alex Kulikov Date: Tue, 8 Sep 2026 02:03:55 +0100 Subject: [PATCH 4/4] test(e2e): cover a real adapter with a mocked Imgur API The harness so far only ran offline-demo -> offline-demo: a fixed string exported by a ten-line exporter and printed by a System.out.println importer. That proves the machinery -- job creation, auth, worker claim, copier, importer -- and nothing about a data path. Adds an Imgur -> Imgur suite driving the real, unmodified ImgurPhotosExporter and ImgurPhotosImporter against a WireMock stand-in for api.imgur.com. A green run now covers a real OAuth2 token exchange, pagination on two independent axes, sub-resource recursion, an image byte round trip through LocalTempFileStore, and the album-id mapping IdempotentImportExecutor maintains across copy iterations. Each adapter gets its own freshly started dtp container. LocalJobStore keeps jobs in private static maps and LocalTempFileStore keeps files on disk, and nothing clears either between jobs, so a shared server would make isolation a matter of luck and ordering. run.sh loops over adapters, runs all of them even when one fails, and captures per-adapter logs and mock request journals. A cold JVM per adapter costs about 17 seconds. The mock URLs reach the adapter through a classpath-prepended directory: config resolution is classpath-only and first-match-per-filename, so dtp now runs `java -cp /workspace/e2e/config: SingleVMMain` rather than `java -jar`, which ignores -cp. e2e/config holds only files the jar does not ship, so it is inert for every other adapter. Two assertions in this suite were initially vacuous and are recorded in e2e/README.md so the next person does not repeat them: - Comparing delivered bytes against the fixture the mock serves makes the fixture its own oracle; corrupting it corrupts both sides. The assertion still proves each distinct fixture arrived exactly once, unmodified, but its negative check has to break the pipeline rather than the fixture. - Asserting that page 1 was requested proves nothing, because the exporter requests page N+1 whenever page N is non-empty. It now asserts page 2, the first request that proves page 1 had content. Verified by hand, since no workflow runs on a stacked PR: both adapters green together and standalone, `gradle check` green, and four negative checks confirming the assertions bite. --- .gitignore | 3 + docker-compose.yml | 55 ++++- e2e/README.md | 144 ++++++++--- e2e/config/config/imgur.yaml | 15 ++ e2e/driver/conftest.py | 9 + e2e/driver/dtp.py | 11 + e2e/driver/pytest.ini | 7 + e2e/driver/test_imgur.py | 228 ++++++++++++++++++ e2e/driver/test_offline_demo.py | 4 + e2e/driver/wiremock.py | 71 ++++++ .../imgur/__files/files/album1Photo1.jpg | Bin 0 -> 167 bytes .../imgur/__files/files/album1Photo2.jpg | Bin 0 -> 167 bytes .../imgur/__files/files/album2Photo1.jpg | Bin 0 -> 167 bytes .../imgur/__files/files/album3Photo1.jpg | Bin 0 -> 167 bytes .../imgur/__files/files/nonAlbumPhoto1.jpg | Bin 0 -> 169 bytes .../imgur/__files/files/nonAlbumPhoto2.jpg | Bin 0 -> 169 bytes .../imgur/mappings/export-account-images.json | 111 +++++++++ .../imgur/mappings/export-album-images.json | 94 ++++++++ e2e/mocks/imgur/mappings/export-albums.json | 94 ++++++++ e2e/mocks/imgur/mappings/import.json | 112 +++++++++ e2e/mocks/imgur/mappings/oauth-token.json | 28 +++ e2e/run.sh | 101 ++++++-- 22 files changed, 1032 insertions(+), 55 deletions(-) create mode 100644 e2e/config/config/imgur.yaml create mode 100644 e2e/driver/test_imgur.py create mode 100644 e2e/driver/wiremock.py create mode 100644 e2e/mocks/imgur/__files/files/album1Photo1.jpg create mode 100644 e2e/mocks/imgur/__files/files/album1Photo2.jpg create mode 100644 e2e/mocks/imgur/__files/files/album2Photo1.jpg create mode 100644 e2e/mocks/imgur/__files/files/album3Photo1.jpg create mode 100644 e2e/mocks/imgur/__files/files/nonAlbumPhoto1.jpg create mode 100644 e2e/mocks/imgur/__files/files/nonAlbumPhoto2.jpg create mode 100644 e2e/mocks/imgur/mappings/export-account-images.json create mode 100644 e2e/mocks/imgur/mappings/export-album-images.json create mode 100644 e2e/mocks/imgur/mappings/export-albums.json create mode 100644 e2e/mocks/imgur/mappings/import.json create mode 100644 e2e/mocks/imgur/mappings/oauth-token.json diff --git a/.gitignore b/.gitignore index 0de387f16..90211408f 100644 --- a/.gitignore +++ b/.gitignore @@ -50,6 +50,9 @@ testem.log # e2e /e2e/*.js /e2e/*.map +# Run artifacts: server logs and mock request journals. The logs are already +# covered by *.log above, the journals are not. +/e2e/.logs/ client-rest/dist/ diff --git a/docker-compose.yml b/docker-compose.yml index 8fc15c316..3c0eda0f3 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -26,6 +26,11 @@ services: # two only ever see the same job when co-located). dtp: build: . + # In every adapter profile: run.sh brings up a *fresh* dtp per adapter, so + # no suite can see another's jobs or temp files. LocalJobStore's static maps + # and LocalTempFileStore's files accumulate across jobs and nothing clears + # them, so a shared container would make test isolation a matter of luck. + profiles: ["offline-demo", "imgur"] volumes: - .:/workspace - dtp-logs:/var/log/dtp @@ -36,6 +41,13 @@ services: # bypasses OAuth entirely. JWT_KEY: e2e-key JWT_SECRET: e2e-secret + # Never checked either, but required: OAuth2ServiceExtension.initialize + # swallows the missing-credential IOException and returns *without* + # setting `initialized`, so the failure surfaces much later as + # "Cannot get OAuth2DataGenerator before initialization" on the first + # POST /api/transfer. + IMGUR_KEY: e2e-imgur-key + IMGUR_SECRET: e2e-imgur-secret # Published so `docker compose up dtp` is also the credential-free way to # poke at a running DTP by hand. JettyTransport hardcodes 8080. ports: @@ -55,16 +67,47 @@ services: # logs dtp`, and run.sh captures the same stream to e2e/.logs/ afterwards. # Merging stderr matters -- ConsoleMonitor writes the server log to stderr # while OfflineDemoImporter prints the delivered payload to stdout. + # + # `-cp

:` rather than `-jar`, because `-jar` ignores -cp entirely. + # Config resolution is classpath-only -- TransferServiceConfig and + # ConfigUtils both use the *singular* getResourceAsStream, so first match + # per filename wins -- which makes a prepended directory the one supported + # way to feed an adapter a different base URL. e2e/config holds only files + # the jar does not ship, so this is purely additive and inert for adapters + # that have no file there. + # + # $$ escapes Compose's own interpolation so the shell gets $(...). entrypoint: ["bash", "-c"] command: - - "java -jar distributions/demo-server/build/libs/*-all.jar 2>&1 | tee /var/log/dtp/dtp.log" + - "java -cp /workspace/e2e/config:$$(ls distributions/demo-server/build/libs/*-all.jar) org.datatransferproject.bootstrap.vm.SingleVMMain 2>&1 | tee /var/log/dtp/dtp.log" + + # Stands in for api.imgur.com. Configured entirely by JSON -- mappings/ for + # the stubs, __files/ for the image bytes the exporter downloads during + # export. Its /__admin/requests journal is the assertion surface for what the + # importer actually sent, which is a far better one than a log line. + wiremock-imgur: + image: wiremock/wiremock:3.9.2 + profiles: ["imgur"] + # Read-only on purpose. WireMock runs as root and will happily mkdir any of + # mappings/ or __files/ that is missing, leaving root-owned directories in + # your checkout -- the same papercut PYTHONDONTWRITEBYTECODE avoids for the + # driver. It never needs to write unless it is recording. + volumes: + - ./e2e/mocks/imgur:/home/wiremock:ro + # Published so run.sh can pull the request journal from the host, keeping + # the artifact owned by you rather than by root. The image ships no curl, + # so `compose exec` is not an option. + ports: + - "18080:8080" + command: ["--verbose"] # Stock python image, no Dockerfile. Deps install into a cached volume. e2e: image: python:3.12-slim working_dir: /workspace - depends_on: - - dtp + # No depends_on: `dtp` is now profiled and run.sh starts it explicitly, one + # fresh container per adapter. The driver's await_ready() is the readiness + # gate regardless -- there has never been a healthcheck. volumes: - .:/workspace - dtp-logs:/var/log/dtp:ro @@ -72,12 +115,16 @@ services: environment: DTP_BASE_URL: https://dtp:8080 DTP_LOG: /var/log/dtp/dtp.log + WIREMOCK_IMGUR_URL: http://wiremock-imgur:8080 + # Which adapter's suite to run, as a pytest marker. Empty runs all of + # them, which is what a bare `docker compose run --rm e2e` does. + E2E_MARKER: "${E2E_MARKER:-}" # Keep the container from littering the bind-mounted repo with # root-owned __pycache__ directories. PYTHONDONTWRITEBYTECODE: "1" entrypoint: ["bash", "-c"] command: - - "pip install --quiet --root-user-action=ignore -r e2e/driver/requirements.txt && pytest e2e/driver -v -p no:cacheprovider" + - "pip install --quiet --root-user-action=ignore -r e2e/driver/requirements.txt && pytest e2e/driver -v -p no:cacheprovider $${E2E_MARKER:+-m $$E2E_MARKER}" volumes: gradle-cache: diff --git a/e2e/README.md b/e2e/README.md index 4b366965c..fd98f7252 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -4,19 +4,35 @@ Runs a complete DTP transfer — job creation, auth, worker claim, export, impor and asserts that the exported payload actually arrives. ```bash -./e2e/run.sh +./e2e/run.sh # every adapter +./e2e/run.sh imgur # just one ``` -No provider credentials, no local JDK, no local Python. About 35 seconds with a -warm Gradle cache. The server log lands in `e2e/.logs/dtp.log` afterwards -whether the run passed or failed. +No provider credentials, no local JDK, no local Python. About 55 seconds for +both adapters with a warm Gradle cache. Server logs and mock request journals +land in `e2e/.logs/`, one set per adapter, whether the run passed or failed. + +Two adapters are covered: + +| Adapter | What it proves | +|---|---| +| `offline-demo` | the machinery — a credential-free transfer with no HTTP surface at all | +| `imgur` | a real data path — a real, unmodified adapter paginating on two axes, recursing into sub-resources, and round-tripping image bytes through the temp store | + +**Each adapter gets its own freshly started `dtp` container.** That is not +tidiness: `LocalJobStore` keeps jobs in `private static` maps and +`LocalTempFileStore` keeps files on disk, and nothing clears either between +jobs. A shared server would make isolation a matter of luck and ordering, so +`run.sh` restarts one per adapter and each suite runs against a cold JVM. Every +adapter runs even if an earlier one fails, and the exit code reflects all of +them. This is also the shortest path to watching DTP do something real: ```bash docker compose run --rm gradle --no-daemon \ :distributions:demo-server:shadowJar -PofflineData=true -PencryptionScheme=cleartext -docker compose up dtp # API on https://localhost:8080 (self-signed) +docker compose --profile offline-demo up dtp # API on https://localhost:8080 (self-signed) ``` ## How it fits together @@ -27,8 +43,9 @@ single `Dockerfile`: | Service | What it is | |---|---| | `gradle` | the Gradle build. Its `ENTRYPOINT` is `./gradlew`, so anything you pass it is a Gradle argument. | -| `dtp` | the same image, with the entrypoint overridden to `java -jar` the shadowJar that `gradle` produced. | +| `dtp` | the same image, with the entrypoint overridden to run the shadowJar that `gradle` produced. | | `e2e` | stock `python:3.12-slim`, pytest, no Dockerfile. | +| `wiremock-imgur` | `wiremock/wiremock`, standing in for `api.imgur.com`. Configured entirely by JSON. | `dtp` tees its output to a shared volume; `e2e` reads that file. Both also bind-mount the repo, so no image needs rebuilding when a test changes. @@ -56,9 +73,11 @@ It does already run the real packaged jar, including the `mergeServiceFiles()` ## What a green run does and does not prove -**Does:** a job was created and authorized over HTTP, a worker claimed it, the -exporter produced data, the copier moved it, and the importer received the -exact expected payload. +**Does:** a job was created and authorized over HTTP — including a real OAuth2 +token exchange, for Imgur — a worker claimed it, the exporter produced data, the +copier recursed over paginated listings and sub-resources, image bytes made a +round trip through `LocalTempFileStore`, and the importer delivered every one of +them to the right album, byte for byte. **Does not:** @@ -71,9 +90,15 @@ exact expected payload. still prints the clean-looking line. This is why the delivered payload is asserted separately, and why failure is detected from `SEVERE` lines rather than from the absence of a success line. -- **Cover any real provider adapter.** `offline-demo` has no HTTP surface, no - pagination and no sub-resources. It exercises the machinery, not a data path. +- **Cover a real provider's API.** The Imgur suite drives the real, unmodified + `ImgurPhotosExporter` and `ImgurPhotosImporter`, but against a mock built from + the adapter's own test fixtures. It tests DTP against *our reading* of Imgur's + API, so it catches DTP regressions and not Imgur changing under us. +- **Cover any vertical but `PHOTOS` and `OFFLINE_DATA`**, or any adapter that + does not talk plain JSON over OkHttp. - **Cover the published image**, since there isn't one. +- **Cover JWE.** Only `cleartext` is exercised; the scheme is fixed at build + time, so the alternative needs a second jar. ### Why the completion signal is a log line @@ -91,26 +116,75 @@ change to shipped code, so it is deliberately not bundled here. ## Adding an adapter -The driver is adapter-agnostic — service ids, vertical and encryption scheme -are parameters, and `dtp.py` names no provider. Adding one should cost a compose -service and a directory of JSON, not a driver change: +The driver is adapter-agnostic — service ids, vertical and encryption scheme are +parameters, and `dtp.py` names no provider. Adding one costs a compose service +and a directory of JSON. `imgur` is the worked example; copy its shape. + +1. **Make the adapter's URLs configurable.** Most are a single + `private static final`, e.g. `MicrosoftTransferExtension.BASE_GRAPH_URL`. + They should read from `TransferServiceConfig`, defaulting to today's value — + the convention Flickr, Deezer and Synology already use for other settings. + Note that no adapter in the repo had a configurable URL on an *export* path + before Imgur; only import-only adapters (`Generic`, Synology) had adopted it. +2. **Point the adapter at the mock** with a `config/.yaml` under + `e2e/config/` — see below. +3. **Add a WireMock service** to `docker-compose.yml` under a profile named + after the adapter, with the endpoints as `mappings/` and any binary payloads + as `__files/`. No code. +4. **Add a suite** beside `test_imgur.py`, marked `@pytest.mark.` (the + marker must be registered in `pytest.ini`), and one line each in `run.sh`'s + `MOCKS` and `MOCK_PORT` tables. + +### How the mock URL reaches the adapter + +Config resolution is classpath-only — there is no environment-variable override +anywhere in the chain. `TransferServiceConfig.getForService(service)` reads +`config/.yaml` with the *singular* `getResourceAsStream`, so the first +match per filename wins. The `dtp` service therefore runs + +``` +java -cp /workspace/e2e/config: org.datatransferproject.bootstrap.vm.SingleVMMain +``` + +rather than `java -jar`, which ignores `-cp` entirely. + +Two things to know: + +- **The path is doubled.** `e2e/config` is the classpath entry and + `config/imgur.yaml` is the resource name, so the file lives at + `e2e/config/config/imgur.yaml`. +- **Prefer adding a file over shadowing one.** Adding a file the jar does not + ship (Imgur's case) is inert for everything else. Shadowing one it *does* + ship — `deezer.yaml`, `flickr.yaml`, `synology.yaml` — replaces it wholesale + rather than merging, silently dropping settings like `perUserRateLimit`. + +### Seeding fixtures + +Seed **more than one page** on every axis the adapter paginates, and assert that +the page *past* the seeded data was requested. Asserting on page 1 is not +enough: Imgur's exporter infers "there is more" from the current page being +non-empty, so it always requests page 1 even when page 0 was the last page with +data. Requesting page 2 is the first request that proves page 1 had content. + +Adapters that infer the end of a listing this way also need an **empty-page +terminator** stub, or the export never stops asking. + +Derive request and response shapes from the adapter's existing `MockWebServer` +tests rather than from the client code. -1. Add a **WireMock standalone** service to `docker-compose.yml` with the - adapter's endpoints as `mappings/` (and `__files/` for any binary payloads). - No code — WireMock is configured entirely by JSON. -2. Make the adapter's base URL configurable. Most are a single - `private static final` constant, e.g. `ImgurTransferExtension.BASE_URL`; - they should read from `TransferServiceConfig`, defaulting to today's value. -3. Add a test module beside `test_offline_demo.py` with that adapter's ids and - fixtures. +### Asserting on what arrived -For the import side, WireMock's `/__admin/requests` admin API returns every -request it received with bodies intact — that is the assertion surface for -"what actually arrived", and a far better one than a log line. +WireMock's `/__admin/requests` returns every request it received with bodies +intact — the assertion surface for "what actually arrived", and a far better one +than a log line. `wiremock.py` wraps it; `run.sh` saves the journal to +`e2e/.logs/-requests.json`. -Derive mock request and response shapes from the adapter's existing -`MockWebServer` tests rather than from the client code, and seed **more than one -page** of data — otherwise pagination never fires and its absence passes. +One caveat learned the hard way: if a test compares delivered bytes against the +fixture file the mock serves, the fixture is its own oracle, and corrupting it +corrupts both sides equally. That assertion still proves each distinct fixture +arrived exactly once, unmodified — enough to catch temp-store cross-talk, +truncation and duplicate imports — but to check that it bites, break the +*pipeline* (make the mock serve the wrong file), not the fixture. Note the fidelity limit that comes with all of this: a mock built from adapter code tests DTP against *our reading* of a provider's API. It catches DTP @@ -136,6 +210,20 @@ session: body; the segment only matters for routing. - **Job ids are base64url of the UUID's 36-character string**, not of its 16 bytes. +- **An adapter with real OAuth needs credentials set, even fake ones.** + `OAuth2ServiceExtension.initialize` catches the missing-credential + `IOException`, logs it at INFO, and returns *without* setting `initialized`. + The failure then surfaces much later, and nowhere near its cause, as + `Cannot get OAuth2DataGenerator before initialization` on the first + `POST /api/transfer`. Hence the dummy `IMGUR_KEY`/`IMGUR_SECRET` on the `dtp` + service — their values are never checked against anything. +- **A clean run is no longer entirely free of `SEVERE` lines.** On a transfer + where export and import are the *same* service, `WorkerModule` resolves one + extension instance and calls `initialize()` on it more than once; + `ImgurTransferExtension` logs each repeat at `SEVERE`. The fail-fast pattern + is `SEVERE[^\n]*` and those lines carry no job id, so it does not + trip — but the "a clean run emits zero SEVERE lines" assumption this harness + was built on is now only true per-job, not globally. - **A failed job can take the whole JVM with it.** `JobCancelWatchingService` calls `System.exit(0)` on `ERROR`, and `SingleVMMain`'s worker loop means that kills the API too. The driver reports diff --git a/e2e/config/config/imgur.yaml b/e2e/config/config/imgur.yaml new file mode 100644 index 000000000..ad83a0900 --- /dev/null +++ b/e2e/config/config/imgur.yaml @@ -0,0 +1,15 @@ +# Points the Imgur adapter at the WireMock stand-in instead of api.imgur.com. +# +# The doubled `config/` in the path is not a typo: e2e/config is what goes on +# the classpath (see the `dtp` service in docker-compose.yml), and the resource +# TransferServiceConfig.getForService("Imgur") looks up is `config/imgur.yaml`. +# +# This is additive -- the demo-server jar ships no imgur.yaml -- so it is inert +# for every other adapter, which is why the `dtp` service can prepend this +# directory unconditionally. Shadowing would be a different matter: config +# resolution is first-match-per-filename, so an imgur.yaml here would replace a +# jar one wholesale rather than merging with it. +serviceConfig: + baseUrl: "http://wiremock-imgur:8080/3" + authUrl: "http://wiremock-imgur:8080/oauth2/authorize" + tokenUrl: "http://wiremock-imgur:8080/oauth2/token" diff --git a/e2e/driver/conftest.py b/e2e/driver/conftest.py index 6e0796c50..520b60144 100644 --- a/e2e/driver/conftest.py +++ b/e2e/driver/conftest.py @@ -9,9 +9,11 @@ import pytest from dtp import DtpClient, ServerLog +from wiremock import WireMock BASE_URL = os.environ.get("DTP_BASE_URL", "https://localhost:8080") LOG_PATH = os.environ.get("DTP_LOG", "/var/log/dtp/dtp.log") +WIREMOCK_IMGUR_URL = os.environ.get("WIREMOCK_IMGUR_URL", "http://wiremock-imgur:8080") # Boot covers a JVM start plus every unconfigured provider adapter logging # "Did you set X_KEY and X_SECRET?" on the way past. @@ -28,3 +30,10 @@ def client() -> DtpClient: @pytest.fixture(scope="session") def server_log() -> ServerLog: return ServerLog(LOG_PATH) + + +@pytest.fixture(scope="session") +def imgur_mock() -> WireMock: + """Only started when run.sh brings up the `imgur` compose profile, which is + also the only time an @pytest.mark.imgur test is selected.""" + return WireMock(WIREMOCK_IMGUR_URL) diff --git a/e2e/driver/dtp.py b/e2e/driver/dtp.py index 5e5e49779..18334fe5b 100644 --- a/e2e/driver/dtp.py +++ b/e2e/driver/dtp.py @@ -211,6 +211,17 @@ def read(self) -> str: def tail(self, lines: int = 40) -> str: return "\n".join(self.read().splitlines()[-lines:]) + def count_matches(self, pattern: str) -> int: + """How many lines match ``pattern``. + + Exists for the copy-iteration assertion: PortabilityAbstractInMemoryDataCopier + logs "Copy iteration: N" once per recursion, so counting those lines is how + an adapter proves the copier actually recursed rather than returning + everything in one pass. Without it, an under-seeded fixture passes green + while covering none of what it claims to. + """ + return len(re.findall(pattern, self.read())) + def assert_contains(self, needle: str, why: str) -> None: """Assert on the log without pytest dumping all of it into the report. diff --git a/e2e/driver/pytest.ini b/e2e/driver/pytest.ini index f902736c0..daf89a6ec 100644 --- a/e2e/driver/pytest.ini +++ b/e2e/driver/pytest.ini @@ -4,3 +4,10 @@ # does not survive pytest's own warnings filter, so silence it here. filterwarnings = ignore::urllib3.exceptions.InsecureRequestWarning + +# One marker per adapter, so run.sh can give each its own dtp container. The +# names use underscores rather than the hyphens the adapter directories use -- +# `-m offline-demo` parses as the expression `offline and (not demo)`. +markers = + offline_demo: the credential-free offline-demo -> offline-demo transfer + imgur: Imgur -> Imgur against a WireMock stand-in for api.imgur.com diff --git a/e2e/driver/test_imgur.py b/e2e/driver/test_imgur.py new file mode 100644 index 000000000..752df4748 --- /dev/null +++ b/e2e/driver/test_imgur.py @@ -0,0 +1,228 @@ +"""An Imgur -> Imgur transfer against a WireMock stand-in for api.imgur.com. + +Where the offline-demo suite proves the machinery, this proves a data path: a +real, unmodified provider adapter paginating over HTTP, recursing into +sub-resources, downloading bytes into the temp store, and reading them back out +on the import side. + +Unlike offline-demo, Imgur runs a real OAuth2 token exchange. That is diverted +to the mock rather than skipped -- see e2e/config/config/imgur.yaml -- so the +driver's request sequence stays identical for both adapters. +""" + +import pathlib +import re + +import pytest + +from dtp import decode_job_id +from wiremock import decoded_image, form_params + +# ImgurOAuthConfig.getServiceName() and ImgurTransferExtension.SERVICE_ID are +# both "Imgur", and TransferExtension.supportsService lowercases, so unlike +# offline-demo one spelling works for both the auth registry's exact-match +# lookup and the transfer registry. +SERVICE = "Imgur" + +# PHOTOS is its own @JsonValue, so body and path spellings agree here too. +DATA_TYPE_JSON = "PHOTOS" +DATA_TYPE_ENUM = "PHOTOS" + +ENCRYPTION_SCHEME = "cleartext" + +# Never visited: the driver stands in for the browser leg of the OAuth flow. +CALLBACK_URL = "http://localhost:3000/callback/imgur" + +# What the mock's token endpoint hands back. Asserting on it is what proves the +# divert worked rather than a real request having silently failed. +EXPECTED_ACCESS_TOKEN = "e2e-access-token" + +TRANSFER_TIMEOUT = 180.0 + +FIXTURES = pathlib.Path(__file__).resolve().parents[1] / "mocks/imgur/__files/files" + +# Seeded albums, and the id the mock hands back for each. Photos are expected to +# arrive carrying the *returned* id, not the original -- that round trip through +# IdempotentImportExecutor's cache is the thing being checked. +EXPECTED_ALBUMS = { + "Album 1": "imported-album-1", + "Album 2": "imported-album-2", + "Album 3": "imported-album-3", + "Non-album photos": "imported-album-default", +} + +# Every photo the export should yield, and the album it belongs in. The two +# nonAlbum* entries are the ones the exporter has to *deduce*: the account-wide +# listing returns album photos too, and it keeps only the ids it has not already +# seen inside an album. +EXPECTED_PHOTOS = { + "album1Photo1": "imported-album-1", + "album1Photo2": "imported-album-1", + "album2Photo1": "imported-album-2", + "album3Photo1": "imported-album-3", + "nonAlbumPhoto1": "imported-album-default", + "nonAlbumPhoto2": "imported-album-default", +} + + +@pytest.mark.imgur +def test_auth_token_exchange_is_diverted_to_the_mock(client, imgur_mock): + """S1: the OAuth2 token exchange reaches WireMock, not api.imgur.com. + + This is the step that has no offline-demo equivalent. + OAuth2DataGenerator.generateAuthData POSTs to config.getTokenUrl() for + real, so without the divert the run either hangs on a network call or + fails against Imgur's actual API. + + It also covers the trap underneath: OAuth2ServiceExtension.initialize + swallows a missing-credential IOException and returns *without* setting + `initialized`, so absent IMGUR_KEY/IMGUR_SECRET this fails at + "Cannot get OAuth2DataGenerator before initialization" -- on job creation, + nowhere near the actual cause. + """ + imgur_mock.reset_requests() + + encoded_job_id = client.create_job( + export_service=SERVICE, + import_service=SERVICE, + data_type=DATA_TYPE_JSON, + encryption_scheme=ENCRYPTION_SCHEME, + callback_url=CALLBACK_URL, + ) + assert encoded_job_id + + export_auth = client.generate_auth(encoded_job_id, "EXPORT", CALLBACK_URL) + + assert EXPECTED_ACCESS_TOKEN in export_auth, ( + "the token exchange did not come back with the mock's token; " + f"got: {export_auth}" + ) + assert imgur_mock.received("POST", "/oauth2/token"), ( + "WireMock never saw the token request -- the tokenUrl override in " + "e2e/config/config/imgur.yaml did not take effect" + ) + + +@pytest.mark.imgur +def test_transfer_delivers_every_photo_to_the_right_album(client, server_log, imgur_mock): + """S2: a complete Imgur -> Imgur transfer, asserted on what the mock received. + + Deliberately one test rather than several. The transfer is a single + expensive act against shared state, and splitting the assertions across + tests would either re-run it or make them order-dependent -- both worse + than a long test body. + """ + imgur_mock.reset_requests() + + encoded_job_id = client.create_job( + export_service=SERVICE, + import_service=SERVICE, + data_type=DATA_TYPE_JSON, + encryption_scheme=ENCRYPTION_SCHEME, + callback_url=CALLBACK_URL, + ) + export_auth = client.generate_auth(encoded_job_id, "EXPORT", CALLBACK_URL) + import_auth = client.generate_auth(encoded_job_id, "IMPORT", CALLBACK_URL) + + client.reserve_worker(encoded_job_id) + client.await_worker_claim(encoded_job_id) + client.start_job(encoded_job_id, export_auth, import_auth) + + job_id = re.escape(decode_job_id(encoded_job_id)) + + # Same completion signal and same fail-fast heuristic as offline-demo, and + # for the same reason: "0 error(s)" is the size of copier.getErrors(), so a + # job that copied nothing reports it too. Everything below is what actually + # distinguishes a transfer from a no-op. + server_log.wait_for( + rf"Finished processing jobId: {job_id} with 0 error\(s\)\.", + timeout=TRANSFER_TIMEOUT, + fail_on=rf"SEVERE[^\n]*{job_id}", + client=client, + ) + + # -- the copier actually recursed ------------------------------------- + # + # Nine iterations are expected: albums pages 0/1/2, the three album image + # sub-resources, and non-album pages 0/1/2. Asserting ">1" rather than "==9" + # keeps this from breaking every time a fixture gains a row, while still + # failing if pagination silently stops firing. + iterations = server_log.count_matches(rf"Job {job_id}: Copy iteration: ") + assert iterations > 1, ( + f"the copier ran {iterations} iteration(s) -- it never recursed, so " + "neither pagination nor sub-resource traversal was exercised" + ) + + # Pagination specifically, on both axes. + # + # Note these check for page *2*, not page 1. The exporter derives "there is + # more" from the current page being non-empty, so it always requests page 1 + # -- even when page 0 was the last page with data. Asking for page 2 is + # therefore the first request that proves a second page actually had + # content, which is the property worth asserting. Checking page 1 would + # pass on single-page fixtures and quietly cover nothing. + assert imgur_mock.received("GET", "/3/account/me/albums/2"), ( + "album pagination never got past the first page of data -- the fixture " + "may have shrunk to a single page" + ) + assert imgur_mock.received("GET", "/3/account/me/images/2"), ( + "non-album photo pagination never got past the first page of data" + ) + + # -- albums arrived ---------------------------------------------------- + album_posts = imgur_mock.requests_to("POST", "/3/album") + created = {form_params(entry).get("title") for entry in album_posts} + assert created == set(EXPECTED_ALBUMS), ( + f"wrong albums created.\n expected: {sorted(EXPECTED_ALBUMS)}\n" + f" actual: {sorted(created)}" + ) + + # Album 1's description is null in the fixture, and importAlbum omits the + # field entirely rather than sending an empty one. + by_title = {form_params(e).get("title"): form_params(e) for e in album_posts} + assert "description" not in by_title["Album 1"] + assert by_title["Album 2"]["description"] == "Description for Album 2" + + # -- photos arrived, byte for byte ------------------------------------- + image_posts = imgur_mock.requests_to("POST", "/3/image") + + # Map each upload back to its source fixture by content. This is the + # assertion that covers the whole data path at once: the exporter's + # HttpURLConnection download, the write into LocalTempFileStore, and the + # importer reading the stream back out. + fixtures = {p.stem: p.read_bytes() for p in FIXTURES.glob("*.jpg")} + by_content = {v: k for k, v in fixtures.items()} + assert len(by_content) == len(fixtures), "fixture images are not distinct" + + delivered = {} + for entry in image_posts: + payload = decoded_image(entry) + assert payload in by_content, ( + "an uploaded image does not match any fixture byte for byte -- the " + "temp store round trip corrupted it or served the wrong stream" + ) + name = by_content[payload] + assert name not in delivered, f"{name} was uploaded more than once" + delivered[name] = form_params(entry).get("album") + + assert delivered == EXPECTED_PHOTOS, ( + f"wrong photos, or wrong album mapping.\n expected: {EXPECTED_PHOTOS}\n" + f" actual: {delivered}" + ) + + # -- ordering ---------------------------------------------------------- + # Every album is created before the first photo that references it; the + # copier's contract is that parents are populated before children. + journal = imgur_mock.requests() + first_image = next( + i for i, e in enumerate(journal) + if e["method"] == "POST" and e["url"].startswith("/3/image") + ) + last_needed_album = max( + i for i, e in enumerate(journal) + if e["method"] == "POST" and e["url"].startswith("/3/album") + and form_params(e).get("title") in ("Album 1", "Album 2", "Album 3") + ) + assert last_needed_album < first_image, ( + "a photo was uploaded before the album it belongs to was created" + ) diff --git a/e2e/driver/test_offline_demo.py b/e2e/driver/test_offline_demo.py index 28212ac3a..4024b49ca 100644 --- a/e2e/driver/test_offline_demo.py +++ b/e2e/driver/test_offline_demo.py @@ -8,6 +8,8 @@ import re +import pytest + from dtp import decode_job_id # OfflineDemoAuthServiceExtension declares "OFFLINE-DEMO" while @@ -39,6 +41,7 @@ TRANSFER_TIMEOUT = 120.0 +@pytest.mark.offline_demo def test_api_advertises_the_credential_free_vertical(client): services = client.services_for(DATA_TYPE_ENUM) @@ -46,6 +49,7 @@ def test_api_advertises_the_credential_free_vertical(client): assert SERVICE in services["importServices"] +@pytest.mark.offline_demo def test_transfer_completes_and_delivers_the_payload(client, server_log): encoded_job_id = client.create_job( export_service=SERVICE, diff --git a/e2e/driver/wiremock.py b/e2e/driver/wiremock.py new file mode 100644 index 000000000..7cf840b19 --- /dev/null +++ b/e2e/driver/wiremock.py @@ -0,0 +1,71 @@ +"""Reader for a WireMock standalone instance's admin API. + +Nothing here is adapter-specific -- it is the generic "what did the mock +actually receive" surface. WireMock records every request it served, bodies +intact, at ``/__admin/requests``, which is what makes assertions about the +delivered payload possible at all; the offline-demo suite has to settle for +grepping a log line. +""" + +from __future__ import annotations + +import base64 +import urllib.parse + +import requests + + +class WireMock: + def __init__(self, base_url: str, timeout: float = 30.0): + self.base_url = base_url.rstrip("/") + self.timeout = timeout + + # -- admin ------------------------------------------------------------ + + def reset_requests(self) -> None: + """Clear the request journal, so one test cannot see another's traffic.""" + response = requests.delete( + f"{self.base_url}/__admin/requests", timeout=self.timeout + ) + response.raise_for_status() + + def requests(self) -> list[dict]: + """Every request served since the last reset, oldest first. + + WireMock returns them newest-first; reversing here means callers can + assert on ordering (albums before photos, say) by list position, which + is the obvious reading. + """ + response = requests.get( + f"{self.base_url}/__admin/requests", timeout=self.timeout + ) + response.raise_for_status() + entries = [entry["request"] for entry in response.json().get("requests", [])] + return list(reversed(entries)) + + # -- querying --------------------------------------------------------- + + def requests_to(self, method: str, url_prefix: str) -> list[dict]: + return [ + entry + for entry in self.requests() + if entry["method"] == method and entry["url"].startswith(url_prefix) + ] + + def received(self, method: str, url_prefix: str) -> bool: + return bool(self.requests_to(method, url_prefix)) + + +def form_params(entry: dict) -> dict[str, str]: + """Decode an ``application/x-www-form-urlencoded`` body. + + Both Imgur import calls post form bodies rather than JSON -- + ``FormBody.Builder`` in ImgurPhotosImporter -- so this is how the delivered + album titles and image bytes are read back out. + """ + return dict(urllib.parse.parse_qsl(entry.get("body", ""), keep_blank_values=True)) + + +def decoded_image(entry: dict) -> bytes: + """The raw bytes behind a POST /image call's base64 ``image`` parameter.""" + return base64.b64decode(form_params(entry)["image"]) diff --git a/e2e/mocks/imgur/__files/files/album1Photo1.jpg b/e2e/mocks/imgur/__files/files/album1Photo1.jpg new file mode 100644 index 0000000000000000000000000000000000000000..cb7150ab6cf45d4c0749850f76a9d224ab45e457 GIT binary patch literal 167 zcmex=o*fRoDvv0RE?FDF6Tf literal 0 HcmV?d00001 diff --git a/e2e/mocks/imgur/__files/files/nonAlbumPhoto2.jpg b/e2e/mocks/imgur/__files/files/nonAlbumPhoto2.jpg new file mode 100644 index 0000000000000000000000000000000000000000..2dde8a29472783881060f23113e9592d5743b12c GIT binary patch literal 169 zcmex=o*fRoDvv0RJp#DgXcg literal 0 HcmV?d00001 diff --git a/e2e/mocks/imgur/mappings/export-account-images.json b/e2e/mocks/imgur/mappings/export-account-images.json new file mode 100644 index 000000000..4fb83be24 --- /dev/null +++ b/e2e/mocks/imgur/mappings/export-account-images.json @@ -0,0 +1,111 @@ +{ + "mappings": [ + { + "name": "account images page 0", + "metadata": { + "comment": [ + "ImgurPhotosExporter.requestNonAlbumPhotos: the account-wide listing, which returns album", + "photos *and* loose ones. The exporter keeps only ids it has not already seen in an album,", + "so the album photos repeated here are expected to be filtered out -- if they are not,", + "they get imported twice and the per-image assertion catches it.", + "This page is also where the synthetic 'Non-album photos' album is emitted." + ] + }, + "request": { + "method": "GET", + "urlPath": "/3/account/me/images/0", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "album1Photo1", + "name": "album1Photo1", + "description": "First photo in Album 1", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album1Photo1.jpg" + }, + { + "id": "album1Photo2", + "name": "album1Photo2", + "description": null, + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album1Photo2.jpg" + }, + { + "id": "album2Photo1", + "name": "album2Photo1", + "description": "First photo in Album 2", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album2Photo1.jpg" + }, + { + "id": "album3Photo1", + "name": "album3Photo1", + "description": "First photo in Album 3", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album3Photo1.jpg" + }, + { + "id": "nonAlbumPhoto1", + "name": "nonAlbumPhoto1", + "description": "Loose photo one", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/nonAlbumPhoto1.jpg" + } + ] + } + } + }, + { + "name": "account images page 1", + "metadata": { + "comment": [ + "A second page on the *other* pagination axis. Its photos reference the synthetic album", + "cached during page 0, so this also exercises IdempotentImportExecutor.getCachedValue", + "across copy iterations rather than within one." + ] + }, + "request": { + "method": "GET", + "urlPath": "/3/account/me/images/1", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "nonAlbumPhoto2", + "name": "nonAlbumPhoto2", + "description": "Loose photo two", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/nonAlbumPhoto2.jpg" + } + ] + } + } + }, + { + "name": "account images page 2 (empty terminator)", + "request": { + "method": "GET", + "urlPath": "/3/account/me/images/2", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { "success": true, "status": 200, "data": [] } + } + } + ] +} diff --git a/e2e/mocks/imgur/mappings/export-album-images.json b/e2e/mocks/imgur/mappings/export-album-images.json new file mode 100644 index 000000000..206f5fbcd --- /dev/null +++ b/e2e/mocks/imgur/mappings/export-album-images.json @@ -0,0 +1,94 @@ +{ + "mappings": [ + { + "name": "album 1 images", + "metadata": { + "comment": [ + "ImgurPhotosExporter.requestPhotos, reached as a sub-resource (IdOnlyContainerResource).", + "This endpoint has no paging -- the exporter returns ResultType.END for it.", + "Fields actually read: id, name (-> PhotoModel title), description, type, link.", + "`link` is fetched during *export* with a plain HttpURLConnection and streamed into the", + "temp store, so it has to be a URL this container can reach." + ] + }, + "request": { "method": "GET", "urlPath": "/3/album/albumId1/images" }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "album1Photo1", + "name": "album1Photo1", + "title": "Album 1 photo 1", + "description": "First photo in Album 1", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album1Photo1.jpg" + }, + { + "id": "album1Photo2", + "name": "album1Photo2", + "title": null, + "description": null, + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album1Photo2.jpg" + } + ] + } + } + }, + { + "name": "album 2 images", + "request": { "method": "GET", "urlPath": "/3/album/albumId2/images" }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "album2Photo1", + "name": "album2Photo1", + "title": "Album 2 photo 1", + "description": "First photo in Album 2", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album2Photo1.jpg" + } + ] + } + } + }, + { + "name": "album 3 images", + "metadata": { + "comment": [ + "Album 3 arrives from albums page 1. Because copyHelper processes pagination *before*", + "sub-resources, this runs before page 0's defaultAlbumId sub-resource -- which is what", + "makes the exporter's non-album detection correct across paginated albums." + ] + }, + "request": { "method": "GET", "urlPath": "/3/album/albumId3/images" }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "album3Photo1", + "name": "album3Photo1", + "title": "Album 3 photo 1", + "description": "First photo in Album 3", + "type": "image/jpeg", + "link": "http://wiremock-imgur:8080/files/album3Photo1.jpg" + } + ] + } + } + } + ] +} diff --git a/e2e/mocks/imgur/mappings/export-albums.json b/e2e/mocks/imgur/mappings/export-albums.json new file mode 100644 index 000000000..f801beb4f --- /dev/null +++ b/e2e/mocks/imgur/mappings/export-albums.json @@ -0,0 +1,94 @@ +{ + "mappings": [ + { + "name": "albums page 0", + "metadata": { + "comment": [ + "ImgurPhotosExporter.requestAlbums hits /account/me/albums/{page}?perPage=10.", + "Page 0 is also where it appends the synthetic 'defaultAlbumId' sub-resource that later", + "collects non-album photos." + ] + }, + "request": { + "method": "GET", + "urlPath": "/3/account/me/albums/0", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "albumId1", + "title": "Album 1", + "description": null, + "privacy": "public", + "images_count": 2 + }, + { + "id": "albumId2", + "title": "Album 2", + "description": "Description for Album 2", + "privacy": "public", + "images_count": 1 + } + ] + } + } + }, + { + "name": "albums page 1", + "metadata": { + "comment": [ + "A second non-empty page is the whole reason Imgur was chosen over Koofr: without it the", + "copier never recurses on pagination and a green run proves nothing about it." + ] + }, + "request": { + "method": "GET", + "urlPath": "/3/account/me/albums/1", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": [ + { + "id": "albumId3", + "title": "Album 3", + "description": "Description for Album 3", + "privacy": "hidden", + "images_count": 1 + } + ] + } + } + }, + { + "name": "albums page 2 (empty terminator)", + "metadata": { + "comment": [ + "REQUIRED. The Imgur response carries no last-page flag, so the exporter infers it from", + "an empty list: `hasMore = items.size() != 0`. Without this stub WireMock 404s, the", + "exporter reads no 'data' array, and the export loops forever." + ] + }, + "request": { + "method": "GET", + "urlPath": "/3/account/me/albums/2", + "queryParameters": { "perPage": { "equalTo": "10" } } + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { "success": true, "status": 200, "data": [] } + } + } + ] +} diff --git a/e2e/mocks/imgur/mappings/import.json b/e2e/mocks/imgur/mappings/import.json new file mode 100644 index 000000000..3248413ef --- /dev/null +++ b/e2e/mocks/imgur/mappings/import.json @@ -0,0 +1,112 @@ +{ + "mappings": [ + { + "name": "create album: Album 1", + "metadata": { + "comment": [ + "ImgurPhotosImporter.importAlbum posts an x-www-form-urlencoded body and reads back", + "data.id, which IdempotentImportExecutor caches under the *original* album id. Every", + "photo in that album then looks the new id up. Returning a distinct id per album is what", + "makes that mapping assertable: a photo carrying the wrong `album` value means the cache", + "returned the wrong entry.", + "The regex tolerates both %20 and + for the space, since which one OkHttp's FormBody", + "emits is an implementation detail we should not pin a test to." + ] + }, + "request": { + "method": "POST", + "urlPath": "/3/album", + "bodyPatterns": [{ "matches": ".*title=Album(%20|\\+)1(&.*)?" }] + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": { "id": "imported-album-1", "deletehash": "hash1" } + } + } + }, + { + "name": "create album: Album 2", + "request": { + "method": "POST", + "urlPath": "/3/album", + "bodyPatterns": [{ "matches": ".*title=Album(%20|\\+)2(&.*)?" }] + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": { "id": "imported-album-2", "deletehash": "hash2" } + } + } + }, + { + "name": "create album: Album 3", + "request": { + "method": "POST", + "urlPath": "/3/album", + "bodyPatterns": [{ "matches": ".*title=Album(%20|\\+)3(&.*)?" }] + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": { "id": "imported-album-3", "deletehash": "hash3" } + } + } + }, + { + "name": "create album: Non-album photos", + "metadata": { + "comment": [ + "The synthetic album ImgurPhotosExporter invents for loose photos. Its title is hardcoded", + "in requestNonAlbumPhotos, not taken from the provider." + ] + }, + "request": { + "method": "POST", + "urlPath": "/3/album", + "bodyPatterns": [{ "matches": ".*title=Non-album(%20|\\+)photos(&.*)?" }] + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": { "id": "imported-album-default", "deletehash": "hash0" } + } + } + }, + { + "name": "upload image", + "metadata": { + "comment": [ + "A catch-all: ImgurPhotosImporter.importPhoto only checks the status code, so there is", + "nothing to vary per photo. The interesting content is in the *request* -- base64 image", + "bytes plus the resolved album id -- which the driver reads back from the journal." + ] + }, + "request": { "method": "POST", "urlPath": "/3/image" }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "success": true, + "status": 200, + "data": { + "id": "imported-image", + "link": "http://wiremock-imgur:8080/files/imported.jpg" + } + } + } + } + ] +} diff --git a/e2e/mocks/imgur/mappings/oauth-token.json b/e2e/mocks/imgur/mappings/oauth-token.json new file mode 100644 index 000000000..a9da058d6 --- /dev/null +++ b/e2e/mocks/imgur/mappings/oauth-token.json @@ -0,0 +1,28 @@ +{ + "name": "OAuth2 token exchange", + "metadata": { + "comment": [ + "The token exchange OAuth2DataGenerator performs during POST /api/transfer/{id}/generate.", + "Field names must be access_token and refresh_token: OAuth2TokenResponse maps exactly those", + "and is @JsonIgnoreProperties(ignoreUnknown = true), so the extra fields here are ignored", + "but kept because Imgur really does return them.", + "Note that WireMock rejects unknown top-level keys in a mapping, so commentary has to live", + "under metadata -- there is no comment syntax in JSON and no '//' escape hatch." + ] + }, + "request": { + "method": "POST", + "urlPath": "/oauth2/token" + }, + "response": { + "status": 200, + "headers": { "Content-Type": "application/json" }, + "jsonBody": { + "access_token": "e2e-access-token", + "refresh_token": "e2e-refresh-token", + "expires_in": 3600, + "token_type": "bearer", + "account_username": "e2e" + } + } +} diff --git a/e2e/run.sh b/e2e/run.sh index 34c97b192..bcea95810 100755 --- a/e2e/run.sh +++ b/e2e/run.sh @@ -1,36 +1,69 @@ #!/usr/bin/env bash # -# Runs a complete offline-demo -> offline-demo transfer and exits non-zero if it -# does not arrive. No provider credentials, no local JDK, no local Python. +# Runs a complete transfer per adapter and exits non-zero if any of them fails. +# No provider credentials, no local JDK, no local Python. # -# ./e2e/run.sh +# ./e2e/run.sh # every adapter +# ./e2e/run.sh imgur # just one +# ./e2e/run.sh offline-demo imgur # -# The server log lands in e2e/.logs/dtp.log afterwards, whether the run passed -# or failed. -set -euo pipefail +# Each adapter gets its own freshly started `dtp` container. That is not +# tidiness: LocalJobStore keeps jobs in private static maps and LocalTempFileStore +# keeps files on disk, and nothing clears either between jobs, so a shared +# server would make isolation a matter of luck and ordering. A cold JVM per +# adapter costs a few seconds and removes the question. +# +# Logs and mock request journals land in e2e/.logs/ afterwards, per adapter, +# whether the run passed or failed. +set -uo pipefail # deliberately not -e: every adapter runs, then we report cd "$(dirname "${BASH_SOURCE[0]}")/.." LOG_DIR="e2e/.logs" -# Capture the server log before tearing anything down, so a failure is -# diagnosable without re-running. The redirect happens here on the host, which -# is also what keeps the file owned by you rather than by root. -capture_and_clean() { - local rc=$? +# Services to start alongside `dtp`, per adapter, and the host port their mock +# admin API is published on. Adding an adapter is one line in each -- the +# driver itself stays free of provider names. +declare -A MOCKS=( [offline-demo]="" [imgur]="wiremock-imgur" ) +declare -A MOCK_PORT=( [offline-demo]="" [imgur]="18080" ) + +ALL_ADAPTERS=(offline-demo imgur) +ADAPTERS=("$@") +[[ ${#ADAPTERS[@]} -eq 0 ]] && ADAPTERS=("${ALL_ADAPTERS[@]}") + +for adapter in "${ADAPTERS[@]}"; do + if [[ -z ${MOCKS[$adapter]+set} ]]; then + echo "Unknown adapter '$adapter'. Known: ${ALL_ADAPTERS[*]}" >&2 + exit 2 + fi +done + +teardown() { + # No -v: that would also delete gradle-cache and make every run a cold build. + docker compose --profile "$1" down --remove-orphans >/dev/null 2>&1 || true +} + +# Capture before teardown, and on the host so the artifacts end up owned by you +# rather than by root. `dtp` tees to a fixed path on a shared volume and +# truncates on restart, so there is no second chance once the next adapter starts. +capture() { + local adapter=$1 mkdir -p "$LOG_DIR" - docker compose logs --no-color --no-log-prefix dtp > "$LOG_DIR/dtp.log" 2>/dev/null || true - # No -v: that would also drop gradle-cache and make every run a cold build. - docker compose down --remove-orphans >/dev/null 2>&1 || true - if [[ $rc -ne 0 ]]; then - echo - echo "FAILED. Server log: $LOG_DIR/dtp.log" + docker compose logs --no-color --no-log-prefix dtp \ + > "$LOG_DIR/dtp-$adapter.log" 2>/dev/null || true + local port=${MOCK_PORT[$adapter]} + if [[ -n $port ]]; then + # The mock's record of what actually arrived -- the assertion surface the + # offline-demo suite has to approximate by grepping a log. + curl -s "http://localhost:$port/__admin/requests" \ + > "$LOG_DIR/$adapter-requests.json" 2>/dev/null || true fi - exit "$rc" } -trap capture_and_clean EXIT -docker compose down --remove-orphans >/dev/null 2>&1 || true +cleanup_all() { for a in "${ADAPTERS[@]}"; do teardown "$a"; done; } +trap cleanup_all EXIT + +cleanup_all # Built through the `gradle` service, whose ENTRYPOINT is already ./gradlew, so # these arguments pass straight through. That single-sources the pinned Gradle @@ -43,8 +76,30 @@ echo "==> Building the demo-server jar (offline-demo, cleartext)" docker compose run --rm gradle --no-daemon \ :distributions:demo-server:shadowJar \ -PofflineData=true \ - -PencryptionScheme=cleartext + -PencryptionScheme=cleartext || exit 1 + +failed=() +for adapter in "${ADAPTERS[@]}"; do + echo + echo "==> $adapter" + teardown "$adapter" + # shellcheck disable=SC2086 # MOCKS entries are deliberately word-split + docker compose --profile "$adapter" up -d dtp ${MOCKS[$adapter]} >/dev/null || { + failed+=("$adapter"); continue + } + + # Marker names use underscores: `-m offline-demo` would parse as the + # expression `offline and (not demo)`. + E2E_MARKER="${adapter//-/_}" docker compose run --rm e2e || failed+=("$adapter") + + capture "$adapter" + teardown "$adapter" +done echo -echo "==> Running the transfer" -docker compose run --rm e2e +if [[ ${#failed[@]} -gt 0 ]]; then + echo "FAILED: ${failed[*]}" + echo "Logs: $LOG_DIR/" + exit 1 +fi +echo "All adapters passed: ${ADAPTERS[*]}"