fix: authorize upload error on create job endpoint (#1642)

MobilityData · Jan 16, 2024 · 5384a3e · 5384a3e
1 parent e0d0f1f
commit 5384a3e
Show file tree

Hide file tree

Showing 5 changed files with 99 additions and 59 deletions.
diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/input/GtfsInput.java
@@ -27,12 +27,9 @@
 import java.util.zip.ZipInputStream;
 import org.apache.commons.compress.archivers.zip.ZipFile;
 import org.apache.commons.compress.utils.SeekableInMemoryByteChannel;
-import org.apache.http.client.methods.CloseableHttpResponse;
-import org.apache.http.client.methods.HttpGet;
-import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
 import org.mobilitydata.gtfsvalidator.notice.InvalidInputFilesInSubfolderNotice;
 import org.mobilitydata.gtfsvalidator.notice.NoticeContainer;
+import org.mobilitydata.gtfsvalidator.util.HttpGetUtil;
 
 /**
  * GtfsInput provides a common interface for reading GTFS data, either from a ZIP archive or from a
@@ -42,8 +39,6 @@ public abstract class GtfsInput implements Closeable {
   public static final String invalidInputMessage =
       "At least 1 GTFS file is in a subfolder. All GTFS files must reside at the root level directly.";
 
-  public static final String USER_AGENT_PREFIX = "MobilityData GTFS-Validator";
-
   /**
    * Creates a specific GtfsInput to read data from the given path.
    *
@@ -147,7 +142,7 @@ public static GtfsInput createFromUrl(
       Files.createDirectories(targetDirectory);
     }
     try (OutputStream outputStream = Files.newOutputStream(targetPath)) {
-      loadFromUrl(sourceUrl, outputStream, validatorVersion);
+      HttpGetUtil.loadFromUrl(sourceUrl, outputStream, validatorVersion);
     }
     return createFromPath(targetPath, noticeContainer);
   }
@@ -166,7 +161,7 @@ public static GtfsInput createFromUrlInMemory(
       URL sourceUrl, NoticeContainer noticeContainer, String validatorVersion)
       throws IOException, URISyntaxException {
     try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
-      loadFromUrl(sourceUrl, outputStream, validatorVersion);
+      HttpGetUtil.loadFromUrl(sourceUrl, outputStream, validatorVersion);
       File zipFile = new File(sourceUrl.toString());
       String fileName = zipFile.getName().replace(".zip", "");
       if (containsGtfsFileInSubfolder(
@@ -179,40 +174,6 @@ public static GtfsInput createFromUrlInMemory(
     }
   }
 
-  /**
-   * Downloads data from network.
-   *
-   * @param sourceUrl the fully qualified URL
-   * @param outputStream the output stream
-   * @param validatorVersion
-   * @throws IOException if no file could not be found at the specified location
-   * @throws URISyntaxException if URL is malformed
-   */
-  private static void loadFromUrl(URL sourceUrl, OutputStream outputStream, String validatorVersion)
-      throws IOException, URISyntaxException {
-    try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
-      HttpGet httpGet = new HttpGet(sourceUrl.toURI());
-      httpGet.setHeader("User-Agent", getUserAgent(validatorVersion));
-      try (CloseableHttpResponse httpResponse = httpClient.execute(httpGet)) {
-        httpResponse.getEntity().writeTo(outputStream);
-      }
-    }
-  }
-
-  /**
-   * @param validatorVersion version of the validator
-   * @return the user agent string in the format: "MobilityData GTFS-Validator/{validatorVersion}
-   *     (Java {java version})"
-   */
-  private static String getUserAgent(String validatorVersion) {
-    return USER_AGENT_PREFIX
-        + "/"
-        + (validatorVersion != null ? validatorVersion : "")
-        + " (Java "
-        + System.getProperty("java.version")
-        + ")";
-  }
-
   /**
    * Lists all files inside the GTFS dataset, even if they are not CSV and do not have .txt
    * extension.

diff --git a/core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java b/core/src/main/java/org/mobilitydata/gtfsvalidator/util/HttpGetUtil.java
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2023 MobilityData
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.mobilitydata.gtfsvalidator.util;
+
+import java.io.IOException;
+import java.io.OutputStream;
+import java.net.URISyntaxException;
+import java.net.URL;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClients;
+
+public class HttpGetUtil {
+
+  public static final String USER_AGENT_PREFIX = "MobilityData GTFS-Validator";
+
+  /**
+   * @param validatorVersion version of the validator
+   * @return the user agent string in the format: "MobilityData GTFS-Validator/{validatorVersion}
+   *     (Java {java version})"
+   */
+  public static String getUserAgent(String validatorVersion) {
+    return USER_AGENT_PREFIX
+        + "/"
+        + (validatorVersion != null ? validatorVersion : "")
+        + " (Java "
+        + System.getProperty("java.version")
+        + ")";
+  }
+
+  /**
+   * Downloads data from network.
+   *
+   * @param sourceUrl the fully qualified URL
+   * @param outputStream the output stream
+   * @param validatorVersion the version of the validator
+   * @throws IOException if no file could not be found at the specified location
+   * @throws URISyntaxException if URL is malformed
+   */
+  public static void loadFromUrl(URL sourceUrl, OutputStream outputStream, String validatorVersion)
+      throws IOException, URISyntaxException {
+    try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
+      HttpGet httpGet = new HttpGet(sourceUrl.toURI());
+      httpGet.setHeader("User-Agent", getUserAgent(validatorVersion));
+      try (CloseableHttpResponse httpResponse = httpClient.execute(httpGet)) {
+        httpResponse.getEntity().writeTo(outputStream);
+      }
+    }
+  }
+}
diff --git a/...main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java b/...main/java/org/mobilitydata/gtfsvalidator/web/service/controller/ValidationController.java
@@ -62,7 +62,8 @@ public CreateJobResponse createJob(@RequestBody CreateJobRequest body) {
           storageHelper.saveJobMetadata(new JobMetadata(jobId, body.getCountryCode()));
         }
         if (!Strings.isNullOrEmpty(body.getUrl())) {
-          storageHelper.saveJobFileFromUrl(jobId, body.getUrl());
+          var validatorVersion = versionResolver.resolveCurrentVersion();
+          storageHelper.saveJobFileFromUrl(jobId, body.getUrl(), validatorVersion.orElse(null));
         } else {
           uploadUrl = storageHelper.generateUniqueUploadUrl(jobId);
         }

diff --git a/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java b/web/service/src/main/java/org/mobilitydata/gtfsvalidator/web/service/util/StorageHelper.java
@@ -1,16 +1,19 @@
 package org.mobilitydata.gtfsvalidator.web.service.util;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.google.cloud.WriteChannel;
 import com.google.cloud.storage.*;
 import java.io.*;
 import java.net.URL;
+import java.nio.channels.Channels;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
+import org.mobilitydata.gtfsvalidator.util.HttpGetUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.context.ApplicationContext;
@@ -101,18 +104,20 @@ public JobMetadata getJobMetadata(String jobId) {
    *
    * @param jobId
    * @param url
+   * @param validatorVersion
    * @throws Exception
    */
-  public void saveJobFileFromUrl(String jobId, String url) throws Exception {
-    // Read file into memory
-    var urlInputStream = new BufferedInputStream(new URL(url).openStream());
-
-    // Upload to GCS
-    var blobId = BlobId.of(USER_UPLOAD_BUCKET_NAME, jobId + "/" + jobId + ".zip");
-    var mimeType = "application/zip";
-    var blobInfo = BlobInfo.newBuilder(blobId).setContentType(mimeType).build();
-    var fileBytes = urlInputStream.readAllBytes();
-    storage.create(blobInfo, fileBytes);
+  public void saveJobFileFromUrl(String jobId, String url, String validatorVersion)
+      throws Exception {
+    var blobId = BlobId.of(USER_UPLOAD_BUCKET_NAME, jobId + "/" + FILE_NAME);
+    var blobInfo = BlobInfo.newBuilder(blobId).setContentType("application/zip").build();
+    URL signedURL =
+        storage.signUrl(
+            blobInfo, 1, TimeUnit.HOURS, Storage.SignUrlOption.httpMethod(HttpMethod.POST));
+    try (WriteChannel writer = storage.writer(signedURL)) {
+      OutputStream outputStream = Channels.newOutputStream(writer);
+      HttpGetUtil.loadFromUrl(new URL(url), outputStream, validatorVersion);
+    }
   }
 
   /** Generates a job-specific signed URL for uploading a file to GCS. */

diff --git a/...est/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java b/...est/java/org/mobilitydata/gtfsvalidator/web/service/controller/CreateJobEndpointTest.java
@@ -4,9 +4,12 @@
 import static org.mockito.Mockito.*;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
 import java.net.URL;
+import java.util.Optional;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.mobilitydata.gtfsvalidator.util.VersionResolver;
 import org.mobilitydata.gtfsvalidator.web.service.util.JobMetadata;
 import org.mobilitydata.gtfsvalidator.web.service.util.StorageHelper;
 import org.mobilitydata.gtfsvalidator.web.service.util.ValidationHandler;
@@ -32,10 +35,14 @@ public class CreateJobEndpointTest {
 
   @Captor ArgumentCaptor<JobMetadata> jobMetadataCaptor;
 
+  @MockBean private VersionResolver versionResolver;
+
   private final ObjectMapper mapper = new ObjectMapper();
   private String testJobId;
   private String testUploadUrl;
 
+  private final String VALIDATOR_TEST_VERSION = "1.0.0";
+
   private void makeCreateJobRequestAndCheckResult(
       CreateJobRequest request, String expectedJobId, String expectedUploadUrl) throws Exception {
     var json = mapper.writeValueAsString(request);
@@ -51,10 +58,11 @@ private void makeCreateJobRequestAndCheckResult(
   }
 
   @BeforeEach
-  public void setUp() {
+  public void setUp() throws IOException {
     testJobId = "123";
     testUploadUrl = "https://gcs.io/bucket/123";
     doReturn(testJobId).when(storageHelper).createNewJobId();
+    doReturn(Optional.of(VALIDATOR_TEST_VERSION)).when(versionResolver).resolveCurrentVersion();
   }
 
   @Test
@@ -67,7 +75,7 @@ public void createJobWithNoUrlNoCountryCode() throws Exception {
     // should not call saveJobMetadata
     verify(storageHelper, times(0)).saveJobMetadata(any(JobMetadata.class));
     // should not call saveJobFileFromUrl
-    verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString());
+    verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString(), anyString());
   }
 
   @Test
@@ -85,7 +93,7 @@ public void createJobWithCountryCodeButNoUrl() throws Exception {
     assert jobMetadata.getCountryCode().equals("US");
 
     // should not call saveJobFileFromUrl
-    verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString());
+    verify(storageHelper, times(0)).saveJobFileFromUrl(anyString(), anyString(), anyString());
   }
 
   @Test
@@ -99,7 +107,7 @@ public void createJobWithUrlButNoCountryCode() throws Exception {
     // should not call saveJobMetadata
     verify(storageHelper, times(0)).saveJobMetadata(any(JobMetadata.class));
     // should saveJobFileFromUrl
-    verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url);
+    verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url, VALIDATOR_TEST_VERSION);
   }
 
   @Test
@@ -117,7 +125,7 @@ public void createJobWithUrlAndCountryCode() throws Exception {
     assert jobMetadata.getJobId().equals(testJobId);
     assert jobMetadata.getCountryCode().equals("US");
     // should saveJobFileFromUrl
-    verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url);
+    verify(storageHelper, times(1)).saveJobFileFromUrl(testJobId, url, VALIDATOR_TEST_VERSION);
   }
 
   @Test
@@ -140,7 +148,7 @@ public void createJobShouldReturn500ErrorIfSaveJobFileFromUrlThrowsException() t
     doReturn(testJobId).when(storageHelper).createNewJobId();
     doThrow(new RuntimeException("test exception"))
         .when(storageHelper)
-        .saveJobFileFromUrl(any(), any());
+        .saveJobFileFromUrl(anyString(), anyString(), anyString());
     String url = "http://myfilehost.com/myfile.zip";
     var request = new CreateJobRequest("US", url);
     var json = mapper.writeValueAsString(request);