[#10944] Migrate Blobstore API to Google Cloud Storage (#10945)

TEAMMATES · Feb 26, 2021 · 56d7c65 · 56d7c65
1 parent 3c3a3b6
commit 56d7c65
Show file tree

Hide file tree

Showing 53 changed files with 360 additions and 534 deletions.
diff --git a/.gitignore b/.gitignore
@@ -60,5 +60,7 @@ src/client/java/teammates/client/scripts/statistics/data/
 src/client/java/teammates/client/scripts/log/
 src/e2e/resources/gmail-api/
 src/e2e/resources/downloads/
+filestorage-dev/*
+src/test/resources/filestorage/**/*
 
 !.gitkeep
diff --git a/build.gradle b/build.gradle
@@ -56,19 +56,7 @@ dependencies {
     implementation("com.google.appengine:appengine-api-1.0-sdk:${appengineVersion}")
     implementation("com.google.cloud:google-cloud-tasks:1.30.11")
     implementation("com.google.cloud:google-cloud-logging:2.1.2")
-
-    // This dependency needs to be resolved individually
-    // because the main dependency (appengine-gcs-client) does not have its transitive dependencies up-to-date
-    implementation("com.google.appengine.tools:appengine-gcs-client:0.8.1") {
-        // Use the newer servlet library instead
-        exclude group: "javax.servlet", module: "servlet-api"
-    }
-    implementation("com.google.api-client:google-api-client-appengine:1.30.9") {
-        // Use the newer servlet library instead
-        exclude group: "javax.servlet", module: "servlet-api"
-    }
-    implementation("com.google.apis:google-api-services-storage:v1-rev20200430-1.30.9")
-
+    implementation("com.google.cloud:google-cloud-storage:1.113.9")
     implementation("com.google.code.gson:gson:2.8.6")
     implementation("com.google.guava:guava:30.1-jre")
     implementation(objectify)
@@ -280,9 +268,8 @@ appengine {
         port = 8080
         jvmFlags = ["-Xss2m", "-Dfile.encoding=UTF-8",
                 // Absolute paths are not supported, the following is relative to the project directory
-                // These only specify the datastore/blobstore paths, but search indexes are still generated in WEB-INF/appengine-generated
-                "-Ddatastore.backing_store=../../appengine-generated/local_db.bin",
-                "-Dblobstore.backing_store=../../appengine-generated"]
+                // These only specify the datastore paths, but search indexes are still generated in WEB-INF/appengine-generated
+                "-Ddatastore.backing_store=../../appengine-generated/local_db.bin"]
         automaticRestart = true
     }
     deploy {

diff --git a/filestorage-dev/.gitkeep b/filestorage-dev/.gitkeep
diff --git a/src/client/java/teammates/client/scripts/GoogleIdMigrationBaseScript.java b/src/client/java/teammates/client/scripts/GoogleIdMigrationBaseScript.java
@@ -3,17 +3,15 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
-import com.google.appengine.tools.cloudstorage.GcsFilename;
-import com.google.appengine.tools.cloudstorage.GcsService;
-import com.google.appengine.tools.cloudstorage.GcsServiceFactory;
-import com.google.appengine.tools.cloudstorage.RetryParams;
+import com.google.cloud.storage.Blob;
+import com.google.cloud.storage.Storage;
+import com.google.cloud.storage.StorageOptions;
 import com.googlecode.objectify.Key;
 import com.googlecode.objectify.cmd.Query;
 
 import teammates.client.util.ClientProperties;
 import teammates.common.datatransfer.attributes.InstructorAttributes;
 import teammates.common.util.Config;
-import teammates.common.util.GoogleCloudStorageHelper;
 import teammates.storage.api.InstructorsDb;
 import teammates.storage.entity.Account;
 import teammates.storage.entity.CourseStudent;
@@ -100,23 +98,14 @@ protected void migrateEntity(Account oldAccount) throws Exception {
         ofy().delete().type(Account.class).id(oldGoogleId).now();
 
         if (oldStudentProfile != null) {
-            String pictureKey = oldStudentProfile.getPictureKey();
-
             if (!ClientProperties.isTargetUrlDevServer()) {
-                try {
-                    GcsFilename oldGcsFilename = new GcsFilename(Config.PRODUCTION_GCS_BUCKETNAME, oldGoogleId);
-                    GcsFilename newGcsFilename = new GcsFilename(Config.PRODUCTION_GCS_BUCKETNAME, newGoogleId);
-                    GcsService gcsService = GcsServiceFactory.createGcsService(RetryParams.getDefaultInstance());
-                    gcsService.copy(oldGcsFilename, newGcsFilename);
-                    gcsService.delete(oldGcsFilename);
-                    pictureKey = GoogleCloudStorageHelper.createBlobKey(newGoogleId);
-                } catch (Exception e) {
-                    log("Profile picture not exist or error during copy: " + e.getMessage());
-                }
+                Storage storage = StorageOptions.newBuilder().setProjectId(Config.APP_ID).build().getService();
+                Blob blob = storage.get(Config.PRODUCTION_GCS_BUCKETNAME, oldGoogleId);
+                blob.copyTo(Config.PRODUCTION_GCS_BUCKETNAME, newGoogleId);
+                blob.delete();
             }
 
             oldStudentProfile.setGoogleId(newGoogleId);
-            oldStudentProfile.setPictureKey(pictureKey);
             ofy().save().entity(oldStudentProfile).now();
             ofy().delete().key(oldStudentProfileKey).now();
         }

diff --git a/src/e2e/resources/data/InstructorCourseStudentDetailsPageE2ETest.json b/src/e2e/resources/data/InstructorCourseStudentDetailsPageE2ETest.json
@@ -72,8 +72,7 @@
       "institute": "TEAMMATES Test Institute 7",
       "nationality": "Laotian",
       "gender": "MALE",
-      "moreInfo": "This is a lot of info...",
-      "pictureKey": ""
+      "moreInfo": "This is a lot of info..."
     }
   }
 }
diff --git a/src/e2e/resources/data/InstructorStudentRecordsPageE2ETest.json b/src/e2e/resources/data/InstructorStudentRecordsPageE2ETest.json
@@ -70,8 +70,7 @@
       "institute": "TEAMMATES Test Institute 7",
       "nationality": "Singaporean",
       "gender": "MALE",
-      "moreInfo": "This is a lot of info!",
-      "pictureKey": ""
+      "moreInfo": "This is a lot of info!"
     }
   }
 }
diff --git a/src/e2e/resources/data/StudentCourseDetailsPageE2ETest.json b/src/e2e/resources/data/StudentCourseDetailsPageE2ETest.json
@@ -105,8 +105,7 @@
       "institute": "inst",
       "nationality": "American",
       "gender": "OTHER",
-      "moreInfo": "I am just a student :P",
-      "pictureKey": ""
+      "moreInfo": "I am just a student :P"
     },
     "SCDet.charlie": {
       "googleId": "tm.e2e.SCDet.charlie",
@@ -115,8 +114,7 @@
       "institute": "inst",
       "nationality": "Singaporean",
       "gender": "OTHER",
-      "moreInfo": "I am also a student :P",
-      "pictureKey": ""
+      "moreInfo": "I am also a student :P"
     }
   }
 }
diff --git a/src/e2e/resources/data/StudentProfilePageE2ETest.json b/src/e2e/resources/data/StudentProfilePageE2ETest.json
@@ -39,8 +39,7 @@
       "institute": "TEAMMATES Test Institute 4",
       "nationality": "Singaporean",
       "gender": "MALE",
-      "moreInfo": "I am just another student :P",
-      "pictureKey": ""
+      "moreInfo": "I am just another student :P"
     }
   }
 }
diff --git a/src/lnp/java/teammates/lnp/cases/StudentProfileLNPTest.java b/src/lnp/java/teammates/lnp/cases/StudentProfileLNPTest.java
@@ -120,7 +120,6 @@ protected Map<String, StudentProfileAttributes> generateProfiles() {
                             .withShortName(String.valueOf(i))
                             .withInstitute("TEAMMATES Test Institute 222")
                             .withMoreInfo("I am " + i)
-                            .withPictureKey("")
                             .withGender(StudentProfileAttributes.Gender.MALE)
                             .withNationality("American")
                             .build()

diff --git a/src/main/java/teammates/common/datatransfer/attributes/StudentProfileAttributes.java b/src/main/java/teammates/common/datatransfer/attributes/StudentProfileAttributes.java
@@ -25,7 +25,6 @@ public class StudentProfileAttributes extends EntityAttributes<StudentProfile> {
     public String nationality;
     public Gender gender;
     public String moreInfo;
-    public String pictureKey;
     public Instant modifiedDate;
 
     private StudentProfileAttributes(String googleId) {
@@ -36,7 +35,6 @@ private StudentProfileAttributes(String googleId) {
         this.nationality = "";
         this.gender = Gender.OTHER;
         this.moreInfo = "";
-        this.pictureKey = "";
         this.modifiedDate = Instant.now();
     }
 
@@ -59,9 +57,6 @@ public static StudentProfileAttributes valueOf(StudentProfile sp) {
         if (sp.getMoreInfo() != null) {
             studentProfileAttributes.moreInfo = sp.getMoreInfo();
         }
-        if (sp.getPictureKey() != null) {
-            studentProfileAttributes.pictureKey = sp.getPictureKey();
-        }
         if (sp.getModifiedDate() != null) {
             studentProfileAttributes.modifiedDate = sp.getModifiedDate();
         }
@@ -85,7 +80,6 @@ public StudentProfileAttributes getCopy() {
         studentProfileAttributes.gender = gender;
         studentProfileAttributes.nationality = nationality;
         studentProfileAttributes.moreInfo = moreInfo;
-        studentProfileAttributes.pictureKey = pictureKey;
         studentProfileAttributes.modifiedDate = modifiedDate;
 
         return studentProfileAttributes;
@@ -119,10 +113,6 @@ public String getMoreInfo() {
         return moreInfo;
     }
 
-    public String getPictureKey() {
-        return pictureKey;
-    }
-
     public Instant getModifiedDate() {
         return modifiedDate;
     }
@@ -153,8 +143,6 @@ public List<String> getInvalidityInfo() {
 
         Assumption.assertNotNull(gender);
 
-        Assumption.assertNotNull(this.pictureKey);
-
         // No validation for modified date as it is determined by the system.
         // No validation for More Info. It will properly sanitized.
 
@@ -170,7 +158,7 @@ public String toString() {
     public int hashCode() {
         StringBuilder stringBuilder = new StringBuilder();
         stringBuilder.append(this.email).append(this.shortName).append(this.institute)
-                .append(this.googleId).append(this.pictureKey).append(this.gender.toString());
+                .append(this.googleId).append(this.gender.toString());
         return stringBuilder.toString().hashCode();
     }
 
@@ -186,7 +174,6 @@ public boolean equals(Object other) {
                     && Objects.equals(this.shortName, otherProfile.shortName)
                     && Objects.equals(this.institute, otherProfile.institute)
                     && Objects.equals(this.googleId, otherProfile.googleId)
-                    && Objects.equals(this.pictureKey, otherProfile.pictureKey)
                     && Objects.equals(this.gender, otherProfile.gender);
         } else {
             return false;
@@ -196,7 +183,7 @@ public boolean equals(Object other) {
     @Override
     public StudentProfile toEntity() {
         return new StudentProfile(googleId, shortName, email, institute, nationality, gender.name().toLowerCase(),
-                                  moreInfo, this.pictureKey);
+                                  moreInfo);
     }
 
     @Override
@@ -214,7 +201,6 @@ public void update(UpdateOptions updateOptions) {
         updateOptions.nationalityOption.ifPresent(s -> nationality = s);
         updateOptions.genderOption.ifPresent(s -> gender = s);
         updateOptions.moreInfoOption.ifPresent(s -> moreInfo = s);
-        updateOptions.pictureKeyOption.ifPresent(s -> pictureKey = s);
     }
 
     /**
@@ -280,7 +266,6 @@ public static class UpdateOptions {
         private UpdateOption<String> nationalityOption = UpdateOption.empty();
         private UpdateOption<Gender> genderOption = UpdateOption.empty();
         private UpdateOption<String> moreInfoOption = UpdateOption.empty();
-        private UpdateOption<String> pictureKeyOption = UpdateOption.empty();
 
         private UpdateOptions(String googleId) {
             Assumption.assertNotNull(googleId);
@@ -380,13 +365,6 @@ public B withMoreInfo(String moreInfo) {
             return thisBuilder;
         }
 
-        public B withPictureKey(String pictureKey) {
-            Assumption.assertNotNull(pictureKey);
-
-            updateOptions.pictureKeyOption = UpdateOption.of(pictureKey);
-            return thisBuilder;
-        }
-
         public abstract T build();
 
     }

diff --git a/src/main/java/teammates/common/util/Config.java b/src/main/java/teammates/common/util/Config.java
@@ -15,9 +15,6 @@
  */
 public final class Config {
 
-    /** The value of the application URL, or null if no server instance is running. */
-    public static final String APP_URL;
-
     /** The value of the "app.id" in build.properties file. */
     public static final String APP_ID;
 
@@ -88,7 +85,6 @@ public final class Config {
     public static final boolean MAINTENANCE;
 
     static {
-        APP_URL = readAppUrl();
         Properties properties = new Properties();
         try (InputStream buildPropStream = FileHelper.getResourceAsStream("build.properties")) {
             properties.load(buildPropStream);
@@ -124,7 +120,7 @@ private Config() {
         // access static fields directly
     }
 
-    private static String readAppUrl() {
+    static String getBaseAppUrl() {
         ApiProxy.Environment serverEnvironment = ApiProxy.getCurrentEnvironment();
         if (serverEnvironment == null) {
             return null;
@@ -179,7 +175,7 @@ public static AppUrl getFrontEndAppUrl(String relativeUrl) {
      * {@code relativeUrl} must start with a "/".
      */
     private static AppUrl getBackEndAppUrl(String relativeUrl) {
-        return new AppUrl(APP_URL + relativeUrl);
+        return new AppUrl(getBaseAppUrl() + relativeUrl);
     }
 
     public static boolean isUsingSendgrid() {

diff --git a/src/main/java/teammates/common/util/GoogleCloudStorageHelper.java b/src/main/java/teammates/common/util/GoogleCloudStorageHelper.java