BT-724 Fix BlobPathBuilder failing on retrieving existing filesystem (#…

…6816) Modify blobPathBuilder to fallback to creating a filesystem if one is not found
broadinstitute · Aug 5, 2022 · c2d59fb · c2d59fb
1 parent 2eb5843
commit c2d59fb
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 14 deletions.
diff --git a/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala b/filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobPathBuilder.scala
@@ -3,18 +3,14 @@ package cromwell.filesystems.blob
 import com.azure.core.credential.AzureSasCredential
 import com.azure.storage.blob.nio.AzureFileSystem
 import com.google.common.net.UrlEscapers
-import cromwell.core.path.NioPath
-import cromwell.core.path.Path
-import cromwell.core.path.PathBuilder
+import cromwell.core.path.{NioPath, Path, PathBuilder}
 import cromwell.filesystems.blob.BlobPathBuilder._
 
-import java.net.MalformedURLException
-import java.net.URI
-import java.nio.file.FileSystems
+import java.net.{MalformedURLException, URI}
+import java.nio.file.{FileSystem, FileSystemNotFoundException, FileSystems}
 import scala.jdk.CollectionConverters._
 import scala.language.postfixOps
-import scala.util.Failure
-import scala.util.Try
+import scala.util.{Failure, Try}
 
 object BlobPathBuilder {
 
@@ -64,14 +60,20 @@ class BlobPathBuilder(credential: AzureSasCredential, container: String, endpoin
   val fileSystemConfig: Map[String, Object] = Map((AzureFileSystem.AZURE_STORAGE_SAS_TOKEN_CREDENTIAL, credential),
                                                   (AzureFileSystem.AZURE_STORAGE_FILE_STORES, container))
 
+  def retrieveFilesystem(uri: URI): Try[FileSystem] = {
+    Try(FileSystems.getFileSystem(uri)) recover {
+      // If no filesystem already exists, this will create a new connection, with the provided configs
+      case _: FileSystemNotFoundException => FileSystems.newFileSystem(uri, fileSystemConfig.asJava)
+    }
+  }
+
   def build(string: String): Try[BlobPath] = {
     validateBlobPath(string, container, endpoint) match {
-      case ValidBlobPath(path) =>
-        Try {
-          val fileSystem = FileSystems.newFileSystem(new URI("azb://?endpoint=" + endpoint), fileSystemConfig.asJava)
-          val blobStoragePath = fileSystem.getPath(path)
-          BlobPath(blobStoragePath, endpoint, container)
-        }
+      case ValidBlobPath(path) => for {
+            fileSystem <- retrieveFilesystem(new URI("azb://?endpoint=" + endpoint))
+            nioPath <- Try(fileSystem.getPath(path))
+            blobPath = BlobPath(nioPath, endpoint, container)
+          } yield blobPath
       case UnparsableBlobPath(errorMessage: Throwable) => Failure(errorMessage)
     }
   }

diff --git a/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala b/filesystems/blob/src/test/scala/cromwell/filesystems/blob/BlobPathBuilderSpec.scala
@@ -62,4 +62,15 @@ class BlobPathBuilderSpec extends AnyFlatSpec with Matchers{
     val fileText = (is.readAllBytes.map(_.toChar)).mkString
     fileText should include ("This is my test file!!!! Did it work?")
   }
+
+  ignore should "build duplicate blob paths in the same filesystem" in {
+    val endpoint = BlobPathBuilderSpec.buildEndpoint("coaexternalstorage")
+    val store = "inputs"
+    val evalPath = "/test/inputFile.txt"
+    val sas = "{SAS TOKEN HERE}"
+    val testString = endpoint + "/" + store + evalPath
+    val blobPath1: BlobPath = new BlobPathBuilder(new AzureSasCredential(sas), store, endpoint) build testString getOrElse fail()
+    val blobPath2: BlobPath = new BlobPathBuilder(new AzureSasCredential(sas), store, endpoint) build testString getOrElse fail()
+    blobPath1 should equal(blobPath2)
+  }
 }