Fixing Google Photos issue where temp photos data was too big for the…

… job store (#619)
google · Nov 20, 2018 · 6e65d0d · 6e65d0d
1 parent 09f8763
commit 6e65d0d
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 11 deletions.
diff --git a/...rc/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosExporter.java b/...rc/main/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosExporter.java
@@ -15,12 +15,17 @@
  */
 package org.datatransferproject.datatransfer.google.photos;
 
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 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 java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.InputStream;
+import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -236,9 +241,20 @@ void populateContainedPhotosList(UUID jobId, TokensAndUrlAuthData authData)
       albumToken = albumListResponse.getNextPageToken();
     } while (albumToken != null);
 
+    // TODO: if we see complaints about objects being too large for JobStore in other places, we
+    // should consider putting logic in JobStore itself to handle it
+    InputStream stream = convertJsonToInputStream(tempPhotosData);
+    jobStore.create(jobId, createCacheKey(), stream);
+
     jobStore.create(jobId, createCacheKey(), tempPhotosData);
   }
 
+  @VisibleForTesting
+  static InputStream convertJsonToInputStream(Object jsonObject) throws JsonProcessingException {
+    String tempString = new ObjectMapper().writeValueAsString(jsonObject);
+    return new ByteArrayInputStream(tempString.getBytes(StandardCharsets.UTF_8));
+  }
+
   private Optional<String> getPhotosPaginationToken(Optional<PaginationData> paginationData) {
     Optional<String> paginationToken = Optional.empty();
     if (paginationData.isPresent()) {
@@ -255,12 +271,24 @@ private Optional<String> getPhotosPaginationToken(Optional<PaginationData> pagin
   private List<PhotoModel> convertPhotosList(Optional<String> albumId,
       GoogleMediaItem[] mediaItems, UUID jobId) throws IOException {
     List<PhotoModel> photos = new ArrayList<>(mediaItems.length);
+
+    TempPhotosData tempPhotosData = null;
+    InputStream stream = jobStore.getStream(jobId, createCacheKey());
+    if (stream != null) {
+      tempPhotosData = new ObjectMapper().readValue(stream, TempPhotosData.class);
+      stream.close();
+    }
+
     for (GoogleMediaItem mediaItem : mediaItems) {
       if (mediaItem.getMediaMetadata().getPhoto() != null) {
         // TODO: address videos
-        if (albumId.isPresent() ||
-            !jobStore.findData(jobId, createCacheKey(), TempPhotosData.class)
-                .isContainedPhotoId(mediaItem.getId())) {
+        boolean shouldUpload = albumId.isPresent();
+
+        if (tempPhotosData != null) {
+          shouldUpload = shouldUpload || !tempPhotosData.isContainedPhotoId(mediaItem.getId());
+        }
+
+        if (shouldUpload) {
           photos.add(convertToPhotoModel(albumId, mediaItem));
         }
       }

diff --git a/...est/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosExporterTest.java b/...est/java/org/datatransferproject/datatransfer/google/photos/GooglePhotosExporterTest.java
@@ -24,14 +24,15 @@
 import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.api.client.json.jackson2.JacksonFactory;
 import java.io.IOException;
+import java.io.InputStream;
 import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.stream.Collectors;
-import org.datatransferproject.cloud.local.LocalJobStore;
 import org.datatransferproject.datatransfer.google.common.GoogleCredentialFactory;
 import org.datatransferproject.datatransfer.google.photos.model.AlbumListResponse;
 import org.datatransferproject.datatransfer.google.photos.model.GoogleAlbum;
@@ -52,6 +53,7 @@
 import org.datatransferproject.types.transfer.models.photos.PhotosContainerResource;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Matchers;
 
 public class GooglePhotosExporterTest {
@@ -74,7 +76,7 @@ public class GooglePhotosExporterTest {
   @Before
   public void setup() throws IOException {
     GoogleCredentialFactory credentialFactory = mock(GoogleCredentialFactory.class);
-    jobStore = new LocalJobStore();
+    jobStore = mock(JobStore.class);
     photosInterface = mock(GooglePhotosInterface.class);
 
     albumListResponse = mock(AlbumListResponse.class);
@@ -166,9 +168,8 @@ public void exportPhotoFirstSet() throws IOException {
 
     IdOnlyContainerResource idOnlyContainerResource = new IdOnlyContainerResource(ALBUM_ID);
 
-    ExportResult<PhotosContainerResource> result =
-        googlePhotosExporter
-            .exportPhotos(null, Optional.of(idOnlyContainerResource), Optional.empty(), uuid);
+    ExportResult<PhotosContainerResource> result = googlePhotosExporter
+        .exportPhotos(null, Optional.of(idOnlyContainerResource), Optional.empty(), uuid);
 
     // Check results
     // Verify correct methods were called
@@ -244,8 +245,11 @@ public void populateContainedPhotosList() throws IOException {
     googlePhotosExporter.populateContainedPhotosList(uuid, null);
 
     // Check contents of job store
-    TempPhotosData tempPhotosData = jobStore.findData(uuid, "tempPhotosData", TempPhotosData.class);
-    assertThat(tempPhotosData.lookupContainedPhotoIds()).containsExactly(PHOTO_ID, secondId);
+    ArgumentCaptor<TempPhotosData> tempPhotosDataArgumentCaptor = ArgumentCaptor
+        .forClass(TempPhotosData.class);
+    verify(jobStore).create(Matchers.eq(uuid), Matchers.eq("tempPhotosData"), tempPhotosDataArgumentCaptor.capture());
+    assertThat(tempPhotosDataArgumentCaptor.getValue().lookupContainedPhotoIds())
+        .containsExactly(PHOTO_ID, secondId);
   }
 
   @Test
@@ -272,7 +276,8 @@ public void onlyExportAlbumlessPhoto() throws IOException {
 
     TempPhotosData tempPhotosData = new TempPhotosData(uuid);
     tempPhotosData.addContainedPhotoId(containedPhotoId);
-    jobStore.create(uuid, "tempPhotosData", tempPhotosData);
+    InputStream stream = GooglePhotosExporter.convertJsonToInputStream(tempPhotosData);
+    when(jobStore.getStream(uuid, "tempPhotosData")).thenReturn(stream);
 
     // Run test
     ExportResult<PhotosContainerResource> result = googlePhotosExporter