fix: Deadlong fix with song and score

When song tries to publish, it calls the download endpoint on score. Since the analysis is currently UNPUBLISHED and score will only allow downloads for PUBLISHED files, therefore song cannot publish because score only allows published files to be download. Fix was to add a new path param, that will return object spec WITHOUT urls.
overture-stack · Feb 13, 2019 · 3ea9927 · 3ea9927
1 parent d35bfd2
commit 3ea9927
Show file tree

Hide file tree

Showing 6 changed files with 71 additions and 58 deletions.
diff --git a/score-server/src/main/java/bio/overture/score/server/controller/DownloadController.java b/score-server/src/main/java/bio/overture/score/server/controller/DownloadController.java
@@ -17,12 +17,13 @@
  */
 package bio.overture.score.server.controller;
 
-import javax.servlet.http.HttpServletRequest;
-
 import bio.overture.score.core.model.ObjectSpecification;
-import bio.overture.score.server.util.HttpServletRequests;
 import bio.overture.score.server.repository.DownloadService;
 import bio.overture.score.server.security.TokenHasher;
+import bio.overture.score.server.util.HttpServletRequests;
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+import lombok.val;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Profile;
 import org.springframework.http.HttpHeaders;
@@ -39,9 +40,7 @@
 import org.springframework.web.bind.annotation.ResponseBody;
 import org.springframework.web.bind.annotation.RestController;
 
-import lombok.Setter;
-import lombok.val;
-import lombok.extern.slf4j.Slf4j;
+import javax.servlet.http.HttpServletRequest;
 
 /**
  * A controller to expose RESTful API for download
@@ -76,14 +75,15 @@ public class DownloadController {
       @RequestParam(value = "offset", required = true) long offset,
       @RequestParam(value = "length", required = true) long length,
       @RequestParam(value = "external", defaultValue = "false") boolean external,
+      @RequestParam(value = "exclude-urls", defaultValue = "false") boolean excludeUrls,
       @RequestHeader(value = "User-Agent", defaultValue = "unknown") String userAgent,
       HttpServletRequest request) {
 
     val ipAddress = HttpServletRequests.getIpAddress(request);
 
     log.info("Requesting download of object id {} with access token {} (MD5) from {} and client version {}", objectId,
         identifier(accessToken), ipAddress, userAgent);
-    return downloadService.download(objectId, offset, length, external);
+    return downloadService.download(objectId, offset, length, external, excludeUrls);
   }
 
   protected String identifier(String accessToken) {

diff --git a/score-server/src/main/java/bio/overture/score/server/repository/DownloadService.java b/score-server/src/main/java/bio/overture/score/server/repository/DownloadService.java
@@ -21,7 +21,7 @@
 
 public interface DownloadService {
 
-  ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse);
+  ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse, boolean excludeUrls);
 
   /**
    * Attempts to fetch a pre-defined object id (defined in application.yml) from the object repository. Used to confirm

diff --git a/...server/src/main/java/bio/overture/score/server/repository/azure/AzureDownloadService.java b/...server/src/main/java/bio/overture/score/server/repository/azure/AzureDownloadService.java
@@ -68,7 +68,7 @@ public class AzureDownloadService implements DownloadService {
   private String sentinelObjectId;
 
   @Override
-  public ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse) {
+  public ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse, boolean excludeUrls) {
     try {
       checkArgument(offset >= 0L);
 

diff --git a/score-server/src/main/java/bio/overture/score/server/repository/s3/S3DownloadService.java b/score-server/src/main/java/bio/overture/score/server/repository/s3/S3DownloadService.java
@@ -17,47 +17,46 @@
  */
 package bio.overture.score.server.repository.s3;
 
-import static bio.overture.score.server.metadata.MetadataService.getAnalysisId;
-import static com.google.common.base.Preconditions.checkArgument;
-
-import bio.overture.score.server.metadata.MetadataEntity;
-import bio.overture.score.server.metadata.MetadataService;
-import lombok.Cleanup;
-import lombok.Setter;
-import lombok.val;
-import lombok.extern.slf4j.Slf4j;
-
-import java.io.IOException;
-import java.time.LocalDateTime;
-import java.time.ZoneId;
-import java.util.Date;
-import java.util.List;
-
 import bio.overture.score.core.model.ObjectKey;
 import bio.overture.score.core.model.ObjectSpecification;
 import bio.overture.score.core.model.Part;
 import bio.overture.score.core.util.ObjectKeys;
+import bio.overture.score.core.util.PartCalculator;
 import bio.overture.score.server.exception.IdNotFoundException;
 import bio.overture.score.server.exception.InternalUnrecoverableError;
 import bio.overture.score.server.exception.NotRetryableException;
 import bio.overture.score.server.exception.RetryableException;
-import bio.overture.score.server.repository.URLGenerator;
+import bio.overture.score.server.metadata.MetadataEntity;
+import bio.overture.score.server.metadata.MetadataService;
 import bio.overture.score.server.repository.BucketNamingService;
 import bio.overture.score.server.repository.DownloadService;
-import bio.overture.score.core.util.PartCalculator;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.annotation.Value;
-import org.springframework.context.annotation.Profile;
-import org.springframework.http.HttpStatus;
-import org.springframework.stereotype.Service;
-
+import bio.overture.score.server.repository.URLGenerator;
 import com.amazonaws.AmazonServiceException;
 import com.amazonaws.services.s3.AmazonS3;
 import com.amazonaws.services.s3.model.GetObjectRequest;
 import com.amazonaws.services.s3.model.S3Object;
 import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.JsonMappingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import lombok.Cleanup;
+import lombok.NonNull;
+import lombok.Setter;
+import lombok.extern.slf4j.Slf4j;
+import lombok.val;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.Profile;
+import org.springframework.http.HttpStatus;
+import org.springframework.stereotype.Service;
+
+import java.io.IOException;
+import java.time.LocalDateTime;
+import java.time.ZoneId;
+import java.util.Date;
+import java.util.List;
+
+import static bio.overture.score.server.metadata.MetadataService.getAnalysisId;
+import static com.google.common.base.Preconditions.checkArgument;
 
 /**
  * service responsible for object download (full or partial)
@@ -100,10 +99,13 @@ public class S3DownloadService implements DownloadService {
   @Autowired
   private MetadataService metadataService;
 
+
   @Override
-  public ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse) {
+  public ObjectSpecification download(String objectId, long offset, long length, boolean forExternalUse, boolean excludeUrls) {
     try {
-      checkPublishedAnalysisState(metadataService.getEntity(objectId));
+      if (!excludeUrls){
+        checkPublishedAnalysisState(metadataService.getEntity(objectId));
+      }
 
       checkArgument(offset > -1L);
 
@@ -112,7 +114,7 @@ public ObjectSpecification download(String objectId, long offset, long length, b
 
       // Short-circuit in default case
       if (!forExternalUse && (offset == 0L && length < 0L)) {
-        return objectSpec;
+        return excludeUrls ? removeUrls(objectSpec) : objectSpec;
       }
 
       // Calculate range values
@@ -142,24 +144,32 @@ public ObjectSpecification download(String objectId, long offset, long length, b
 
       fillPartUrls(objectKey, parts, objectSpec.isRelocated(), forExternalUse);
 
-      return new ObjectSpecification(objectKey.getKey(), objectId, objectId, parts, length, objectSpec.getObjectMd5(),
+      val spec = new ObjectSpecification(objectKey.getKey(), objectId, objectId, parts, length, objectSpec.getObjectMd5(),
           objectSpec.isRelocated());
+
+      return excludeUrls ? removeUrls(spec) : spec;
+
     } catch (Exception e) {
-      log.error("Failed to download objectId: {}, offset: {}, length: {}, forExternalUse: {}: {} ",
-          objectId, offset, length, forExternalUse, e);
+      log.error("Failed to download objectId: {}, offset: {}, length: {}, forExternalUse: {}, excludeUrls: {} : {} ",
+          objectId, offset, length, forExternalUse, excludeUrls, e);
 
       throw e;
     }
   }
 
+  private static ObjectSpecification removeUrls(ObjectSpecification spec){
+    spec.getParts().forEach(x -> x.setUrl(null));
+    return spec;
+  }
+
   void checkPublishedAnalysisState(MetadataEntity entity){
     if(!useLegacyMode){
       val objectId = entity.getId();
       val analysisState = metadataService.getAnalysisStateForMetadata(entity);
       if (!analysisState.equals(PUBLISHED_ANALYSIS_STATE)){
         val message = String.format("Critical Error: cannot complete download for objectId '%s' with "
                         + "analysisState '%s' and analysisId '%s'. "
-                        + "Can only download objects that have the analysisState '%s'. Update the file metadata and retry.",
+                        + "Can only download objects that have the analysisState '%s' or when the 'exclude-urls=true' flag is set. Update the file metadata or url parameters and retry.",
                 objectId, analysisState, getAnalysisId(entity), PUBLISHED_ANALYSIS_STATE);
         log.error(message); // Log to audit log file
         throw new NotRetryableException(new IllegalStateException(message));

diff --git a/...e-server/src/test/java/bio/overture/score/server/repository/s3/S3DownloadServiceTest.java b/...e-server/src/test/java/bio/overture/score/server/repository/s3/S3DownloadServiceTest.java
@@ -9,8 +9,10 @@
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.runners.MockitoJUnitRunner;
+
 import static java.lang.String.format;
-import static org.assertj.core.api.Assertions.*;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -50,11 +52,11 @@ public void test_if_unpublished_file_triggers_error() {
     }
 
     @Test
-    public void verify_if_download_unpublished_objectId_is_blocked() {
+    public void verify_if_download_is_blocked_with_unpublished_objectId() {
         when(mockService.getAnalysisStateForMetadata(metadataEntity)).thenReturn("UNPUBLISHED");
         when(mockService.getEntity(objectId)).thenReturn(metadataEntity);
 
-        val throwable = catchThrowable(() -> s3DownloadService.download(objectId, 0, -1, false));
+        val throwable = catchThrowable(() -> s3DownloadService.download(objectId, 0, -1, false, false));
         assertThat(throwable).isExactlyInstanceOf(NotRetryableException.class);
     }
 

diff --git a/...r/src/test/java/bio/overture/score/server/service/download/ObjectDownloadServiceTest.java b/...r/src/test/java/bio/overture/score/server/service/download/ObjectDownloadServiceTest.java
@@ -17,22 +17,19 @@
  */
 package bio.overture.score.server.service.download;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-import static org.mockito.Mockito.*;
-
-import java.net.URL;
-import java.util.regex.Pattern;
-
 import bio.overture.score.core.util.ObjectKeys;
+import bio.overture.score.core.util.SimplePartCalculator;
 import bio.overture.score.server.config.ServerConfig;
 import bio.overture.score.server.exception.IdNotFoundException;
-import bio.overture.score.core.util.SimplePartCalculator;
 import bio.overture.score.server.metadata.MetadataEntity;
 import bio.overture.score.server.metadata.MetadataService;
 import bio.overture.score.server.repository.s3.S3BucketNamingService;
 import bio.overture.score.server.repository.s3.S3DownloadService;
 import bio.overture.score.server.repository.s3.S3URLGenerator;
+import com.amazonaws.AmazonServiceException;
+import com.amazonaws.services.s3.AmazonS3;
+import com.google.common.base.Splitter;
+import lombok.val;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -43,11 +40,15 @@
 import org.springframework.test.context.ContextConfiguration;
 import org.springframework.test.util.ReflectionTestUtils;
 
-import com.amazonaws.AmazonServiceException;
-import com.amazonaws.services.s3.AmazonS3;
-import com.google.common.base.Splitter;
+import java.net.URL;
+import java.util.regex.Pattern;
 
-import lombok.val;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
 
 @RunWith(MockitoJUnitRunner.class)
 @ContextConfiguration(classes = ServerConfig.class)
@@ -114,7 +115,7 @@ public void it_takes_two_to_fail_with_not_found() {
     firstException.setStatusCode(HttpStatus.NOT_FOUND.value());
     when(s3Client.getObject(Mockito.any())).thenThrow(firstException, firstException); // stubs first two calls to
                                                                                        // s3Client.getObject()
-    service.download(objectId, 0, 1000, false);
+    service.download(objectId, 0, 1000, false, false);
   }
 
   @Test
@@ -148,7 +149,7 @@ public void verify_fallback_in_download_presigned_urls() throws Exception {
     val sut = spy(service);
     doReturn(os).when(sut).getSpecification(objectId);
 
-    val objSpec = sut.download(objectId, 0, 104857600, false);
+    val objSpec = sut.download(objectId, 0, 104857600, false, false);
 
     val p = objSpec.getParts().get(0);
 
@@ -194,7 +195,7 @@ public void verify_partitioned_buckets_in_download_presigned_urls() throws Excep
     val sut = spy(service);
     doReturn(os).when(sut).getSpecification(objectId);
 
-    val objSpec = sut.download(objectId, 0, 104857600, false);
+    val objSpec = sut.download(objectId, 0, 104857600, false, false);
 
     val p = objSpec.getParts().get(0);