Conversation
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<A, T extends DataModel> 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.
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.
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.
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:<jar> 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.
alexeyqu
added this pull request to stack #1517
September 14, 2026 23:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an
Imgur → Imgursuite to the e2e harness, driving the real, unmodifiedImgurPhotosExporterandImgurPhotosImporteragainst a WireMock stand-in forapi.imgur.com.Until now the harness only ran
offline-demo → offline-demo: a fixed string exported by a ten-line exporter and printed by aSystem.out.printlnimporter. That proves the machinery — job creation, auth, worker claim, copier, importer — and nothing about a data path. A green run now additionally covers a real OAuth2 token exchange, pagination on two independent axes, sub-resource recursion, an image byte round trip throughLocalTempFileStore, and the original→new album id mappingIdempotentImportExecutormaintains across copy iterations.Stack
feat/offline-demo-exporter)test/e2e-offline-demo-compose)feat/imgur-configurable-urls)Retarget down the stack as the parents land.
One cold JVM per adapter
run.shbecomes a loop, and each adapter gets its own freshly starteddtpcontainer. This is not tidiness:LocalJobStorekeeps jobs inprivate staticmaps andLocalTempFileStorekeeps files on disk, and nothing clears either between jobs. A shared server would make isolation a matter of luck and ordering.dtptherefore joins per-adapter compose profiles;gradleande2edeliberately stay out of every profile sodocker compose run --rm gradlekeeps working verbatim, sincedocker-build.ymlandDocumentation/Developer.mdboth hardcode it.The loop runs every adapter even when one fails, and captures per-adapter server logs and mock request journals to
e2e/.logs/before teardown. Cost is one extra JVM boot per adapter, measured at ~17s.Each adapter also passes standalone, which is the check that the isolation is real.
How the mock URL reaches the adapter
Config resolution is classpath-only — there is no environment-variable override anywhere in the chain — and
ConfigUtils/TransferServiceConfigboth use the singulargetResourceAsStream, so first match per filename wins. Thedtpservice now runsrather than
java -jar, which ignores-cpentirely.e2e/configholds onlyconfig/imgur.yaml, a file the jar does not ship, so it is purely additive and inert for every other adapter. Shadowing a file the jar does ship (deezer.yaml,flickr.yaml,synology.yaml) would replace it wholesale rather than merge, silently dropping settings likeperUserRateLimit— noted in the README.Two assertions that were initially vacuous
Both were written, ran green, and covered nothing. They are recorded in
e2e/README.mdbecause they are the same failure mode as the0 error(s)trap #1516 documented: an assertion that is true on a green run, but would also be true if the thing it claims to check never happened.The byte-exactness check had a circular oracle. It compares each uploaded image against the fixture files — which are the files WireMock serves. Corrupting a fixture moves the subject and the oracle together, so the negative check passed when it should have failed. The assertion is still worth having: it proves each distinct fixture arrived exactly once, unmodified, which catches temp-store cross-talk, truncation and duplicate imports. What was wrong was the negative check, which has to break the pipeline — make the mock serve the wrong file — not the fixture. It then fails correctly with
album2Photo1 was uploaded more than once.The pagination check asserted the wrong page. It checked that page 1 was requested. But
ImgurPhotosExporterderives "there is more" fromitems.size() != 0, so it always requests page N+1 after any non-empty page — meaning page 1 is fetched even when page 0 was the last page with data. That would pass on single-page fixtures, covering none of the pagination Imgur was chosen for. It now asserts page 2, the first request that can only happen if page 1 had content.Notes for reviewers
SEVERElines. On a same-service transferWorkerModuleresolves one extension instance and callsinitialize()on it repeatedly, andImgurTransferExtensionlogs each repeat atSEVERE— four lines on a successful run. The fail-fast heuristic requires the job id on the line and these carry none, so it does not trip, but test(e2e): add black-box offline-demo transfer harness #1516's "a clean run emits zero SEVERE lines" assumption is now only true per-job. Downgrading that log line is a candidate one-wordfix:, deliberately not bundled here.mkdirany missingmappings//__files/in a bind mount, leaving root-owned directories in your checkout.X-DTP-*header assertions are not here — those belong to the generic importer, not Imgur, and stay with the schema work.Verification
No workflow runs on a stacked PR (
pull_request: branches: [master]matches the base), so this was verified by hand:Four negative checks, none committed:
album2Photo1 was uploaded more than once.album pagination never got past the first page of data.baseUrlpointed at a dead port → fails in seconds on theSEVEREline with the server's ownCopyException.CopyException, not the infinite loop expected: WireMock's 404 body is not parseable JSON, sorequestDatathrows before the loop can run away.