Skip to content

Commit 8a14321

Browse files
osamahan999osamahan999
authored
Refactor GoogleMediaExporter to call PhotosInterface.getAlbum with error retrying and skipping (#1302)
* Add getGoogleAlbum to GoogleMediaExporter' : * Adding tests * Tests * Skip album failure * Added extra test cases * remove old test cases * un-capitalize photo ids
1 parent b6e3d95 commit 8a14321

File tree

2 files changed

+165
-15
lines changed

2 files changed

+165
-15
lines changed

‎extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java‎

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,11 @@ private ExportResult<MediaContainerResource> exportPhotosContainer(
204204
List<IdOnlyContainerResource> subResources = new ArrayList<>();
205205

206206
for (PhotoAlbum album : container.getAlbums()) {
207-
GoogleAlbum googleAlbum = getOrCreatePhotosInterface(authData).getAlbum(album.getId());
207+
GoogleAlbum googleAlbum = getGoogleAlbum(album.getIdempotentId(), album.getId(), album.getName(), authData);
208+
if (googleAlbum == null) {
209+
continue;
210+
}
211+
208212
albumBuilder.add(new MediaAlbum(googleAlbum.getId(), googleAlbum.getTitle(), null));
209213
// Adding subresources tells the framework to recall export to get all the photos
210214
subResources.add(new IdOnlyContainerResource(googleAlbum.getId()));
@@ -238,7 +242,11 @@ private ExportResult<MediaContainerResource> exportMediaContainer(
238242
List<IdOnlyContainerResource> subResources = new ArrayList<>();
239243

240244
for (MediaAlbum album : container.getAlbums()) {
241-
GoogleAlbum googleAlbum = getOrCreatePhotosInterface(authData).getAlbum(album.getId());
245+
GoogleAlbum googleAlbum = getGoogleAlbum(album.getIdempotentId(), album.getId(), album.getName(), authData);
246+
if (googleAlbum == null) {
247+
continue;
248+
}
249+
242250
albumBuilder.add(new MediaAlbum(googleAlbum.getId(), googleAlbum.getTitle(), null));
243251
// Adding subresources tells the framework to recall export to get all the photos
244252
subResources.add(new IdOnlyContainerResource(googleAlbum.getId()));
@@ -453,6 +461,27 @@ private MediaContainerResource convertMediaListToResource(
453461
return new MediaContainerResource(null /*albums*/, photos, videos);
454462
}
455463

464+
@VisibleForTesting
465+
@Nullable
466+
GoogleAlbum getGoogleAlbum(String albumIdempotentId, String albumId, String albumName,
467+
TokensAndUrlAuthData authData) throws IOException, InvalidTokenException,
468+
PermissionDeniedException {
469+
if (retryingExecutor == null || !enableRetrying) {
470+
return getOrCreatePhotosInterface(authData).getAlbum(albumId);
471+
}
472+
473+
try {
474+
GoogleAlbum googleAlbum = retryingExecutor.executeAndSwallowIOExceptions(
475+
albumIdempotentId, albumName,
476+
() -> getOrCreatePhotosInterface(authData).getAlbum(albumId)
477+
);
478+
return googleAlbum;
479+
} catch (Exception e) {
480+
monitor.info(() -> format("Retry exception encountered while fetching an album: %s", e));
481+
}
482+
return null;
483+
}
484+
456485
//TODO(#1308): Make the retrying methods API & adaptor agnostic
457486
@VisibleForTesting
458487
@Nullable

‎extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporterTest.java‎

Lines changed: 134 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import java.util.List;
3939
import java.util.Optional;
4040
import java.util.UUID;
41+
import java.util.concurrent.Callable;
4142
import java.util.stream.Collectors;
4243
import org.datatransferproject.api.launcher.Monitor;
4344
import org.datatransferproject.datatransfer.google.common.GoogleCredentialFactory;
@@ -74,6 +75,11 @@
7475
import org.datatransferproject.types.transfer.retry.UniformRetryStrategy;
7576
import org.junit.jupiter.api.BeforeEach;
7677
import org.junit.jupiter.api.Test;
78+
import org.datatransferproject.spi.transfer.idempotentexecutor.RetryingInMemoryIdempotentImportExecutor;
79+
import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData;
80+
import org.datatransferproject.types.transfer.errors.ErrorDetail;
81+
import org.datatransferproject.types.transfer.retry.RetryStrategyLibrary;
82+
import org.datatransferproject.types.transfer.retry.UniformRetryStrategy;
7783
import org.mockito.ArgumentCaptor;
7884

7985
public class GoogleMediaExporterTest {
@@ -82,11 +88,10 @@ public class GoogleMediaExporterTest {
8288
private static String ALBUM_ID = "GoogleAlbum id";
8389
private static String ALBUM_TOKEN = "some-upstream-generated-album-token";
8490
private static String MEDIA_TOKEN = "some-upstream-generated-media-token";
85-
8691
private static long RETRY_INTERVAL_MILLIS = 100L;
8792
private static int RETRY_MAX_ATTEMPTS = 1;
88-
8993
private static UUID uuid = UUID.randomUUID();
94+
9095
private TokensAndUrlAuthData authData;
9196
private RetryingInMemoryIdempotentImportExecutor retryingExecutor;
9297
private GoogleMediaExporter googleMediaExporter;
@@ -112,15 +117,22 @@ public void setup()
112117
authData = mock(TokensAndUrlAuthData.class);
113118

114119
retryingExecutor = new RetryingInMemoryIdempotentImportExecutor(monitor,
115-
new RetryStrategyLibrary(ImmutableList.of(), new UniformRetryStrategy(RETRY_MAX_ATTEMPTS, RETRY_INTERVAL_MILLIS, "identifier"))
120+
new RetryStrategyLibrary(
121+
ImmutableList.of(),
122+
new UniformRetryStrategy(RETRY_MAX_ATTEMPTS, RETRY_INTERVAL_MILLIS, "identifier")
123+
)
116124
);
117125

118126
googleMediaExporter =
119127
new GoogleMediaExporter(
120128
credentialFactory, jobStore, GsonFactory.getDefaultInstance(), monitor, photosInterface);
121129

122130
retryingGoogleMediaExporter = new GoogleMediaExporter(
123-
credentialFactory, jobStore, GsonFactory.getDefaultInstance(), monitor, photosInterface,
131+
credentialFactory,
132+
jobStore,
133+
GsonFactory.getDefaultInstance(),
134+
monitor,
135+
photosInterface,
124136
retryingExecutor,
125137
true);
126138

@@ -384,17 +396,17 @@ public void testGetGoogleMediaItemSucceeds() throws IOException, InvalidTokenExc
384396
}
385397

386398
@Test
387-
public void testExportPhotosContainerRetrying() throws IOException, InvalidTokenException, PermissionDeniedException, UploadErrorException {
388-
String PHOTO_ID_TO_FAIL_1 = "photo3";
389-
String PHOTO_ID_TO_FAIL_2 = "photo5";
399+
public void testExportPhotosContainer_photosRetrying() throws IOException, InvalidTokenException, PermissionDeniedException, UploadErrorException {
400+
String photoIdToFail1 = "photo3";
401+
String photoIdToFail2 = "photo5";
390402

391403
ImmutableList<PhotoAlbum> albums = ImmutableList.of();
392404
ImmutableList<PhotoModel> photos = ImmutableList.of(
393405
setUpSinglePhotoModel("", "photo1"),
394406
setUpSinglePhotoModel("", "photo2"),
395-
setUpSinglePhotoModel("", PHOTO_ID_TO_FAIL_1),
407+
setUpSinglePhotoModel("", photoIdToFail1),
396408
setUpSinglePhotoModel("", "photo4"),
397-
setUpSinglePhotoModel("", PHOTO_ID_TO_FAIL_2),
409+
setUpSinglePhotoModel("", photoIdToFail2),
398410
setUpSinglePhotoModel("", "photo6")
399411
);
400412

@@ -406,11 +418,11 @@ public void testExportPhotosContainerRetrying() throws IOException, InvalidToken
406418

407419

408420
// For the photo_id_to_fail photos, throw an exception.
409-
when(photosInterface.getMediaItem(PHOTO_ID_TO_FAIL_1)).thenThrow(IOException.class);
410-
when(photosInterface.getMediaItem(PHOTO_ID_TO_FAIL_2)).thenThrow(IOException.class);
421+
when(photosInterface.getMediaItem(photoIdToFail1)).thenThrow(IOException.class);
422+
when(photosInterface.getMediaItem(photoIdToFail2)).thenThrow(IOException.class);
411423
// For all other photos, return a media item.
412424
for (PhotoModel photoModel: photos) {
413-
if (photoModel.getDataId().equals(PHOTO_ID_TO_FAIL_1) || photoModel.getDataId().equals(PHOTO_ID_TO_FAIL_2)) {
425+
if (photoModel.getDataId().equals(photoIdToFail1) || photoModel.getDataId().equals(photoIdToFail2)) {
414426
continue;
415427
}
416428
when(photosInterface.getMediaItem(photoModel.getDataId())).thenReturn(
@@ -427,7 +439,7 @@ public void testExportPhotosContainerRetrying() throws IOException, InvalidToken
427439
photos.stream().map(
428440
x -> x.getDataId()
429441
).filter(
430-
dataId -> !(dataId.equals(PHOTO_ID_TO_FAIL_1) || dataId.equals(PHOTO_ID_TO_FAIL_2))
442+
dataId -> !(dataId.equals(photoIdToFail1) || dataId.equals(photoIdToFail2))
431443
).collect(
432444
Collectors.toList()
433445
)
@@ -460,6 +472,99 @@ public void testGetGoogleMediaItemFailed() throws IOException, InvalidTokenExcep
460472
assertThat(end - start).isLessThan(RETRY_INTERVAL_MILLIS * RETRY_MAX_ATTEMPTS);
461473
}
462474

475+
@Test
476+
public void testExportAlbums_failureInterruptsTransfer() throws Exception {
477+
String albumIdToFail1 = "albumid3";
478+
String albumIdToFail2 = "albumid5";
479+
480+
ImmutableList<PhotoModel> photos = ImmutableList.of();
481+
ImmutableList<PhotoAlbum> albums = ImmutableList.of(
482+
setUpSinglePhotoAlbum("albumid1", "album 1`", ""),
483+
setUpSinglePhotoAlbum("albumid2", "album 2", ""),
484+
setUpSinglePhotoAlbum(albumIdToFail1, "album 3", ""),
485+
setUpSinglePhotoAlbum("albumid4", "album 4", ""),
486+
setUpSinglePhotoAlbum(albumIdToFail2, "album 5", ""),
487+
setUpSinglePhotoAlbum("albumid6", "album 6", "")
488+
);
489+
490+
PhotosContainerResource container = new PhotosContainerResource(albums, photos);
491+
ExportInformation exportInfo = new ExportInformation(null, container);
492+
493+
MediaMetadata photoMediaMetadata = new MediaMetadata();
494+
photoMediaMetadata.setPhoto(new Photo());
495+
496+
// For the album_id_to_fail albums, throw an exception.
497+
when(photosInterface.getAlbum(albumIdToFail1)).thenThrow(IOException.class);
498+
when(photosInterface.getAlbum(albumIdToFail2)).thenThrow(IOException.class);
499+
// For all other albums, return a GoogleMediaAlbum.
500+
for (PhotoAlbum photoAlbum: albums) {
501+
if (photoAlbum.getId().equals(albumIdToFail1) || photoAlbum.getId().equals(albumIdToFail2)) {
502+
continue;
503+
}
504+
when(photosInterface.getAlbum(photoAlbum.getId())).thenReturn(
505+
setUpGoogleAlbum(Optional.of(photoAlbum.getId()), Optional.of(photoAlbum.getName()))
506+
);
507+
}
508+
509+
assertThrows(IOException.class, () -> googleMediaExporter.export(
510+
uuid, authData, Optional.of(exportInfo)
511+
));
512+
}
513+
514+
@Test
515+
public void testExportAlbums_retryingSkipsFailures() throws Exception {
516+
String albumIdToFail1 = "albumid3";
517+
String albumIdToFail2 = "albumid5";
518+
519+
ImmutableList<PhotoModel> photos = ImmutableList.of();
520+
ImmutableList<PhotoAlbum> albums = ImmutableList.of(
521+
setUpSinglePhotoAlbum("albumid1", "album 1`", ""),
522+
setUpSinglePhotoAlbum("albumid2", "album 2", ""),
523+
setUpSinglePhotoAlbum(albumIdToFail1, "album 3", ""),
524+
setUpSinglePhotoAlbum("albumid4", "album 4", ""),
525+
setUpSinglePhotoAlbum(albumIdToFail2, "album 5", ""),
526+
setUpSinglePhotoAlbum("albumid6", "album 6", "")
527+
);
528+
529+
PhotosContainerResource container = new PhotosContainerResource(albums, photos);
530+
ExportInformation exportInfo = new ExportInformation(null, container);
531+
532+
MediaMetadata photoMediaMetadata = new MediaMetadata();
533+
photoMediaMetadata.setPhoto(new Photo());
534+
535+
// For the album_id_to_fail albums, throw an exception.
536+
when(photosInterface.getAlbum(albumIdToFail1)).thenThrow(IOException.class);
537+
when(photosInterface.getAlbum(albumIdToFail2)).thenThrow(IOException.class);
538+
// For all other albums, return a GoogleMediaAlbum.
539+
for (PhotoAlbum photoAlbum: albums) {
540+
if (photoAlbum.getId().equals(albumIdToFail1) || photoAlbum.getId().equals(albumIdToFail2)) {
541+
continue;
542+
}
543+
when(photosInterface.getAlbum(photoAlbum.getId())).thenReturn(
544+
setUpGoogleAlbum(Optional.of(photoAlbum.getId()), Optional.of(photoAlbum.getName()))
545+
);
546+
}
547+
548+
ExportResult<MediaContainerResource> result = retryingGoogleMediaExporter.export(
549+
uuid, authData, Optional.of(exportInfo)
550+
);
551+
552+
assertThat(
553+
result.getExportedData().getAlbums().stream().map(x -> x.getId()).collect(Collectors.toList())
554+
).isEqualTo(
555+
albums.stream().map(
556+
x -> x.getId()
557+
).filter(
558+
id -> !(id.equals(albumIdToFail1) || id.equals(albumIdToFail2))
559+
).collect(
560+
Collectors.toList()
561+
)
562+
);
563+
assertThat(result.getExportedData().getAlbums().size()).isEqualTo(albums.size() - 2);
564+
assertThat(retryingExecutor.getErrors().size()).isEqualTo(2);
565+
assertThat(retryingExecutor.getErrors().stream().findFirst().toString().contains("IOException")).isTrue();
566+
}
567+
463568
/** Sets up a response with a single album, containing a single photo */
464569
private void setUpSingleAlbum() {
465570
GoogleAlbum albumEntry = new GoogleAlbum();
@@ -469,6 +574,22 @@ private void setUpSingleAlbum() {
469574
when(albumListResponse.getAlbums()).thenReturn(new GoogleAlbum[] {albumEntry});
470575
}
471576

577+
private GoogleAlbum setUpGoogleAlbum(Optional<String> albumId, Optional<String> albumTitle) {
578+
GoogleAlbum album = new GoogleAlbum();
579+
if (albumId.isPresent()) {
580+
album.setId(albumId.get());
581+
}
582+
if (albumTitle.isPresent()) {
583+
album.setTitle(albumTitle.get());
584+
}
585+
586+
return album;
587+
}
588+
589+
private static PhotoAlbum setUpSinglePhotoAlbum(String albumId, String albumName, String description) {
590+
return new PhotoAlbum(albumId, albumName, description);
591+
}
592+
472593
private static PhotoModel setUpSinglePhotoModel(String albumId, String dataId) {
473594
return new PhotoModel("Title", "fetchableUrl", "description", "photo", dataId, albumId, false);
474595
}

0 commit comments

Comments
 (0)