fixing forbidden characters for onedrive imports (#820)

dtinit · Jan 13, 2020 · b5edade · b5edade
1 parent e6e126f
commit b5edade
Show file tree

Hide file tree

Showing 9 changed files with 382 additions and 36 deletions.
diff --git a/...in/java/org/datatransferproject/transfer/microsoft/MicrosoftTransmogrificationConfig.java b/...in/java/org/datatransferproject/transfer/microsoft/MicrosoftTransmogrificationConfig.java
@@ -0,0 +1,13 @@
+package org.datatransferproject.transfer.microsoft;
+
+import org.datatransferproject.types.common.models.TransmogrificationConfig;
+
+// This class defines transmogrification paramaters for Microsoft imports
+public class MicrosoftTransmogrificationConfig extends TransmogrificationConfig {
+	/** 
+	OneDrive has forbidden characters for file names:
+		https://support.office.com/en-us/article/invalid-file-names-and-file-types-in-onedrive-onedrive-for-business-and-sharepoint-64883a5d-228e-48f5-b3d2-eb39e07630fa#invalidcharacters
+	*/	
+	private final String 	PHOTO_TITLE_FORBIDDEN_CHARACTERS 	= "~\"#%&*:<>?/\\{|}.";
+	private final String 	ALBUM_NAME_FORBIDDEN_CHARACTERS 	= "~\"#%&*:<>?/\\{|}.";
+}
diff --git a/.../main/java/org/datatransferproject/transfer/microsoft/photos/MicrosoftPhotosImporter.java b/.../main/java/org/datatransferproject/transfer/microsoft/photos/MicrosoftPhotosImporter.java
@@ -19,7 +19,6 @@
 import com.google.api.client.auth.oauth2.Credential;
 import com.google.common.base.Strings;
 import com.google.common.base.Preconditions;
-import java.util.stream.Collectors;
 import okhttp3.MediaType;
 import okhttp3.OkHttpClient;
 import okhttp3.Request;
@@ -37,6 +36,7 @@
 import org.datatransferproject.spi.transfer.provider.ImportResult;
 import org.datatransferproject.spi.transfer.provider.Importer;
 import org.datatransferproject.transfer.microsoft.common.MicrosoftCredentialFactory;
+import org.datatransferproject.transfer.microsoft.MicrosoftTransmogrificationConfig;
 import org.datatransferproject.types.common.models.photos.PhotoAlbum;
 import org.datatransferproject.types.common.models.photos.PhotoModel;
 import org.datatransferproject.types.common.models.photos.PhotosContainerResource;
@@ -45,7 +45,9 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.URL;
+import java.util.Arrays;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.UUID;
 
@@ -65,6 +67,7 @@ public class MicrosoftPhotosImporter implements Importer<TokensAndUrlAuthData, P
   private final TemporaryPerJobDataStore jobStore;
   private final Monitor monitor;
   private final MicrosoftCredentialFactory credentialFactory;
+  private final MicrosoftTransmogrificationConfig transmogrificationConfig = new MicrosoftTransmogrificationConfig();
   private Credential credential;
 
   private final String createFolderUrl;
@@ -106,6 +109,9 @@ public ImportResult importItem(
     // Ensure credential is populated
     getOrCreateCredential(authData);
 
+    // Make the data onedrive compatinle
+    resource.transmogrify(transmogrificationConfig);
+
     for (PhotoAlbum album : resource.getAlbums()) {
       // Create a OneDrive folder and then save the id with the mapping data
       idempotentImportExecutor.executeAndSwallowIOExceptions(
@@ -128,7 +134,7 @@ public ImportResult importItem(
   private String createOneDriveFolder(PhotoAlbum album) throws IOException {
 
     Map<String, Object> rawFolder = new LinkedHashMap<>();
-    rawFolder.put("name", cleanName(album.getName()));
+    rawFolder.put("name", album.getName());
     rawFolder.put("folder", new LinkedHashMap());
     rawFolder.put("@microsoft.graph.conflictBehavior", "rename");
 
@@ -236,16 +242,6 @@ private String importSinglePhoto(
     }
   }
 
-  // Replace any non-whitespace, non-letter, or non-digit characters with -
-  static String cleanName(String name) {
-    return name.chars()
-        .mapToObj(c -> (char) c)
-        .map(c -> (!Character.isLetterOrDigit(c) && !Character.isWhitespace(c)) ? '-' : c)
-        .limit(40)
-        .map(Object::toString)
-        .collect(Collectors.joining(""));
-  }
-
   private Credential getOrCreateCredential(TokensAndUrlAuthData authData){
     if (this.credential == null){
       this.credential = this.credentialFactory.createCredential(authData);

diff --git a/...c/main/java/org/datatransferproject/transfer/smugmug/SmugMugTransmogrificationConfig.java b/...c/main/java/org/datatransferproject/transfer/smugmug/SmugMugTransmogrificationConfig.java
@@ -0,0 +1,12 @@
+package org.datatransferproject.transfer.smugmug;
+
+import org.datatransferproject.types.common.models.TransmogrificationConfig;
+
+// This class defines transmogrification paramaters for SmugMug imports
+public class SmugMugTransmogrificationConfig extends TransmogrificationConfig {
+	// Smugmug doesn't allow photos to exist outside of a folder
+	private final boolean 	ALBUM_ALLOW_ROOT_PHOTOS				= false;
+	// Album size specified here:
+  	// https://github.com/google/data-transfer-project/pull/805/files
+	private final int       ALBUM_MAX_SIZE						= 5000;
+}
diff --git a/.../src/main/java/org/datatransferproject/transfer/smugmug/photos/SmugMugPhotosImporter.java b/.../src/main/java/org/datatransferproject/transfer/smugmug/photos/SmugMugPhotosImporter.java
@@ -25,6 +25,7 @@
 import org.datatransferproject.spi.transfer.idempotentexecutor.IdempotentImportExecutor;
 import org.datatransferproject.spi.transfer.provider.ImportResult;
 import org.datatransferproject.spi.transfer.provider.Importer;
+import org.datatransferproject.transfer.smugmug.SmugMugTransmogrificationConfig;
 import org.datatransferproject.transfer.smugmug.photos.model.SmugMugAlbumResponse;
 import org.datatransferproject.transfer.smugmug.photos.model.SmugMugImageUploadResponse;
 import org.datatransferproject.types.common.models.photos.PhotoAlbum;
@@ -42,20 +43,16 @@
 public class SmugMugPhotosImporter
     implements Importer<TokenSecretAuthData, PhotosContainerResource> {
 
-  // Album size specified here:
-  // https://github.com/google/data-transfer-project/pull/805/files
-  private static final int MAX_ALBUM_SIZE = 5000;
-  // Smugmug doesn't allow photos to exist outside of a folder
-  private static final boolean ALLOW_LOOSE_PHOTOS = false;
-
   private final TemporaryPerJobDataStore jobStore;
   private final AppCredentials appCredentials;
   private final HttpTransport transport;
   private final ObjectMapper mapper;
   private final Monitor monitor;
+  private final SmugMugTransmogrificationConfig transmogrificationConfig = new SmugMugTransmogrificationConfig();;
 
   private SmugMugInterface smugMugInterface;
 
+
   public SmugMugPhotosImporter(
       TemporaryPerJobDataStore jobStore,
       HttpTransport transport,
@@ -78,7 +75,7 @@ public SmugMugPhotosImporter(
     this.transport = transport;
     this.appCredentials = appCredentials;
     this.mapper = mapper;
-    this.monitor = monitor;
+    this.monitor = monitor; 
   }
 
   @Override
@@ -87,7 +84,10 @@ public ImportResult importItem(
       IdempotentImportExecutor idempotentExecutor,
       TokenSecretAuthData authData,
       PhotosContainerResource data) throws Exception {
-    data.transmogrifyAlbums(MAX_ALBUM_SIZE, ALLOW_LOOSE_PHOTOS);
+
+    // Make the data smugmug compatible
+    data.transmogrify(transmogrificationConfig);
+
     try {
       SmugMugInterface smugMugInterface = getOrCreateSmugMugInterface(authData);
       for (PhotoAlbum album : data.getAlbums()) {

diff --git a/...n/src/main/java/org/datatransferproject/types/common/models/TransmogrificationConfig.java b/...n/src/main/java/org/datatransferproject/types/common/models/TransmogrificationConfig.java
@@ -0,0 +1,46 @@
+package org.datatransferproject.types.common.models;
+
+public class TransmogrificationConfig {
+	private final String 	PHOTO_TITLE_FORBIDDEN_CHARACTERS 	= "";
+	private final char 		PHOTO_TITLE_REPLACEMENT_CHARACTER 	= '_';
+	private final int 		PHOTO_TITLE_MAX_LENGTH				= -1;
+
+	private final String 	ALBUM_NAME_FORBIDDEN_CHARACTERS 	= "";
+	private final char 		ALBUM_NAME_REPLACEMENT_CHARACTER	= '_';
+	private final int 		ALBUM_NAME_MAX_LENGTH			 	= -1;
+	private final boolean 	ALBUM_ALLOW_ROOT_PHOTOS				= true;
+	private final int       ALBUM_MAX_SIZE						= -1;
+
+	public String getPhotoTitleForbiddenCharacters() {
+		return PHOTO_TITLE_FORBIDDEN_CHARACTERS;
+	}
+
+	public char getPhotoTitleReplacementCharater() {
+		return PHOTO_TITLE_REPLACEMENT_CHARACTER;
+	}
+
+	public int getPhotoTitleMaxLength() {
+		return PHOTO_TITLE_MAX_LENGTH;
+	}
+
+	public String getAlbumNameForbiddenCharacters() {
+		return ALBUM_NAME_FORBIDDEN_CHARACTERS;
+	}
+
+	public char getAlbumNameReplacementCharacter() {
+		return ALBUM_NAME_REPLACEMENT_CHARACTER;
+	}
+
+	public int getAlbumNameMaxLength() {
+		return ALBUM_NAME_MAX_LENGTH;
+	}
+
+	public boolean getAlbumAllowRootPhotos() {
+		return ALBUM_ALLOW_ROOT_PHOTOS;
+	}
+
+	public int getAlbumMaxSize() {
+		return ALBUM_MAX_SIZE;
+	}
+
+}
diff --git a/...s-common/src/main/java/org/datatransferproject/types/common/models/photos/PhotoAlbum.java b/...s-common/src/main/java/org/datatransferproject/types/common/models/photos/PhotoAlbum.java
@@ -22,10 +22,11 @@
 import java.util.Objects;
 import java.util.List;
 import java.util.ArrayList;
+import java.util.stream.Collectors;
 
 public class PhotoAlbum {
   private final String id;
-  private final String name;
+  private String name;
   private final String description;
 
   /** The {@code id} is used to associate photos with this album. * */
@@ -83,4 +84,16 @@ public List<PhotoAlbum> split(int numberOfNewAlbums){
     }
     return newAlbums;
   }
+
+  public void cleanName(String forbiddenCharacters, char replacementCharacter, int maxLength) {
+    name = name.chars()
+        .mapToObj(c -> (char) c)
+        .map(c -> forbiddenCharacters.contains(Character.toString(c)) ? replacementCharacter : c)
+        .map(Object::toString)
+        .collect(Collectors.joining(""));
+    if (maxLength <= 0) {
+      return;
+    }
+    name = name.substring(0, maxLength);
+  }
 }
diff --git a/...s-common/src/main/java/org/datatransferproject/types/common/models/photos/PhotoModel.java b/...s-common/src/main/java/org/datatransferproject/types/common/models/photos/PhotoModel.java
@@ -20,9 +20,11 @@
 import com.google.common.base.MoreObjects;
 import com.google.common.base.Objects;
 
+import java.util.stream.Collectors;
+
 public class PhotoModel {
 
-  private final String title;
+  private String title;
   private final String fetchableUrl;
   private final String description;
   private final String mediaType;
@@ -75,6 +77,19 @@ public String getDataId() {
     return dataId;
   }
 
+  // remove all forbidden characters
+  public void cleanTitle(String forbiddenCharacters, char replacementCharacter, int maxLength) {
+    title = title.chars()
+        .mapToObj(c -> (char) c)
+        .map(c -> forbiddenCharacters.contains(Character.toString(c)) ? replacementCharacter : c)
+        .map(Object::toString)
+        .collect(Collectors.joining(""));
+    if (maxLength <= 0) {
+      return;
+    }
+    title = title.substring(0, Math.min(maxLength, title.length()));
+  }
+
   public boolean isInTempStore() { return inTempStore; }
 
   @Override

diff --git a/...main/java/org/datatransferproject/types/common/models/photos/PhotosContainerResource.java b/...main/java/org/datatransferproject/types/common/models/photos/PhotosContainerResource.java
@@ -22,6 +22,7 @@
 import com.google.common.collect.ImmutableMap;
 import java.util.Map;
 import org.datatransferproject.types.common.models.ContainerResource;
+import org.datatransferproject.types.common.models.TransmogrificationConfig;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
 
@@ -84,15 +85,19 @@ public int hashCode() {
     return Objects.hash(getAlbums(), getPhotos());
   }
 
-  public void transmogrifyAlbums(int maxSize){
-    transmogrifyAlbums(maxSize, true);
+  public void transmogrify(TransmogrificationConfig config) {
+    transmogrifyAlbums(config);
+    transmogrifyPhotos(config);
   }
 
   // Coerce the albums of the transfer using the specification provided, e.g.
   // limiting max album size or grouping un-collected photos into a root album.
-  public void transmogrifyAlbums(int maxSize, boolean allowRootPhotos){
-      ensureRootAlbum(allowRootPhotos);
-      ensureAlbumSize(maxSize);
+  private void transmogrifyAlbums(TransmogrificationConfig config) {
+    ensureRootAlbum(config.getAlbumAllowRootPhotos());
+    ensureAlbumSize(config.getAlbumMaxSize());
+    ensureCleanAlbumNames(config.getAlbumNameForbiddenCharacters(), 
+      config.getAlbumNameReplacementCharacter(),
+      config.getAlbumNameMaxLength());
   }
 
   // Splits albumns that are too large into albums that are smaller than {maxSize}.
@@ -161,4 +166,29 @@ void ensureRootAlbum(boolean allowRootPhotos){
       this.albums = albums_;
     }
   }
+
+  // Replaces forbidden characters and makes sure that the name is not too long
+  void ensureCleanAlbumNames(String forbiddenTitleCharacters, char replacementCharacter, int maxTitleLength) {
+    for (PhotoAlbum album: albums) {
+      album.cleanName(forbiddenTitleCharacters, replacementCharacter, maxTitleLength);
+    }
+  }
+
+  // Coerce the photos of the transfer using the specification provided, e.g.
+  // limiting max title length or removing forbidden characters, etc.
+  private void transmogrifyPhotos(TransmogrificationConfig config) {
+    ensureCleanPhotoTitles(
+      config.getPhotoTitleForbiddenCharacters(),
+      config.getPhotoTitleReplacementCharater(), 
+      config.getPhotoTitleMaxLength());
+  }
+
+  // Replaces forbidden characters and makes sure that the title is not too long
+  void ensureCleanPhotoTitles(String forbiddenTitleCharacters, char replacementCharacter, int maxTitleLength) {
+    for (PhotoModel photo: photos) {
+      photo.cleanTitle(forbiddenTitleCharacters, replacementCharacter, maxTitleLength);
+    }
+  }
+
+
 }