From bc44fa782d0d1dbd10510f06e535dd594fb54282 Mon Sep 17 00:00:00 2001 From: osamahan999 <35288172+osamahan999@users.noreply.github.com> Date: Tue, 14 Nov 2023 10:29:20 -0800 Subject: [PATCH 1/8] Add the RetryingInMemoryIdempotentExecutor to the GoogleMediaExporter constructor (#1294) * Add a RetryingIdempotentExecutor to GoogleMediaExporter * Pass the retrying executor into the MediaExporter --- .../google/GoogleTransferExtension.java | 2 +- .../google/media/GoogleMediaExporter.java | 51 +++++++++++++++++-- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java index 769a9836f..41cfb7953 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java @@ -151,7 +151,7 @@ public void initialize(ExtensionContext context) { PHOTOS, new GooglePhotosExporter(credentialFactory, jobStore, jsonFactory, monitor)); exporterBuilder.put(VIDEOS, new GoogleVideosExporter(credentialFactory, jsonFactory)); exporterBuilder.put( - MEDIA, new GoogleMediaExporter(credentialFactory, jobStore, jsonFactory, monitor)); + MEDIA, new GoogleMediaExporter(credentialFactory, jobStore, jsonFactory, monitor, idempotentImportExecutor, enableRetrying)); exporterBuilder.put(MUSIC, new GoogleMusicExporter(credentialFactory, jsonFactory, monitor)); exporterMap = exporterBuilder.build(); diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java index a10cd43f4..ab11d9690 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java @@ -39,6 +39,7 @@ import org.datatransferproject.datatransfer.google.mediaModels.MediaItemSearchResponse; import org.datatransferproject.datatransfer.google.photos.GooglePhotosInterface; import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore; +import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor; import org.datatransferproject.spi.transfer.provider.ExportResult; import org.datatransferproject.spi.transfer.provider.ExportResult.ResultType; import org.datatransferproject.spi.transfer.provider.Exporter; @@ -69,16 +70,39 @@ public class GoogleMediaExporter implements Exporter Date: Wed, 15 Nov 2023 16:12:28 -0800 Subject: [PATCH 2/8] fix video count in import result of AppleMediaImporter (#1296) --- .../apple/photos/AppleMediaImporter.java | 8 ++-- .../apple/photos/AppleMediaImporterTest.java | 10 ++--- .../apple/photos/ApplePhotosImporterTest.java | 40 +++++++++---------- .../apple/photos/AppleVideosImporterTest.java | 18 ++++----- 4 files changed, 38 insertions(+), 38 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporter.java b/extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporter.java index a3e45dc5b..866ac5512 100644 --- a/extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporter.java +++ b/extensions/data-transfer/portability-data-transfer-apple/src/main/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporter.java @@ -76,7 +76,7 @@ public ImportResult importItem( idempotentExecutor, data.getAlbums(), DataVertical.MEDIA.getDataType()); - final Map importResultMap = + final Map importPhotosMap = mediaInterface.importAllMedia( jobId, idempotentExecutor, @@ -94,15 +94,15 @@ public ImportResult importItem( .put(MediaContainerResource.ALBUMS_COUNT_DATA_NAME, albumCount) .put( MediaContainerResource.PHOTOS_COUNT_DATA_NAME, - importResultMap.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue()) + importPhotosMap.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue()) .put( MediaContainerResource.VIDEOS_COUNT_DATA_NAME, - importResultMap.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue()) + importVideosResult.getOrDefault(ApplePhotosConstants.COUNT_KEY, 0L).intValue()) .build(); return ImportResult.OK .copyWithBytes( - importResultMap.getOrDefault(ApplePhotosConstants.BYTES_KEY, 0L) + importPhotosMap.getOrDefault(ApplePhotosConstants.BYTES_KEY, 0L) + importVideosResult.getOrDefault(ApplePhotosConstants.BYTES_KEY, 0L)) .copyWithCounts(counts); } diff --git a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporterTest.java b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporterTest.java index dff8164ef..4dd029428 100644 --- a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleMediaImporterTest.java @@ -113,19 +113,19 @@ public void importPhotosVideosAndAlbums() throws Exception { verify(mediaInterface, times(2)).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); + assertThat(importResult.getCounts().isPresent()).isTrue(); assertThat( - importResult.getCounts().get().get(PhotosContainerResource.ALBUMS_COUNT_DATA_NAME) == 1); + importResult.getCounts().get().get(PhotosContainerResource.ALBUMS_COUNT_DATA_NAME) == 1).isTrue(); assertThat( importResult.getCounts().get().get(PhotosContainerResource.PHOTOS_COUNT_DATA_NAME) - == photoCount); + == photoCount).isTrue(); assertThat( importResult.getCounts().get().get(VideosContainerResource.VIDEOS_COUNT_DATA_NAME) - == videoCount); + == videoCount).isTrue(); assertThat( importResult.getBytes().get() - == photoCount * PHOTOS_FILE_SIZE + videoCount * VIDEOS_FILE_SIZE); + == photoCount * PHOTOS_FILE_SIZE + videoCount * VIDEOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = mediaAlbums.stream() diff --git a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/ApplePhotosImporterTest.java b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/ApplePhotosImporterTest.java index a82bea4be..b460db744 100644 --- a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/ApplePhotosImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/ApplePhotosImporterTest.java @@ -94,8 +94,8 @@ public void importAlbums() throws Exception { .collect(Collectors.toList())); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount).isTrue(); final Map expectedKnownValue = photoAlbums.stream() .collect( @@ -152,8 +152,8 @@ public void importAlbumWithoutName() throws Exception { .collect(Collectors.toList())); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount).isTrue(); final Map expectedKnownValue = photoAlbums.stream() .collect( @@ -196,8 +196,8 @@ public void importAlbumsMultipleBatches() throws Exception { .collect(Collectors.toList())); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount).isTrue(); final Map expectedKnownValue = photoAlbums.stream() .collect( @@ -242,9 +242,9 @@ public void importAlbumsWithFailure() throws Exception { .collect(Collectors.toList())); // check the result - assertThat(importResult.getCounts().isPresent()); + assertThat(importResult.getCounts().isPresent()).isTrue(); assertThat( - importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount - errorCount); + importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == albumCount - errorCount).isTrue(); final Map expectedKnownValue = photoAlbums.stream() .filter( @@ -303,10 +303,10 @@ public void importSinglePhoto() throws Exception { verify(mediaInterface).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0); - assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == photoCount); - assertThat(importResult.getBytes().get() == photoCount * PHOTOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0).isTrue(); + assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == photoCount).isTrue(); + assertThat(importResult.getBytes().get() == photoCount * PHOTOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = photos.stream() @@ -358,10 +358,10 @@ public void importPhotosMultipleBatches() throws Exception { verify(mediaInterface, times(2)).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0); - assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == photoCount); - assertThat(importResult.getBytes().get() == photoCount * PHOTOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0).isTrue(); + assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == photoCount).isTrue(); + assertThat(importResult.getBytes().get() == photoCount * PHOTOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = photos.stream() @@ -422,10 +422,10 @@ public void importPhotosWithFailure() throws Exception { verify(mediaInterface, times(2)).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0); - assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == successCount); - assertThat(importResult.getBytes().get() == successCount * PHOTOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(ALBUMS_COUNT_DATA_NAME) == 0).isTrue(); + assertThat(importResult.getCounts().get().get(PHOTOS_COUNT_DATA_NAME) == successCount).isTrue(); + assertThat(importResult.getBytes().get() == successCount * PHOTOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = photos.stream() diff --git a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleVideosImporterTest.java b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleVideosImporterTest.java index 63729d580..27cc70e82 100644 --- a/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleVideosImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-apple/src/test/java/org/datatransferproject/datatransfer/apple/photos/AppleVideosImporterTest.java @@ -64,9 +64,9 @@ public void importSingleVideo() throws Exception { verify(mediaInterface).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == videoCount); - assertThat(importResult.getBytes().get() == videoCount * VIDEOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == videoCount).isTrue(); + assertThat(importResult.getBytes().get() == videoCount * VIDEOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = videos.stream() @@ -117,9 +117,9 @@ public void importVideosMultipleBatches() throws Exception { verify(mediaInterface, times(2)).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == videoCount); - assertThat(importResult.getBytes().get() == videoCount * VIDEOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == videoCount).isTrue(); + assertThat(importResult.getBytes().get() == videoCount * VIDEOS_FILE_SIZE).isTrue(); } @Test @@ -172,9 +172,9 @@ public void importVideosWithFailure() throws Exception { verify(mediaInterface, times(2)).createMedia(anyString(), anyString(), anyList()); // check the result - assertThat(importResult.getCounts().isPresent()); - assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == videoCount); - assertThat(importResult.getBytes().get() == successCount * VIDEOS_FILE_SIZE); + assertThat(importResult.getCounts().isPresent()).isTrue(); + assertThat(importResult.getCounts().get().get(VIDEOS_COUNT_DATA_NAME) == successCount).isTrue(); + assertThat(importResult.getBytes().get() == successCount * VIDEOS_FILE_SIZE).isTrue(); final Map expectedKnownValue = videos.stream() From ab4fcb8d56cb7582b576aa14e54e245c7cf36170 Mon Sep 17 00:00:00 2001 From: Alex Kulikov <86608180+alexeyqu-fb@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:12:14 +0000 Subject: [PATCH 3/8] chore: bump to v1.0.2 (#1293) --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 9ce5ae446..9e89d35cd 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,5 +1,5 @@ projectGroup=org.datatransferproject -projectVersion=1.0.1-SNAPSHOT +projectVersion=1.0.2-SNAPSHOT annotationApiVersion=1.2 autoValueVersion=1.9 commonsLangVersion=3.4 From 0a901ad6a9010f1a7a30818add97b1e7ce0875ae Mon Sep 17 00:00:00 2001 From: osamahan999 <35288172+osamahan999@users.noreply.github.com> Date: Fri, 17 Nov 2023 13:44:12 -0800 Subject: [PATCH 4/8] Refactored GoogleMediaImporter to include the GoogleVideosImporter implementation of PhotosLibraryClient, current implementation always null (#1301) * Fix GoogleMediaImporer having a null PhotosLibraryClient --- .../google/media/GoogleMediaImporter.java | 30 ++++++++------ .../google/media/GoogleMediaImporterTest.java | 40 ++++++++++++------- 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporter.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporter.java index 752550c25..37ba3d16d 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporter.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporter.java @@ -19,14 +19,10 @@ import static org.datatransferproject.datatransfer.google.photos.GooglePhotosInterface.ERROR_HASH_MISMATCH; import static org.datatransferproject.datatransfer.google.videos.GoogleVideosInterface.uploadBatchOfVideos; -import com.google.api.gax.core.FixedCredentialsProvider; import com.google.api.client.auth.oauth2.Credential; import com.google.api.client.json.JsonFactory; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.base.Strings; -import com.google.common.collect.Iterators; -import com.google.common.collect.UnmodifiableIterator; import com.google.photos.library.v1.PhotosLibraryClient; import com.google.rpc.Code; import java.io.IOException; @@ -38,8 +34,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.UUID; -import java.util.function.Function; -import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.datatransferproject.api.launcher.Monitor; import org.datatransferproject.datatransfer.google.common.GoogleCredentialFactory; @@ -53,6 +47,7 @@ import org.datatransferproject.datatransfer.google.mediaModels.Status; import org.datatransferproject.datatransfer.google.photos.GooglePhotosInterface; import org.datatransferproject.datatransfer.google.photos.PhotoResult; +import org.datatransferproject.datatransfer.google.videos.GoogleVideosInterface; import org.datatransferproject.spi.cloud.connection.ConnectionProvider; import org.datatransferproject.spi.cloud.storage.JobStore; import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore; @@ -67,13 +62,12 @@ import org.datatransferproject.spi.transfer.types.InvalidTokenException; import org.datatransferproject.spi.transfer.types.PermissionDeniedException; import org.datatransferproject.spi.transfer.types.UploadErrorException; -import org.datatransferproject.types.common.DownloadableItem; import org.datatransferproject.types.common.ImportableItem; import org.datatransferproject.types.common.models.media.MediaContainerResource; import org.datatransferproject.types.common.models.media.MediaAlbum; -import org.datatransferproject.types.common.models.photos.PhotoAlbum; import org.datatransferproject.types.common.models.photos.PhotoModel; import org.datatransferproject.types.common.models.videos.VideoModel; +import org.datatransferproject.types.transfer.auth.AppCredentials; import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData; public class GoogleMediaImporter @@ -94,7 +88,8 @@ public class GoogleMediaImporter // singleton appears to have been left behind during PR #882 private final GooglePhotosInterface photosInterface; private final HashMap multilingualStrings = new HashMap<>(); - private final PhotosLibraryClient photosLibraryClient; + private final Map photosLibraryClientMap; + private final AppCredentials appCredentials; // We partition into groups of 49 as 50 is the maximum number of items that can be created // in one call. (We use 49 to avoid potential off by one errors) @@ -106,6 +101,7 @@ public GoogleMediaImporter( JobStore jobStore, TemporaryPerJobDataStore dataStore, JsonFactory jsonFactory, + AppCredentials appCredentials, Monitor monitor, double writesPerSecond) { this( @@ -114,8 +110,9 @@ public GoogleMediaImporter( dataStore, jsonFactory, new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, null, /*photosInterface*/ - null, /*photosLibraryClient*/ new ConnectionProvider(jobStore), monitor, writesPerSecond); @@ -128,8 +125,9 @@ public GoogleMediaImporter( TemporaryPerJobDataStore dataStore, JsonFactory jsonFactory, Map photosInterfacesMap, + Map photosLibraryClientMap, + AppCredentials appCredentials, GooglePhotosInterface photosInterface, - PhotosLibraryClient photosLibraryClient, ConnectionProvider connectionProvider, Monitor monitor, double writesPerSecond) { @@ -138,8 +136,9 @@ public GoogleMediaImporter( this.dataStore = dataStore; this.jsonFactory = jsonFactory; this.photosInterfacesMap = photosInterfacesMap; + this.photosLibraryClientMap = photosLibraryClientMap; + this.appCredentials = appCredentials; this.photosInterface = photosInterface; - this.photosLibraryClient = photosLibraryClient; this.connectionProvider = connectionProvider; this.monitor = monitor; this.writesPerSecond = writesPerSecond; @@ -156,6 +155,11 @@ public ImportResult importItem( // Nothing to do return ImportResult.OK; } + + if (!photosLibraryClientMap.containsKey(jobId)) { + photosLibraryClientMap.put(jobId, GoogleVideosInterface.buildPhotosLibraryClient(appCredentials, authData)); + } + // WARNING: this should be constructed PER request so as to not conflate job IDs or auth data // across processes. That is: do NOT cache an instance of this object across your requests, say // by storing the instance as a member of your adapter's Importer or Exporter implementations. @@ -331,7 +335,7 @@ private long importVideosBatch( jobId, batch, dataStore, - photosLibraryClient, + photosLibraryClientMap.get(jobId), executor, connectionProvider, monitor); diff --git a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporterTest.java b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporterTest.java index c0e1ed56a..4938248bf 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaImporterTest.java @@ -32,7 +32,7 @@ import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; -import java.net.HttpURLConnection; +import java.util.HashMap; import java.util.UUID; import org.datatransferproject.api.launcher.Monitor; import org.datatransferproject.cloud.local.LocalJobStore; @@ -57,6 +57,7 @@ import org.datatransferproject.spi.transfer.types.UploadErrorException; import org.datatransferproject.types.common.models.media.MediaAlbum; import org.datatransferproject.types.common.models.photos.PhotoModel; +import org.datatransferproject.types.transfer.auth.AppCredentials; import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData; import org.datatransferproject.types.transfer.errors.ErrorDetail; import org.hamcrest.CoreMatchers; @@ -81,11 +82,13 @@ public class GoogleMediaImporterTest { private IdempotentImportExecutor executor; private ConnectionProvider connectionProvider; private Monitor monitor; + private AppCredentials appCredentials; @Before public void setUp() throws Exception { googlePhotosInterface = mock(GooglePhotosInterface.class); monitor = mock(Monitor.class); + appCredentials = mock(AppCredentials.class); // Initialize the executor with an old album ID -> new album ID mapping. executor = new InMemoryIdempotentImportExecutor(monitor); @@ -110,9 +113,10 @@ public void setUp() throws Exception { jobStore, dataStore, null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -302,9 +306,10 @@ public void importAlbumWithITString() jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -340,9 +345,10 @@ public void retrieveAlbumStringOnlyOnce() jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -380,9 +386,10 @@ public void importPhotoInTempStore() throws Exception { jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -432,9 +439,10 @@ public void importPhotoInTempStoreFailure() throws Exception { jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -480,9 +488,10 @@ public void importPhotoFailedToFindAlbum() throws Exception { jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); @@ -525,9 +534,10 @@ public void importPhotoCreatePhotosOtherException() throws Exception { jobStore, mock(TemporaryPerJobDataStore.class), null, /*jsonFactory*/ - null, /*photosInterfacesMap*/ + new HashMap<>(), /*photosInterfacesMap*/ + new HashMap<>(), /*photosLibraryClientMap*/ + appCredentials, googlePhotosInterface, - photosLibraryClient, connectionProvider, monitor, 1.0 /*writesPerSecond*/); From 3e9c6432f2cfd9f0c01780c16f0a76abd66f7334 Mon Sep 17 00:00:00 2001 From: Siyu Gao Date: Tue, 21 Nov 2023 10:06:19 -0800 Subject: [PATCH 5/8] Refactor artist and release in GoogleTrack as reference (#1303) * Refactor artist and release in GoogleTrack as reference * Fix google music importer/exporter unit tests --- .../google/music/GoogleMusicExporter.java | 4 +-- .../google/music/GoogleMusicImporter.java | 4 +-- .../google/musicModels/GoogleTrack.java | 32 +++++++++---------- .../google/music/GoogleMusicExporterTest.java | 2 +- .../google/music/GoogleMusicImporterTest.java | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporter.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporter.java index 84e106c9d..bb3e858d5 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporter.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporter.java @@ -270,7 +270,7 @@ private int getTokenPrefixLength(String token) { private MusicPlaylistItem convertPlaylistItem( String playlistId, GooglePlaylistItem googlePlaylistItem) throws ParseException { GoogleTrack track = googlePlaylistItem.getTrack(); - GoogleRelease release = track.getRelease(); + GoogleRelease release = track.getReleaseReference(); return new MusicPlaylistItem( new MusicRecording( track.getIsrc(), @@ -280,7 +280,7 @@ private MusicPlaylistItem convertPlaylistItem( release.getIcpn(), release.getTitle(), createMusicGroups(release.getArtists())), - createMusicGroups(track.getArtists()), + createMusicGroups(track.getArtistReferences()), "EXPLICIT_TYPE_EXPLICIT".equals(track.getExplicitType())), playlistId, googlePlaylistItem.getOrder()); diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporter.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporter.java index a03946f62..92d08c4e0 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporter.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporter.java @@ -268,7 +268,7 @@ private ImportPlaylistItemRequest buildImportPlaylistItemRequest( googleTrack.setIsrc(playlistItem.getTrack().getIsrcCode()); googleTrack.setTitle(playlistItem.getTrack().getTitle()); - googleTrack.setArtists(getArtists(playlistItem.getTrack().getByArtists())); + googleTrack.setArtistReferences(getArtists(playlistItem.getTrack().getByArtists())); googleTrack.setDuration( Durations.toString(Durations.fromMillis(playlistItem.getTrack().getDurationMillis()))); if (playlistItem.getTrack().getIsExplicit()) { @@ -276,7 +276,7 @@ private ImportPlaylistItemRequest buildImportPlaylistItemRequest( } else { googleTrack.setExplicitType("EXPLICIT_TYPE_NOT_EXPLICIT"); } - googleTrack.setRelease(googleRelease); + googleTrack.setReleaseReference(googleRelease); googlePlaylistItem.setTrack(googleTrack); googlePlaylistItem.setOrder(playlistItem.getOrder()); diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/musicModels/GoogleTrack.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/musicModels/GoogleTrack.java index 782c88b50..e0af110ef 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/musicModels/GoogleTrack.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/musicModels/GoogleTrack.java @@ -30,14 +30,14 @@ public class GoogleTrack { @JsonProperty("isrc") private String isrc; - @JsonProperty("release") - private GoogleRelease release; + @JsonProperty("releaseReference") + private GoogleRelease releaseReference; @JsonProperty("title") private String title; - @JsonProperty("artists") - private GoogleArtist[] artists; + @JsonProperty("artistReferences") + private GoogleArtist[] artistReferences; // Json format of google.protobuf.Duration is encoded as a string ends in the suffix "s". // 3 seconds and 1 microsecond should be expressed in JSON format as "3.000001s". @@ -51,16 +51,16 @@ public String getIsrc() { return isrc; } - public GoogleRelease getRelease() { - return release; + public GoogleRelease getReleaseReference() { + return releaseReference; } public String getTitle() { return title; } - public GoogleArtist[] getArtists() { - return artists; + public GoogleArtist[] getArtistReferences() { + return artistReferences; } public String getDuration() { @@ -82,16 +82,16 @@ public void setIsrc(String isrc) { this.isrc = isrc; } - public void setRelease(GoogleRelease release) { - this.release = release; + public void setReleaseReference(GoogleRelease releaseReference) { + this.releaseReference = releaseReference; } public void setTitle(String title) { this.title = title; } - public void setArtists(GoogleArtist[] artists) { - this.artists = artists; + public void setArtistReferences(GoogleArtist[] artistReferences) { + this.artistReferences = artistReferences; } public void setDuration(String duration) { @@ -112,9 +112,9 @@ public boolean equals(Object o) { } GoogleTrack that = (GoogleTrack) o; return Objects.equals(isrc, that.isrc) - && Objects.equals(release, that.release) + && Objects.equals(releaseReference, that.releaseReference) && Objects.equals(title, that.title) - && Arrays.equals(artists, that.artists) + && Arrays.equals(artistReferences, that.artistReferences) && Objects.equals(duration, that.duration) && Objects.equals(explicitType, that.explicitType); } @@ -123,9 +123,9 @@ public boolean equals(Object o) { public int hashCode() { return Objects.hash( getIsrc(), - getRelease(), + getReleaseReference(), getTitle(), - Arrays.hashCode(getArtists()), + Arrays.hashCode(getArtistReferences()), getDuration(), getExplicitType()); } diff --git a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporterTest.java b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporterTest.java index 825eb9655..7767c1576 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicExporterTest.java @@ -250,7 +250,7 @@ private GooglePlaylistItem setUpSinglePlaylistItem(String isrc, String icpn) { GoogleRelease release = new GoogleRelease(); release.setIcpn(icpn); track.setIsrc(isrc); - track.setRelease(release); + track.setReleaseReference(release); track.setExplicitType("EXPLICIT_TYPE_EXPLICIT"); playlistItemEntry.setTrack(track); return playlistItemEntry; diff --git a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporterTest.java b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporterTest.java index 9a927fae5..beb1cd63d 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/music/GoogleMusicImporterTest.java @@ -307,7 +307,7 @@ private GooglePlaylistItem buildGooglePlaylistItem(String trackIsrc, String rele GoogleTrack track = new GoogleTrack(); track.setIsrc(trackIsrc); track.setDuration(Durations.toString(Durations.fromMillis(180000L))); - track.setRelease(release); + track.setReleaseReference(release); track.setExplicitType("EXPLICIT_TYPE_NOT_EXPLICIT" + ""); From ff997a5bc405ac90b601e8fc5282a907507119e9 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 24 Nov 2023 10:09:17 +0000 Subject: [PATCH 6/8] Bump loader-utils from 1.4.0 to 1.4.2 in /client-rest (#1181) Bumps [loader-utils](https://github.com/webpack/loader-utils) from 1.4.0 to 1.4.2. - [Release notes](https://github.com/webpack/loader-utils/releases) - [Changelog](https://github.com/webpack/loader-utils/blob/v1.4.2/CHANGELOG.md) - [Commits](https://github.com/webpack/loader-utils/compare/v1.4.0...v1.4.2) --- updated-dependencies: - dependency-name: loader-utils dependency-type: indirect ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: William Morland --- client-rest/package-lock.json | 36 +++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/client-rest/package-lock.json b/client-rest/package-lock.json index cdbc2e9f0..1823f92a9 100644 --- a/client-rest/package-lock.json +++ b/client-rest/package-lock.json @@ -4277,9 +4277,9 @@ } }, "node_modules/babel-loader/node_modules/loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "dependencies": { "big.js": "^5.2.2", @@ -17778,9 +17778,9 @@ } }, "node_modules/webpack/node_modules/loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "dependencies": { "big.js": "^5.2.2", @@ -18152,9 +18152,9 @@ } }, "node_modules/worker-plugin/node_modules/loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "dependencies": { "big.js": "^5.2.2", @@ -21704,9 +21704,9 @@ } }, "loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "requires": { "big.js": "^5.2.2", @@ -31845,9 +31845,9 @@ } }, "loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "requires": { "big.js": "^5.2.2", @@ -32541,9 +32541,9 @@ } }, "loader-utils": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.0.tgz", - "integrity": "sha512-qH0WSMBtn/oHuwjy/NucEgbx5dbxxnxup9s4PVXJUDHZBQY+s0NWA9rJf53RBnQZxfch7euUui7hpoAPvALZdA==", + "version": "1.4.2", + "resolved": "https://registry.npmjs.org/loader-utils/-/loader-utils-1.4.2.tgz", + "integrity": "sha512-I5d00Pd/jwMD2QCduo657+YM/6L3KZu++pmX9VFncxaxvHcru9jx1lBaFft+r4Mt2jK0Yhp41XlRAihzPxHNCg==", "dev": true, "requires": { "big.js": "^5.2.2", From c695d547792b44941e8c27ced9f1417b3afbbfc6 Mon Sep 17 00:00:00 2001 From: William Morland Date: Fri, 24 Nov 2023 14:36:03 +0000 Subject: [PATCH 7/8] Upgrade karma to 6.4.2 (#1305) --- client-rest/package-lock.json | 52 +++++++++++++++++------------------ client-rest/package.json | 2 +- 2 files changed, 27 insertions(+), 27 deletions(-) diff --git a/client-rest/package-lock.json b/client-rest/package-lock.json index 1823f92a9..7542623a7 100644 --- a/client-rest/package-lock.json +++ b/client-rest/package-lock.json @@ -33,7 +33,7 @@ "codelyzer": "6.0.2", "jasmine-core": "~2.99.1", "jasmine-spec-reporter": "~4.2.1", - "karma": "~6.3.16", + "karma": "^6.4.2", "karma-chrome-launcher": "~2.2.0", "karma-coverage-istanbul-reporter": "~1.4.2", "karma-jasmine": "~1.1.1", @@ -2736,6 +2736,15 @@ "node": ">=4" } }, + "node_modules/@colors/colors": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/@colors/colors/-/colors-1.5.0.tgz", + "integrity": "sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ==", + "dev": true, + "engines": { + "node": ">=0.1.90" + } + }, "node_modules/@discoveryjs/json-ext": { "version": "0.5.2", "resolved": "https://registry.npmjs.org/@discoveryjs/json-ext/-/json-ext-0.5.2.tgz", @@ -9910,15 +9919,15 @@ } }, "node_modules/karma": { - "version": "6.3.16", - "resolved": "https://registry.npmjs.org/karma/-/karma-6.3.16.tgz", - "integrity": "sha512-nEU50jLvDe5yvXqkEJRf8IuvddUkOY2x5Xc4WXHz6dxINgGDrgD2uqQWeVrJs4hbfNaotn+HQ1LZJ4yOXrL7xQ==", + "version": "6.4.2", + "resolved": "https://registry.npmjs.org/karma/-/karma-6.4.2.tgz", + "integrity": "sha512-C6SU/53LB31BEgRg+omznBEMY4SjHU3ricV6zBcAe1EeILKkeScr+fZXtaI5WyDbkVowJxxAI6h73NcFPmXolQ==", "dev": true, "dependencies": { + "@colors/colors": "1.5.0", "body-parser": "^1.19.0", "braces": "^3.0.2", "chokidar": "^3.5.1", - "colors": "1.4.0", "connect": "^3.7.0", "di": "^0.0.1", "dom-serialize": "^2.2.1", @@ -9934,7 +9943,7 @@ "qjobs": "^1.2.0", "range-parser": "^1.2.1", "rimraf": "^3.0.2", - "socket.io": "^4.2.0", + "socket.io": "^4.4.1", "source-map": "^0.6.1", "tmp": "^0.2.1", "ua-parser-js": "^0.7.30", @@ -10048,15 +10057,6 @@ "node": ">=7.0.0" } }, - "node_modules/karma/node_modules/colors": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/colors/-/colors-1.4.0.tgz", - "integrity": "sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==", - "dev": true, - "engines": { - "node": ">=0.1.90" - } - }, "node_modules/karma/node_modules/glob": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.0.tgz", @@ -20443,6 +20443,12 @@ } } }, + "@colors/colors": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/@colors/colors/-/colors-1.5.0.tgz", + "integrity": "sha512-ooWCrlZP11i8GImSjTHYHLkvFDP48nS4+204nGb1RiX/WXYHmJA2III9/e2DWVabCESdW7hBAEzHRqUn9OUVvQ==", + "dev": true + }, "@discoveryjs/json-ext": { "version": "0.5.2", "resolved": "https://registry.npmjs.org/@discoveryjs/json-ext/-/json-ext-0.5.2.tgz", @@ -26191,15 +26197,15 @@ } }, "karma": { - "version": "6.3.16", - "resolved": "https://registry.npmjs.org/karma/-/karma-6.3.16.tgz", - "integrity": "sha512-nEU50jLvDe5yvXqkEJRf8IuvddUkOY2x5Xc4WXHz6dxINgGDrgD2uqQWeVrJs4hbfNaotn+HQ1LZJ4yOXrL7xQ==", + "version": "6.4.2", + "resolved": "https://registry.npmjs.org/karma/-/karma-6.4.2.tgz", + "integrity": "sha512-C6SU/53LB31BEgRg+omznBEMY4SjHU3ricV6zBcAe1EeILKkeScr+fZXtaI5WyDbkVowJxxAI6h73NcFPmXolQ==", "dev": true, "requires": { + "@colors/colors": "1.5.0", "body-parser": "^1.19.0", "braces": "^3.0.2", "chokidar": "^3.5.1", - "colors": "1.4.0", "connect": "^3.7.0", "di": "^0.0.1", "dom-serialize": "^2.2.1", @@ -26215,7 +26221,7 @@ "qjobs": "^1.2.0", "range-parser": "^1.2.1", "rimraf": "^3.0.2", - "socket.io": "^4.2.0", + "socket.io": "^4.4.1", "source-map": "^0.6.1", "tmp": "^0.2.1", "ua-parser-js": "^0.7.30", @@ -26257,12 +26263,6 @@ "color-name": "~1.1.4" } }, - "colors": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/colors/-/colors-1.4.0.tgz", - "integrity": "sha512-a+UqTh4kgZg/SlGvfbzDHpgRu7AAQOmmqRHJnxhRZICKFUT91brVhNNt58CMWU9PsBbv3PDCZUHbVxuDiH2mtA==", - "dev": true - }, "glob": { "version": "7.2.0", "resolved": "https://registry.npmjs.org/glob/-/glob-7.2.0.tgz", diff --git a/client-rest/package.json b/client-rest/package.json index c52b86ac1..3826e2478 100644 --- a/client-rest/package.json +++ b/client-rest/package.json @@ -37,7 +37,7 @@ "codelyzer": "6.0.2", "jasmine-core": "~2.99.1", "jasmine-spec-reporter": "~4.2.1", - "karma": "~6.3.16", + "karma": "^6.4.2", "karma-chrome-launcher": "~2.2.0", "karma-coverage-istanbul-reporter": "~1.4.2", "karma-jasmine": "~1.1.1", From b6e3d95de3f534c722b3c7ff6af5ae58dbc3384c Mon Sep 17 00:00:00 2001 From: osamahan999 <35288172+osamahan999@users.noreply.github.com> Date: Fri, 1 Dec 2023 11:32:46 -0800 Subject: [PATCH 8/8] Refactor GoogleMediaExporter to call thirdparty getMediaItem with retrying & error skipping. (#1295) * Test for the getGoogleMediaItem method * fixed GoogleMediaExporter constructor to take in photo interface * tweak the tests * Add a test for a set of photos with one failing * Made the test have two failures, such that we know the retrying worked and both got logged * Added todos for retrying & interface stuff * Remove VisibleForTesting on exportPhotosContainer * add nullable to the nullable fields in the constructor * Comment null fields with the field name * One more comment * Constructor signatures fixed, removed duplicate constructor * new line * reorder the other constructor --- .../google/GoogleTransferExtension.java | 2 +- .../google/media/GoogleMediaExporter.java | 92 ++++++++------ .../google/photos/GooglePhotosInterface.java | 1 + .../google/media/GoogleMediaExporterTest.java | 118 +++++++++++++++++- 4 files changed, 173 insertions(+), 40 deletions(-) diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java index 41cfb7953..39740b4e9 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/GoogleTransferExtension.java @@ -151,7 +151,7 @@ public void initialize(ExtensionContext context) { PHOTOS, new GooglePhotosExporter(credentialFactory, jobStore, jsonFactory, monitor)); exporterBuilder.put(VIDEOS, new GoogleVideosExporter(credentialFactory, jsonFactory)); exporterBuilder.put( - MEDIA, new GoogleMediaExporter(credentialFactory, jobStore, jsonFactory, monitor, idempotentImportExecutor, enableRetrying)); + MEDIA, new GoogleMediaExporter(credentialFactory, jobStore, jsonFactory, monitor, /* photosInterface= */ null, idempotentImportExecutor, enableRetrying)); exporterBuilder.put(MUSIC, new GoogleMusicExporter(credentialFactory, jsonFactory, monitor)); exporterMap = exporterBuilder.build(); diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java index ab11d9690..ec5cebc37 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporter.java @@ -15,6 +15,8 @@ */ package org.datatransferproject.datatransfer.google.media; +import static java.lang.String.format; + import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.api.client.auth.oauth2.Credential; @@ -31,6 +33,7 @@ import java.util.List; import java.util.Optional; import java.util.UUID; +import javax.annotation.Nullable; import org.datatransferproject.api.launcher.Monitor; import org.datatransferproject.datatransfer.google.common.GoogleCredentialFactory; import org.datatransferproject.datatransfer.google.mediaModels.AlbumListResponse; @@ -82,26 +85,8 @@ public GoogleMediaExporter( credentialFactory, jobStore, jsonFactory, - null, - monitor - ); - } - - public GoogleMediaExporter( - GoogleCredentialFactory credentialFactory, - TemporaryPerJobDataStore jobStore, - JsonFactory jsonFactory, - Monitor monitor, - IdempotentImportExecutor retryingExecutor, - boolean enableRetrying) { - this( - credentialFactory, - jobStore, - jsonFactory, - null, monitor, - retryingExecutor, - enableRetrying + /* photosInterface= */ null ); } @@ -110,24 +95,27 @@ public GoogleMediaExporter( GoogleCredentialFactory credentialFactory, TemporaryPerJobDataStore jobStore, JsonFactory jsonFactory, - GooglePhotosInterface photosInterface, - Monitor monitor) { + Monitor monitor, @Nullable + GooglePhotosInterface photosInterface + ) { this( credentialFactory, jobStore, jsonFactory, - photosInterface, monitor, - null, + photosInterface, + /* retryingExecutor= */ null, false); } - GoogleMediaExporter(GoogleCredentialFactory credentialFactory, + @VisibleForTesting + public GoogleMediaExporter( + GoogleCredentialFactory credentialFactory, TemporaryPerJobDataStore jobStore, JsonFactory jsonFactory, - GooglePhotosInterface photosInterface, Monitor monitor, - IdempotentImportExecutor retryingExecutor, + @Nullable GooglePhotosInterface photosInterface, + @Nullable IdempotentImportExecutor retryingExecutor, boolean enableRetrying) { this.credentialFactory = credentialFactory; this.jobStore = jobStore; @@ -224,7 +212,11 @@ private ExportResult exportPhotosContainer( for (PhotoModel photo : container.getPhotos()) { GoogleMediaItem googleMediaItem = - getOrCreatePhotosInterface(authData).getMediaItem(photo.getDataId()); + getGoogleMediaItem(photo.getIdempotentId(), photo.getDataId(), photo.getName(), authData); + if (googleMediaItem == null) { + continue; + } + photosBuilder.add(GoogleMediaItem.convertToPhotoModel(Optional.empty(), googleMediaItem)); } @@ -253,14 +245,23 @@ private ExportResult exportMediaContainer( } for (PhotoModel photo : container.getPhotos()) { - GoogleMediaItem googleMediaItem = - getOrCreatePhotosInterface(authData).getMediaItem(photo.getDataId()); - photosBuilder.add(GoogleMediaItem.convertToPhotoModel(Optional.empty(), googleMediaItem)); + GoogleMediaItem photoMediaItem = + getGoogleMediaItem(photo.getIdempotentId(), photo.getDataId(), photo.getName(), authData); + if (photoMediaItem == null) { + continue; + } + + photosBuilder.add(GoogleMediaItem.convertToPhotoModel(Optional.empty(), photoMediaItem)); } for (VideoModel video : container.getVideos()) { - GoogleMediaItem googleMediaItem = getOrCreatePhotosInterface(authData).getMediaItem(video.getDataId()); - videosBuilder.add(GoogleMediaItem.convertToVideoModel(Optional.empty(), googleMediaItem)); + GoogleMediaItem videoMediaItem = + getGoogleMediaItem(video.getIdempotentId(), video.getDataId(), video.getName(), authData); + if (videoMediaItem == null) { + continue; + } + + videosBuilder.add(GoogleMediaItem.convertToVideoModel(Optional.empty(), videoMediaItem)); } MediaContainerResource mediaContainerResource = @@ -310,7 +311,7 @@ ExportResult exportAlbums( albums.add(album); monitor.debug( - () -> String.format("%s: Google Photos exporting album: %s", jobId, album.getId())); + () -> format("%s: Google Photos exporting album: %s", jobId, album.getId())); // Add album id to continuation data continuationData.addContainerResource(new IdOnlyContainerResource(googleAlbum.getId())); @@ -438,20 +439,41 @@ private MediaContainerResource convertMediaListToResource( photos.add(photoModel); monitor.debug( - () -> String.format("%s: Google exporting photo: %s", jobId, photoModel.getDataId())); + () -> format("%s: Google exporting photo: %s", jobId, photoModel.getDataId())); } } else if (mediaItem.isVideo()) { if (shouldUpload) { VideoModel videoModel = GoogleMediaItem.convertToVideoModel(albumId, mediaItem); videos.add(videoModel); monitor.debug( - () -> String.format("%s: Google exporting video: %s", jobId, videoModel.getDataId())); + () -> format("%s: Google exporting video: %s", jobId, videoModel.getDataId())); } } } return new MediaContainerResource(null /*albums*/, photos, videos); } + //TODO(#1308): Make the retrying methods API & adaptor agnostic + @VisibleForTesting + @Nullable + GoogleMediaItem getGoogleMediaItem(String photoIdempotentId, String photoDataId, + String photoName, TokensAndUrlAuthData authData) throws IOException, InvalidTokenException, PermissionDeniedException { + if (retryingExecutor == null || !enableRetrying) { + return getOrCreatePhotosInterface(authData).getMediaItem(photoDataId); + } + + try { + GoogleMediaItem googleMediaItem = retryingExecutor.executeAndSwallowIOExceptions( + photoIdempotentId, photoName, + () -> getOrCreatePhotosInterface(authData).getMediaItem(photoDataId) + ); + return googleMediaItem; + } catch (Exception e) { + monitor.info(() -> format("Retry exception encountered while fetching a photo: %s", e)); + } + return null; + } + private synchronized GooglePhotosInterface getOrCreatePhotosInterface( TokensAndUrlAuthData authData) { return photosInterface == null ? makePhotosInterface(authData) : photosInterface; diff --git a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosInterface.java b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosInterface.java index 42f3c581f..75481532d 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosInterface.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosInterface.java @@ -65,6 +65,7 @@ import org.datatransferproject.spi.transfer.types.PermissionDeniedException; import org.datatransferproject.spi.transfer.types.UploadErrorException; +// TODO (#1307): Find a way to consolidate all 3P API interfaces public class GooglePhotosInterface { public static final String ERROR_HASH_MISMATCH = "Hash mismatch"; diff --git a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporterTest.java b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporterTest.java index 36eb8fc6e..6c0e957fd 100644 --- a/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporterTest.java +++ b/extensions/data-transfer/portability-data-transfer-google/src/test/java/org/datatransferproject/datatransfer/google/media/GoogleMediaExporterTest.java @@ -20,6 +20,7 @@ import static org.datatransferproject.datatransfer.google.media.GoogleMediaExporter.MEDIA_TOKEN_PREFIX; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -30,6 +31,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.google.api.client.json.gson.GsonFactory; +import com.google.common.collect.ImmutableList; import java.io.IOException; import java.io.InputStream; import java.util.Collection; @@ -49,12 +51,14 @@ import org.datatransferproject.datatransfer.google.photos.GooglePhotosInterface; import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore; import org.datatransferproject.spi.cloud.storage.TemporaryPerJobDataStore.InputStreamWrapper; +import org.datatransferproject.spi.transfer.idempotentexecutor.RetryingInMemoryIdempotentImportExecutor; import org.datatransferproject.spi.transfer.provider.ExportResult; import org.datatransferproject.spi.transfer.types.ContinuationData; import org.datatransferproject.spi.transfer.types.InvalidTokenException; import org.datatransferproject.spi.transfer.types.PermissionDeniedException; import org.datatransferproject.spi.transfer.types.TempMediaData; import org.datatransferproject.spi.transfer.types.UploadErrorException; +import org.datatransferproject.types.common.ExportInformation; import org.datatransferproject.types.common.PaginationData; import org.datatransferproject.types.common.StringPaginationToken; import org.datatransferproject.types.common.models.ContainerResource; @@ -62,11 +66,12 @@ import org.datatransferproject.types.common.models.photos.PhotoAlbum; import org.datatransferproject.types.common.models.photos.PhotoModel; import org.datatransferproject.types.common.models.photos.PhotosContainerResource; -import org.datatransferproject.types.common.models.videos.VideoAlbum; import org.datatransferproject.types.common.models.videos.VideoModel; -import org.datatransferproject.types.common.models.videos.VideosContainerResource; import org.datatransferproject.types.common.models.media.MediaAlbum; import org.datatransferproject.types.common.models.media.MediaContainerResource; +import org.datatransferproject.types.transfer.auth.TokensAndUrlAuthData; +import org.datatransferproject.types.transfer.retry.RetryStrategyLibrary; +import org.datatransferproject.types.transfer.retry.UniformRetryStrategy; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -78,9 +83,14 @@ public class GoogleMediaExporterTest { private static String ALBUM_TOKEN = "some-upstream-generated-album-token"; private static String MEDIA_TOKEN = "some-upstream-generated-media-token"; - private static UUID uuid = UUID.randomUUID(); + private static long RETRY_INTERVAL_MILLIS = 100L; + private static int RETRY_MAX_ATTEMPTS = 1; + private static UUID uuid = UUID.randomUUID(); + private TokensAndUrlAuthData authData; + private RetryingInMemoryIdempotentImportExecutor retryingExecutor; private GoogleMediaExporter googleMediaExporter; + private GoogleMediaExporter retryingGoogleMediaExporter; private TemporaryPerJobDataStore jobStore; private GooglePhotosInterface photosInterface; @@ -99,10 +109,20 @@ public void setup() mediaItemSearchResponse = mock(MediaItemSearchResponse.class); Monitor monitor = mock(Monitor.class); + authData = mock(TokensAndUrlAuthData.class); + + retryingExecutor = new RetryingInMemoryIdempotentImportExecutor(monitor, + new RetryStrategyLibrary(ImmutableList.of(), new UniformRetryStrategy(RETRY_MAX_ATTEMPTS, RETRY_INTERVAL_MILLIS, "identifier")) + ); googleMediaExporter = new GoogleMediaExporter( - credentialFactory, jobStore, GsonFactory.getDefaultInstance(), photosInterface, monitor); + credentialFactory, jobStore, GsonFactory.getDefaultInstance(), monitor, photosInterface); + + retryingGoogleMediaExporter = new GoogleMediaExporter( + credentialFactory, jobStore, GsonFactory.getDefaultInstance(), monitor, photosInterface, + retryingExecutor, + true); when(photosInterface.listAlbums(any(Optional.class))).thenReturn(albumListResponse); when(photosInterface.listMediaItems(any(Optional.class), any(Optional.class))) @@ -354,6 +374,92 @@ public void onlyExportAlbumlessPhoto() .containsExactly(albumlessPhotoUri + "=d"); // download } + @Test + public void testGetGoogleMediaItemSucceeds() throws IOException, InvalidTokenException, PermissionDeniedException { + String mediaItemID = "media_id"; + when(photosInterface.getMediaItem(any())).thenReturn(setUpSingleMediaItem(mediaItemID, mediaItemID, null)); + + assertThat(retryingGoogleMediaExporter.getGoogleMediaItem(mediaItemID, mediaItemID, mediaItemID, authData)).isInstanceOf(GoogleMediaItem.class); + + } + + @Test + public void testExportPhotosContainerRetrying() throws IOException, InvalidTokenException, PermissionDeniedException, UploadErrorException { + String PHOTO_ID_TO_FAIL_1 = "photo3"; + String PHOTO_ID_TO_FAIL_2 = "photo5"; + + ImmutableList albums = ImmutableList.of(); + ImmutableList photos = ImmutableList.of( + setUpSinglePhotoModel("", "photo1"), + setUpSinglePhotoModel("", "photo2"), + setUpSinglePhotoModel("", PHOTO_ID_TO_FAIL_1), + setUpSinglePhotoModel("", "photo4"), + setUpSinglePhotoModel("", PHOTO_ID_TO_FAIL_2), + setUpSinglePhotoModel("", "photo6") + ); + + PhotosContainerResource container = new PhotosContainerResource(albums, photos); + ExportInformation exportInfo = new ExportInformation(null, container); + + MediaMetadata photoMediaMetadata = new MediaMetadata(); + photoMediaMetadata.setPhoto(new Photo()); + + + // For the photo_id_to_fail photos, throw an exception. + when(photosInterface.getMediaItem(PHOTO_ID_TO_FAIL_1)).thenThrow(IOException.class); + when(photosInterface.getMediaItem(PHOTO_ID_TO_FAIL_2)).thenThrow(IOException.class); + // For all other photos, return a media item. + for (PhotoModel photoModel: photos) { + if (photoModel.getDataId().equals(PHOTO_ID_TO_FAIL_1) || photoModel.getDataId().equals(PHOTO_ID_TO_FAIL_2)) { + continue; + } + when(photosInterface.getMediaItem(photoModel.getDataId())).thenReturn( + setUpSingleMediaItem(photoModel.getDataId(), photoModel.getDataId(), photoMediaMetadata) + ); + } + + ExportResult result = retryingGoogleMediaExporter.export( + uuid, authData, Optional.of(exportInfo) + ); + assertThat( + result.getExportedData().getPhotos().stream().map(x -> x.getDataId()).collect(Collectors.toList()) + ).isEqualTo( + photos.stream().map( + x -> x.getDataId() + ).filter( + dataId -> !(dataId.equals(PHOTO_ID_TO_FAIL_1) || dataId.equals(PHOTO_ID_TO_FAIL_2)) + ).collect( + Collectors.toList() + ) + ); + assertThat(result.getExportedData().getPhotos().size()).isEqualTo(photos.size() - 2); + assertThat(retryingExecutor.getErrors().size()).isEqualTo(2); + assertThat(retryingExecutor.getErrors().stream().findFirst().toString().contains("IOException")).isTrue(); + } + + @Test + public void testGetGoogleMediaItemFailed() throws IOException, InvalidTokenException, PermissionDeniedException { + String mediaItemID = "media_id"; + when(photosInterface.getMediaItem(mediaItemID)).thenThrow(IOException.class); + + long start = System.currentTimeMillis(); + assertThat(retryingGoogleMediaExporter.getGoogleMediaItem(mediaItemID, mediaItemID, mediaItemID, authData)).isNull(); + long end = System.currentTimeMillis(); + + // If retrying occurred, then the retry_interval must have been waited at least max_attempts + // amount of times. + assertThat(end - start).isAtLeast(RETRY_INTERVAL_MILLIS * RETRY_MAX_ATTEMPTS); + assertThat(retryingExecutor.getErrors().size()).isEqualTo(1); + assertThat(retryingExecutor.getErrors().stream().findFirst().toString()).contains("IOException"); + + + start = System.currentTimeMillis(); + assertThrows(IOException.class, () -> googleMediaExporter.getGoogleMediaItem(mediaItemID, mediaItemID, mediaItemID, authData)); + end = System.currentTimeMillis(); + + assertThat(end - start).isLessThan(RETRY_INTERVAL_MILLIS * RETRY_MAX_ATTEMPTS); + } + /** Sets up a response with a single album, containing a single photo */ private void setUpSingleAlbum() { GoogleAlbum albumEntry = new GoogleAlbum(); @@ -363,6 +469,10 @@ private void setUpSingleAlbum() { when(albumListResponse.getAlbums()).thenReturn(new GoogleAlbum[] {albumEntry}); } + private static PhotoModel setUpSinglePhotoModel(String albumId, String dataId) { + return new PhotoModel("Title", "fetchableUrl", "description", "photo", dataId, albumId, false); + } + /** Sets up a response for a single photo */ // TODO(zacsh) delete this helper in favor of explicitly setting the fields that an assertion will // _actually_ use (and doing so _inlined, visibly_ in the arrange phase).