Skip to content

Commit 5177064

Browse files
authored
feat(amazon-photos): harden Photos importer and add shared import helper (#1512)
## Summary Refactors the Amazon Photos importer for reliability and testability. Introduces typed API error handling, structured failure classification, platform-level retry integration, and multipart upload with robust completion polling. Client lifecycle and shared import utilities are extracted into a new `AmazonImportHelper`. > **Note:** This PR has been reviewed internally already. This is a behavior-preserving refactor for existing flows, plus hardening for transient failures and large uploads. ## What's changed ### Error handling - New `AmazonPhotosApiException` carrying HTTP status + service `errorCode`. The client parses the error body and throws the typed exception, logging only `errorCode`/`message` (never the raw response body). - `UploadErrorCodes` centralizes the client-facing codes we classify on: - Duplicates (`DuplicatesConflictError`) are skipped idempotently. - Storage-quota conditions (`InsufficientStorage`, `NoActiveSubscriptionFound`) map to a terminal `DestinationMemoryFullException`. ### Retry integration - Wires the platform `RetryingIdempotentImportExecutor` (opt-in via the `enableRetrying` setting) so transient failures are retried per the host `RetryStrategyLibrary`, consistent with other providers. ### Multipart upload - Initiate → upload parts → complete, followed by completion polling that: - fails fast on a definitive 4xx, - retries transient 5xx/429/malformed responses until the budget is exhausted, and - guards against a missing status field. - `RetryUtils` backoff is exponential-with-jitter; explicit connect/read/write timeouts on the OkHttp client. ### Shared `AmazonImportHelper` - Owns per-job client creation keyed by `jobId` (bounded access-order LRU), resolving endpoints once per job. Client build + endpoint resolution happen outside the lock so one job's latency can't stall others. - Provides MD5/streamed download (untrusted `dataId` is sanitized for the temp-file prefix) and the shared error classification used by the importer. - Endpoint resolution now requires `uploadServiceUrl` (dropped the `contentUrl` fallback). ## Testing Unit tests added/updated across: - **Helper** — per-job client isolation, error classification. - **Client** — multipart poll success / fail-fast / transient-retry / budget-exhausted, error extraction, configured timeouts, lazy endpoint resolution. - **Importer** — retry-executor selection, quota → terminal failure, duplicate skip, upload happy paths. Completed transfers and validated. ## Notes - `okhttp`/`mockwebserver` now use the shared `${okHttpVersion}` instead of a hardcoded version.
1 parent 2e24062 commit 5177064

18 files changed

Lines changed: 1803 additions & 161 deletions

extensions/data-transfer/portability-data-transfer-amazon/build.gradle

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ dependencies {
3030
implementation "com.fasterxml.jackson.core:jackson-annotations:${jacksonVersion}"
3131
implementation "com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}"
3232
implementation "com.google.guava:guava:${guavaVersion}"
33-
implementation "com.squareup.okhttp3:okhttp:4.9.0"
33+
implementation "com.squareup.okhttp3:okhttp:${okHttpVersion}"
3434

35-
testImplementation "com.squareup.okhttp3:mockwebserver:4.9.0"
35+
testImplementation "com.squareup.okhttp3:mockwebserver:${okHttpVersion}"
3636
}

extensions/data-transfer/portability-data-transfer-amazon/src/main/java/org/datatransferproject/transfer/amazon/AmazonTransferExtension.java

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import org.datatransferproject.spi.cloud.storage.AppCredentialStore;
2323
import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore;
2424
import org.datatransferproject.spi.transfer.extension.TransferExtension;
25+
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor;
26+
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutorExtension;
2527
import org.datatransferproject.spi.transfer.provider.Exporter;
2628
import org.datatransferproject.spi.transfer.provider.Importer;
2729
import org.datatransferproject.transfer.amazon.photos.AmazonPhotosImporter;
@@ -34,7 +36,7 @@ public class AmazonTransferExtension implements TransferExtension {
3436

3537
private static final String SERVICE_ID = "Amazon";
3638

37-
private AmazonPhotosImporter importer;
39+
private AmazonPhotosImporter photosImporter;
3840
private volatile boolean initialized = false;
3941

4042
@Override
@@ -51,8 +53,10 @@ public String getServiceId() {
5153
@Override
5254
public Importer<?, ?> getImporter(DataVertical transferDataType) {
5355
Preconditions.checkArgument(initialized, "Extension not initialized");
54-
Preconditions.checkArgument(transferDataType == DataVertical.PHOTOS);
55-
return importer;
56+
if (transferDataType == DataVertical.PHOTOS) {
57+
return photosImporter;
58+
}
59+
throw new IllegalArgumentException("Unsupported data type: " + transferDataType);
5660
}
5761

5862
@Override
@@ -70,9 +74,15 @@ public synchronized void initialize(ExtensionContext context) {
7074
return;
7175
}
7276

73-
importer = new AmazonPhotosImporter(
77+
IdempotentImportExecutor retryingIdempotentExecutor =
78+
context.getService(IdempotentImportExecutorExtension.class)
79+
.getRetryingIdempotentImportExecutor(context);
80+
boolean enableRetrying = context.getSetting("enableRetrying", false);
81+
82+
photosImporter = new AmazonPhotosImporter(
7483
monitor, appCredentials.getKey(), appCredentials.getSecret(),
75-
context.getService(TemporaryPerJobDataStore.class));
84+
context.getService(TemporaryPerJobDataStore.class),
85+
retryingIdempotentExecutor, enableRetrying);
7686

7787
initialized = true;
7888
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Copyright 2026 The Data Transfer Project Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.datatransferproject.transfer.amazon.photos;
18+
19+
import org.datatransferproject.spi.cloud.connection.ConnectionProvider;
20+
import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore;
21+
import org.datatransferproject.api.launcher.Monitor;
22+
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor;
23+
import org.datatransferproject.types.common.DownloadableItem;
24+
import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData;
25+
26+
import java.io.File;
27+
import java.io.IOException;
28+
import java.io.InputStream;
29+
import java.security.DigestInputStream;
30+
import java.security.MessageDigest;
31+
import java.util.LinkedHashMap;
32+
import java.util.Map;
33+
import java.util.UUID;
34+
35+
/**
36+
* Shared helper for Amazon Photos/Videos importers.
37+
* Holds common collaborators and provides reusable import operations.
38+
*/
39+
class AmazonImportHelper {
40+
41+
static final String IMPORTED_SUFFIX = " - Imported from ";
42+
43+
private final TemporaryPerJobDataStore dataStore;
44+
private final ConnectionProvider connectionProvider;
45+
private final String clientId;
46+
private final String clientSecret;
47+
private final Monitor monitor;
48+
// Test seam: when set it is always returned in place of a real client.
49+
private final AmazonPhotosInterface injectedClient;
50+
// Safeguard to avoid sharing a client across jobs: keying by jobId keeps each job's credentials
51+
// separate. Bounded, access-order LRU so a long-lived (multi-job) process can't accumulate
52+
// clients/tokens without limit; an evicted job just rebuilds its client on its next chunk.
53+
// Guarded by getOrCreateClient (synchronized).
54+
private static final int MAX_CACHED_CLIENTS = 1000;
55+
private final Map<UUID, AmazonPhotosInterface> clientsByJob =
56+
new LinkedHashMap<UUID, AmazonPhotosInterface>(16, 0.75f, true) {
57+
@Override
58+
protected boolean removeEldestEntry(Map.Entry<UUID, AmazonPhotosInterface> eldest) {
59+
return size() > MAX_CACHED_CLIENTS;
60+
}
61+
};
62+
63+
/** Classification/download-only helper (no client provisioning); used where no client is built. */
64+
AmazonImportHelper(TemporaryPerJobDataStore dataStore) {
65+
this(dataStore, null, null, null, null);
66+
}
67+
68+
/** Production helper that builds a real Amazon client per job from the given app credentials. */
69+
AmazonImportHelper(TemporaryPerJobDataStore dataStore, String clientId, String clientSecret,
70+
Monitor monitor) {
71+
this(dataStore, clientId, clientSecret, monitor, null);
72+
}
73+
74+
/** Test helper: {@code injectedClient} is always returned by {@link #getOrCreateClient}. */
75+
AmazonImportHelper(TemporaryPerJobDataStore dataStore, AmazonPhotosInterface injectedClient) {
76+
this(dataStore, null, null, null, injectedClient);
77+
}
78+
79+
private AmazonImportHelper(TemporaryPerJobDataStore dataStore, String clientId,
80+
String clientSecret, Monitor monitor,
81+
AmazonPhotosInterface injectedClient) {
82+
this.dataStore = dataStore;
83+
this.connectionProvider = new ConnectionProvider(dataStore);
84+
this.clientId = clientId;
85+
this.clientSecret = clientSecret;
86+
this.monitor = monitor;
87+
this.injectedClient = injectedClient;
88+
}
89+
90+
/**
91+
* Returns the client for this job, creating and caching it (and resolving endpoints) on first
92+
* use. Injected clients are returned as-is; per-job caching keeps each job's credentials
93+
* isolated and avoids re-resolving endpoints on every chunk of the same job.
94+
*/
95+
AmazonPhotosInterface getOrCreateClient(UUID jobId, TokensAndUrlAuthData authData)
96+
throws IOException {
97+
if (injectedClient != null) {
98+
return injectedClient;
99+
}
100+
synchronized (this) {
101+
AmazonPhotosInterface existing = clientsByJob.get(jobId);
102+
if (existing != null) {
103+
return existing;
104+
}
105+
}
106+
// Build and resolve endpoints outside the lock: resolveEndpoints() is a network call, so
107+
// holding the shared lock across it would let one job's latency stall getOrCreateClient for
108+
// every other concurrent job. Two concurrent first-chunks of the same new job may each build
109+
// once; putIfAbsent dedupes storage and the loser is discarded.
110+
AmazonPhotosInterface created = createClient(authData);
111+
created.resolveEndpoints();
112+
synchronized (this) {
113+
AmazonPhotosInterface race = clientsByJob.putIfAbsent(jobId, created);
114+
return race != null ? race : created;
115+
}
116+
}
117+
118+
// Visible-for-testing seam: endpoint resolution otherwise requires a live network call, so
119+
// tests override this to inject a fake client.
120+
AmazonPhotosInterface createClient(TokensAndUrlAuthData authData) {
121+
return new AmazonPhotosClient(
122+
AmazonPhotosClient.createDefaultHttpClient(),
123+
authData.getAccessToken(), authData.getRefreshToken(), clientId, clientSecret, monitor);
124+
}
125+
126+
/** Creates a new MD5 MessageDigest instance. */
127+
MessageDigest newMd5Digest() {
128+
return Md5Utils.newDigest();
129+
}
130+
131+
/** Converts a byte array to its lowercase hex string representation. */
132+
String toHexString(byte[] bytes) {
133+
return Md5Utils.toHexString(bytes);
134+
}
135+
136+
/** Whether the API error indicates the item already exists (duplicate). */
137+
boolean isDuplicate(AmazonPhotosApiException e) {
138+
return e.isErrorCode(UploadErrorCodes.DUPLICATES_CONFLICT_ERROR);
139+
}
140+
141+
/** Whether the API error indicates the destination storage quota is exceeded. */
142+
boolean isStorageQuotaExceeded(AmazonPhotosApiException e) {
143+
return e.isErrorCode(UploadErrorCodes.INSUFFICIENT_STORAGE)
144+
|| e.isErrorCode(UploadErrorCodes.NO_ACTIVE_SUBSCRIPTION_FOUND);
145+
}
146+
147+
/**
148+
* Downloads a content item to a temp file, computing MD5 in a single pass.
149+
*
150+
* <p>The provider-supplied {@code dataId} is untrusted filesystem input, so path separators are
151+
* stripped before using it as the temp-file prefix — otherwise {@code Files.createTempFile}
152+
* rejects it with an IllegalArgumentException. Only the local prefix is affected; the content
153+
* sent to Amazon is unchanged.
154+
*/
155+
File downloadToTempFile(UUID jobId, DownloadableItem item, String dataId,
156+
MessageDigest md5) throws IOException {
157+
String prefix = dataId.replaceAll("[/\\\\]", "_");
158+
try (InputStream raw = connectionProvider.getInputStreamForItem(jobId, item).getStream();
159+
DigestInputStream dis = new DigestInputStream(raw, md5)) {
160+
return dataStore.getTempFileFromInputStream(dis, prefix, ".tmp");
161+
}
162+
}
163+
164+
/**
165+
* Resolves the target album ID from the executor cache.
166+
* If albumId is provided but not cached (album creation failed), throws to prevent
167+
* silent data loss — photos/videos should not be uploaded without their album.
168+
*/
169+
String resolveTargetAlbumId(String albumId, IdempotentImportExecutor executor)
170+
throws Exception {
171+
if (albumId == null) {
172+
return null;
173+
}
174+
// This will throw if album creation failed, which marks the item as failed too,
175+
// preventing silent loss of album organization.
176+
return executor.getCachedValue(albumId);
177+
}
178+
179+
/** Removes temp data from the job store. */
180+
void cleanupTempData(UUID jobId, String fetchableUrl) throws IOException {
181+
dataStore.removeData(jobId, fetchableUrl);
182+
}
183+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/*
2+
* Copyright 2026 The Data Transfer Project Authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.datatransferproject.transfer.amazon.photos;
18+
19+
import java.io.IOException;
20+
21+
/**
22+
* Typed exception for non-success Amazon Photos / Upload Service API responses.
23+
*
24+
* <p>Carries the HTTP status and the service {@code errorCode} (see {@link UploadErrorCodes})
25+
* so callers can classify failures on a structured field rather than by substring-matching a
26+
* raw response body. Extends {@link IOException} so it flows through the existing DTP
27+
* import error handling (e.g. {@code executeAndSwallowIOExceptions}).
28+
*/
29+
public class AmazonPhotosApiException extends IOException {
30+
31+
private final int httpStatus;
32+
private final String errorCode;
33+
34+
/**
35+
* @param httpStatus the HTTP status code of the response
36+
* @param errorCode the service errorCode from the response body, or {@code null} if absent
37+
* @param message a human-readable message (must not embed sensitive raw response detail)
38+
*/
39+
public AmazonPhotosApiException(int httpStatus, String errorCode, String message) {
40+
super(message);
41+
this.httpStatus = httpStatus;
42+
this.errorCode = errorCode;
43+
}
44+
45+
public int getHttpStatus() {
46+
return httpStatus;
47+
}
48+
49+
/** The service errorCode, or {@code null} if the response carried none. */
50+
public String getErrorCode() {
51+
return errorCode;
52+
}
53+
54+
/** Whether this error's {@code errorCode} equals the given code. */
55+
public boolean isErrorCode(String code) {
56+
return code != null && code.equals(errorCode);
57+
}
58+
}

0 commit comments

Comments
 (0)