Skip to content

Commit 8dc02fe

Browse files
yy9669jzacsh
yy9669
and
jzacsh
authored
add Retry and skip logic in AppleMediaImporter (#1329)
Co-authored-by: Jonathan Zacsh <j@zac.sh>
1 parent e800b0e commit 8dc02fe

File tree

10 files changed

+424
-100
lines changed

10 files changed

+424
-100
lines changed

‎extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/AppleTransferExtension.java‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@
3333
import org.datatransferproject.datatransfer.apple.photos.AppleVideosImporter;
3434
import org.datatransferproject.spi.cloud.storage.AppCredentialStore;
3535
import org.datatransferproject.spi.transfer.extension.TransferExtension;
36+
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor;
37+
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutorExtension;
3638
import org.datatransferproject.spi.transfer.provider.Exporter;
3739
import org.datatransferproject.spi.transfer.provider.Importer;
3840
import org.datatransferproject.types.common.models.DataVertical;
@@ -86,11 +88,15 @@ public void initialize(ExtensionContext context) {
8688
return;
8789
}
8890

91+
IdempotentImportExecutor idempotentImportExecutor = context.getService(
92+
IdempotentImportExecutorExtension.class).getRetryingIdempotentImportExecutor(context);
93+
boolean enableRetrying = context.getSetting("enableRetrying", false);
94+
8995
importerMap =
9096
ImmutableMap.<DataVertical, Importer>builder()
9197
.put(PHOTOS, new ApplePhotosImporter(appCredentials, monitor))
9298
.put(VIDEOS, new AppleVideosImporter(appCredentials, monitor))
93-
.put(MEDIA, new AppleMediaImporter(appCredentials, monitor))
99+
.put(MEDIA, new AppleMediaImporter(appCredentials, monitor, idempotentImportExecutor, enableRetrying))
94100
.build();
95101

96102
exporterMap = ImmutableMap.<DataVertical, Exporter>builder().build();

‎extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/constants/ApplePhotosConstants.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,5 @@ public class ApplePhotosConstants {
3030
public static final Long maxMediaTransferByteSize = 50_000_000_000L;
3131
public static final String BYTES_KEY = "bytes";
3232
public static final String COUNT_KEY = "count";
33+
public static final String APPLE_PHOTOS_IMPORT_ERROR_PREFIX = "APPLE PHOTOS IMPORT:";
3334
}

‎extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/constants/AuditKeys.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public enum AuditKeys {
6565
transactionId,
6666
updatedTimeInMs,
6767
uri,
68+
uploadUrl,
6869
value,
6970
error,
7071
}

‎extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporter.java‎

Lines changed: 88 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,32 @@
1818

1919
import com.google.common.annotations.VisibleForTesting;
2020
import com.google.common.collect.ImmutableMap;
21+
22+
import java.io.IOException;
23+
import java.util.Collection;
24+
import java.util.Collections;
25+
import java.util.HashMap;
2126
import java.util.Map;
2227
import java.util.UUID;
2328
import org.datatransferproject.api.launcher.Monitor;
2429
import org.datatransferproject.datatransfer.apple.AppleInterfaceFactory;
2530
import org.datatransferproject.datatransfer.apple.constants.ApplePhotosConstants;
2631
import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor;
32+
import org.datatransferproject.spi.transfer.idempotentexecutor.RetryingInMemoryIdempotentImportExecutor;
2733
import org.datatransferproject.spi.transfer.provider.ImportResult;
2834
import org.datatransferproject.spi.transfer.provider.Importer;
2935
import org.datatransferproject.transfer.JobMetadata;
3036
import org.datatransferproject.types.common.models.DataVertical;
3137
import org.datatransferproject.types.common.models.media.MediaContainerResource;
3238
import org.datatransferproject.types.transfer.auth.AppCredentials;
3339
import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData;
40+
import org.datatransferproject.types.transfer.errors.ErrorDetail;
3441
import org.jetbrains.annotations.NotNull;
3542

43+
import javax.annotation.Nullable;
44+
45+
import static java.lang.String.format;
46+
3647
/**
3748
* An Apple importer to import the Photos and Videos into Apple iCloud-photos.
3849
*/
@@ -41,20 +52,26 @@ public class AppleMediaImporter implements Importer<TokensAndUrlAuthData, MediaC
4152
private final String exportingService;
4253
private final Monitor monitor;
4354
private final AppleInterfaceFactory factory;
55+
private IdempotentImportExecutor retryingIdempotentExecutor;
56+
private Boolean enableRetrying;
4457

4558
public AppleMediaImporter(
46-
@NotNull final AppCredentials appCredentials, @NotNull final Monitor monitor) {
47-
this(appCredentials, JobMetadata.getExportService(), monitor, new AppleInterfaceFactory());
59+
@NotNull final AppCredentials appCredentials, @NotNull final Monitor monitor, @Nullable IdempotentImportExecutor retryingIdempotentExecutor, boolean enableRetrying) {
60+
this(appCredentials, JobMetadata.getExportService(), monitor, new AppleInterfaceFactory(), retryingIdempotentExecutor, enableRetrying);
4861
}
4962

5063
@VisibleForTesting
5164
AppleMediaImporter(
5265
@NotNull final AppCredentials appCredentials, @NotNull String exportingService,
53-
@NotNull final Monitor monitor, @NotNull AppleInterfaceFactory factory) {
66+
@NotNull final Monitor monitor, @NotNull AppleInterfaceFactory factory,
67+
@Nullable IdempotentImportExecutor retryingIdempotentExecutor,
68+
boolean enableRetrying) {
5469
this.appCredentials = appCredentials;
5570
this.exportingService = exportingService;
5671
this.monitor = monitor;
5772
this.factory = factory;
73+
this.retryingIdempotentExecutor = retryingIdempotentExecutor;
74+
this.enableRetrying = enableRetrying;
5875
}
5976
@Override
6077
public ImportResult importItem(
@@ -67,38 +84,77 @@ public ImportResult importItem(
6784
return ImportResult.OK;
6885
}
6986

87+
IdempotentImportExecutor executor =
88+
(retryingIdempotentExecutor != null && enableRetrying) ? retryingIdempotentExecutor : idempotentExecutor;
89+
7090
AppleMediaInterface mediaInterface = factory
7191
.getOrCreateMediaInterface(jobId, authData, appCredentials, exportingService, monitor);
7292

73-
final int albumCount =
74-
mediaInterface.importAlbums(
75-
jobId,
76-
idempotentExecutor,
77-
data.getAlbums(),
78-
DataVertical.MEDIA.getDataType());
79-
final Map<String, Long> importPhotosMap =
80-
mediaInterface.importAllMedia(
81-
jobId,
82-
idempotentExecutor,
83-
data.getPhotos(),
84-
DataVertical.MEDIA.getDataType());
85-
final Map<String, Long> importVideosResult =
86-
mediaInterface.importAllMedia(
87-
jobId,
88-
idempotentExecutor,
89-
data.getVideos(),
90-
DataVertical.MEDIA.getDataType());
91-
92-
final Map<String, Integer> counts =
93-
new ImmutableMap.Builder<String, Integer>()
94-
.put(MediaContainerResource.ALBUMS_COUNT_DATA_NAME, albumCount)
95-
.put(
96-
MediaContainerResource.PHOTOS_COUNT_DATA_NAME,
97-
importPhotosMap.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue())
98-
.put(
99-
MediaContainerResource.VIDEOS_COUNT_DATA_NAME,
100-
importVideosResult.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue())
101-
.build();
93+
final String retryId = format("AppleMediaImporter_%s_%s", jobId, UUID.randomUUID());
94+
95+
final Map<String, Long> importPhotosMap = new HashMap<>();
96+
final Map<String, Long> importVideosResult = new HashMap<>();
97+
final Map<String, Integer> counts = new HashMap<>();
98+
99+
// lower stack retry logic in data copier will not handle the skippable error case, that's why we want to build retry logic in importer itself.
100+
101+
// executor can either be a RetryingInMemoryIdempotentImportExecutor or an InMemoryIdempotentImportExecutor
102+
// RetryingInMemoryIdempotentImportExecutor will return null for skippable error, throw for the others
103+
// InMemoryIdempotentImportExecutor will throw every error (the callableImporter will throw it as well if we don't throw it here)
104+
105+
// if executor is a RetryingInMemoryIdempotentImportExecutor, then it will be different from the idempotentExecutor in the param, which will check for recent errors in lower stack (CallableImporter),
106+
// So if the error in executor is skippable, we need to clean the errors in idempotentExecutor to make sure they will not be thrown in the lower stack.
107+
108+
executor.executeOrThrowException(retryId, retryId, () -> {
109+
importPhotosMap.clear();
110+
importVideosResult.clear();
111+
counts.clear();
112+
idempotentExecutor.resetRecentErrors();
113+
114+
final int albumCount =
115+
mediaInterface.importAlbums(
116+
jobId,
117+
idempotentExecutor,
118+
data.getAlbums(),
119+
DataVertical.MEDIA.getDataType());
120+
importPhotosMap.putAll(
121+
mediaInterface.importAllMedia(
122+
jobId,
123+
idempotentExecutor,
124+
data.getPhotos(),
125+
DataVertical.MEDIA.getDataType()));
126+
importVideosResult.putAll(
127+
mediaInterface.importAllMedia(
128+
jobId,
129+
idempotentExecutor,
130+
data.getVideos(),
131+
DataVertical.MEDIA.getDataType()));
132+
counts.putAll(
133+
new ImmutableMap.Builder<String, Integer>()
134+
.put(MediaContainerResource.ALBUMS_COUNT_DATA_NAME, albumCount)
135+
.put(
136+
MediaContainerResource.PHOTOS_COUNT_DATA_NAME,
137+
importPhotosMap.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue())
138+
.put(
139+
MediaContainerResource.VIDEOS_COUNT_DATA_NAME,
140+
importVideosResult.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue())
141+
.build());
142+
143+
Collection<ErrorDetail> errors = idempotentExecutor.getRecentErrors();
144+
if (!errors.isEmpty() && executor instanceof RetryingInMemoryIdempotentImportExecutor) { // throw the error for retryExecutor to retry, only include the actual error message, but not the stack traces
145+
throw new IOException(errors.iterator().hasNext() ? errors.iterator().next().exception().lines().findFirst().get() : ApplePhotosConstants.APPLE_PHOTOS_IMPORT_ERROR_PREFIX + " Unknown Error");
146+
}
147+
return true;
148+
});
149+
150+
// if retryingExecutor thinks the errors are skippable, we need to clean the errors in idempotentExecutor to make sure they will not be thrown in the lower stack.
151+
// (Notice that lower stack CallableImporter does not have skip logic)
152+
if(executor instanceof RetryingInMemoryIdempotentImportExecutor) {
153+
Collection<ErrorDetail> recentErrorsFromRetryingExecutor = executor.getRecentErrors();
154+
if (!recentErrorsFromRetryingExecutor.isEmpty() && recentErrorsFromRetryingExecutor.iterator().hasNext() && recentErrorsFromRetryingExecutor.stream().allMatch(errorDetail -> errorDetail.canSkip())) {
155+
idempotentExecutor.resetRecentErrors();
156+
}
157+
}
102158

103159
return ImportResult.OK
104160
.copyWithBytes(

‎extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaInterface.java‎

Lines changed: 47 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,11 @@ public Map<String, String> uploadContent(
192192
totalSize += data.length;
193193

194194
if (totalSize > ApplePhotosConstants.maxMediaTransferByteSize) {
195-
monitor.severe(
196-
() -> "file too large to import to Apple: ",
197-
AuditKeys.dataId, dataId,
198-
AuditKeys.downloadURL, downloadURL,
199-
authorizeUploadResponse.getUploadUrl());
200195
uploadClient.completeUpload();
201-
throw new AppleContentException("file too large to import to Apple");
196+
throw new AppleContentException(getApplePhotosImportThrowingMessage("file too large to import to Apple", ImmutableMap.of(
197+
AuditKeys.dataId, dataId,
198+
AuditKeys.downloadURL, downloadURL,
199+
AuditKeys.uploadUrl, authorizeUploadResponse.getUploadUrl())));
202200
}
203201

204202
uploadClient.uploadBytes(data);
@@ -283,19 +281,19 @@ private void convertAndThrowException(@NotNull final IOException e, @NotNull fin
283281

284282
switch (con.getResponseCode()) {
285283
case SC_UNAUTHORIZED:
286-
throw new UnconfirmedUserException("Unauthorized iCloud User", e);
284+
throw new UnconfirmedUserException(getApplePhotosImportThrowingMessage("Unauthorized iCloud User"), e);
287285
case SC_PRECONDITION_FAILED:
288-
throw new PermissionDeniedException("Permission Denied", e);
286+
throw new PermissionDeniedException(getApplePhotosImportThrowingMessage("Permission Denied"), e);
289287
case SC_NOT_FOUND:
290-
throw new DestinationNotFoundException("iCloud Photos Library not found", e);
288+
throw new DestinationNotFoundException(getApplePhotosImportThrowingMessage("iCloud Photos Library not found"), e);
291289
case SC_INSUFFICIENT_STORAGE:
292-
throw new DestinationMemoryFullException("iCloud Storage is full", e);
290+
throw new DestinationMemoryFullException(getApplePhotosImportThrowingMessage("iCloud Storage is full"), e);
293291
case SC_SERVICE_UNAVAILABLE:
294-
throw new IOException("DTP import service unavailable", e);
292+
throw new IOException(getApplePhotosImportThrowingMessage("DTP import service unavailable"), e);
295293
case SC_BAD_REQUEST:
296-
throw new IOException("Bad request sent to iCloud Photos import api", e);
294+
throw new IOException(getApplePhotosImportThrowingMessage("Bad request sent to iCloud Photos import api"), e);
297295
case SC_INTERNAL_SERVER_ERROR:
298-
throw new IOException("Internal server error in iCloud Photos service", e);
296+
throw new IOException(getApplePhotosImportThrowingMessage("Internal server error in iCloud Photos service"), e);
299297
case SC_OK:
300298
break;
301299
default:
@@ -323,7 +321,7 @@ public byte[] makePhotosServicePostRequest(
323321
return responseData;
324322
}
325323

326-
private void refreshTokens() throws IOException, InvalidTokenException {
324+
private void refreshTokens() throws InvalidTokenException {
327325
final String refreshToken = authData.getRefreshToken();
328326
final String refreshUrlString = authData.getTokenServerEncodedUrl();
329327
final String clientId = appCredentials.getKey();
@@ -351,9 +349,7 @@ private void refreshTokens() throws IOException, InvalidTokenException {
351349

352350
} catch (ParseException | IOException | CopyExceptionWithFailureReason e) {
353351

354-
monitor.debug(() -> "Failed to refresh token", e);
355-
356-
throw new InvalidTokenException("Unable to refresh token", e);
352+
throw new InvalidTokenException(getApplePhotosImportThrowingMessage("Unable to refresh token"), e);
357353
}
358354
}
359355

@@ -442,16 +438,11 @@ public int importAlbums(
442438
mediaAlbum.getId(),
443439
mediaAlbum.getName(),
444440
() -> {
445-
monitor.severe(
446-
() -> "Error importing album: ",
447-
AuditKeys.jobId, jobId,
448-
AuditKeys.albumId, mediaAlbum.getId(),
449-
AuditKeys.errorCode, newPhotoAlbumResponse.getStatus().getCode());
450-
451-
throw new IOException(
452-
String.format(
453-
"Failed to create album, error code: %d",
454-
newPhotoAlbumResponse.getStatus().getCode()));
441+
throw new IOException(getApplePhotosImportThrowingMessage("Fail to create album",
442+
ImmutableMap.of(
443+
AuditKeys.errorCode, String.valueOf(newPhotoAlbumResponse.getStatus().getCode()),
444+
AuditKeys.jobId, jobId.toString(),
445+
AuditKeys.albumId, mediaAlbum.getId())));
455446
});
456447
}
457448
}
@@ -530,16 +521,13 @@ Map<String, Long> importMediaBatch(
530521
downloadableFile.getIdempotentId(),
531522
downloadableFile.getName(),
532523
() -> {
533-
monitor.severe(
534-
() -> "Fail to get upload url: ",
535-
AuditKeys.jobId, jobId,
536-
AuditKeys.dataId, getDataId(downloadableFile),
537-
AuditKeys.albumId, downloadableFile.getFolderId(),
538-
AuditKeys.statusCode, authorizeUploadResponse.getStatus().getCode());
539524
throw new IOException(
540-
String.format(
541-
"Fail to get upload url, error code: %d",
542-
authorizeUploadResponse.getStatus().getCode()));
525+
getApplePhotosImportThrowingMessage(
526+
"Fail to get upload url", ImmutableMap.of(
527+
AuditKeys.errorCode, String.valueOf(authorizeUploadResponse.getStatus().getCode()),
528+
AuditKeys.jobId, jobId.toString(),
529+
AuditKeys.dataId, getDataId(downloadableFile),
530+
AuditKeys.albumId, downloadableFile.getFolderId())));
543531
});
544532
}
545533
}
@@ -564,12 +552,11 @@ AuditKeys.dataId, getDataId(downloadableFile),
564552
downloadableFile.getIdempotentId(),
565553
downloadableFile.getName(),
566554
() -> {
567-
monitor.severe(
568-
() -> "Fail to upload content: ",
569-
AuditKeys.jobId, jobId,
570-
AuditKeys.dataId, getDataId(downloadableFile),
571-
AuditKeys.albumId, downloadableFile.getFolderId());
572-
throw new IOException("Fail to upload content");
555+
throw new IOException(getApplePhotosImportThrowingMessage("Fail to upload content", ImmutableMap.of(
556+
AuditKeys.jobId, jobId.toString(),
557+
AuditKeys.dataId, getDataId(downloadableFile),
558+
AuditKeys.albumId, downloadableFile.getFolderId()
559+
)));
573560
});
574561
}
575562
}
@@ -641,16 +628,13 @@ AuditKeys.dataId, getDataId(downloadableFile),
641628
downloadableFile.getIdempotentId(),
642629
downloadableFile.getName(),
643630
() -> {
644-
monitor.severe(
645-
() -> "Fail to create media: ",
646-
AuditKeys.jobId, jobId,
647-
AuditKeys.dataId, getDataId(downloadableFile),
648-
AuditKeys.albumId, downloadableFile.getFolderId(),
649-
AuditKeys.statusCode, newMediaResponse.getStatus().getCode());
650631
throw new IOException(
651-
String.format(
652-
"Fail to create media, error code: %d",
653-
newMediaResponse.getStatus().getCode()));
632+
getApplePhotosImportThrowingMessage(
633+
"Fail to create media", ImmutableMap.of(
634+
AuditKeys.errorCode, String.valueOf(newMediaResponse.getStatus().getCode()),
635+
AuditKeys.jobId, jobId.toString(),
636+
AuditKeys.dataId, getDataId(downloadableFile),
637+
AuditKeys.albumId, downloadableFile.getFolderId())));
654638
});
655639
}
656640
}
@@ -696,4 +680,16 @@ private static String getDescription(DownloadableFile downloadableFile) {
696680
}
697681
return null;
698682
}
683+
684+
public static String getApplePhotosImportThrowingMessage(final String cause) {
685+
return getApplePhotosImportThrowingMessage(cause, ImmutableMap.of());
686+
}
687+
688+
public static String getApplePhotosImportThrowingMessage(final String cause, final ImmutableMap<AuditKeys, String> keyValuePairs) {
689+
String finalLogMessage = String.format("%s " + cause, ApplePhotosConstants.APPLE_PHOTOS_IMPORT_ERROR_PREFIX);
690+
for (AuditKeys key: keyValuePairs.keySet()){
691+
finalLogMessage = String.format("%s, %s:%s", finalLogMessage, key.name(), keyValuePairs.get(key));
692+
}
693+
return finalLogMessage;
694+
}
699695
}

0 commit comments

Comments
 (0)