Skip to content

Commit

Permalink
more sensible mocks
Browse files Browse the repository at this point in the history
  • Loading branch information
THWiseman committed Nov 7, 2023
1 parent e7cd74c commit 510aa31
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -239,19 +239,20 @@ class WSMBlobSasTokenGenerator(wsmClientProvider: WorkspaceManagerApiClientProvi
/**
* Return a REST endpoint that will reply with a sas token for the blob storage container associated with the provided blob path.
* @param blobPath A blob path of a file living in a blob container that WSM knows about (likely a workspace container).
*
* @param tokenDuration How long will the token last after being generated. Default is 8 hours. Sas tokens won't last longer than 24h.
* NOTE: If a blobPath is provided for a file in a container other than what this token generator was constructed for,
* this function will make two REST requests. Otherwise, the relevant data is already cached locally.
*/
def getWSMSasFetchEndpoint(blobPath: BlobPath): Try[String] = {
def getWSMSasFetchEndpoint(blobPath: BlobPath, tokenDuration: Option[Duration] = None): Try[String] = {
val lifetimeSeconds: Int = tokenDuration.map(d => d.toSeconds.intValue).getOrElse(8*60*60)
val wsmEndpoint = wsmClientProvider.getBaseWorkspaceManagerUrl

Check warning on line 248 in filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala

View check run for this annotation

Codecov / codecov/patch

filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala#L247-L248

Added lines #L247 - L248 were not covered by tests
val terraInfo: Try[WSMTerraCoordinates] = for {
workspaceId <- parseTerraWorkspaceIdFromPath(blobPath)
containerResourceId <- getContainerResourceId(workspaceId, blobPath.container, None)
coordinates = WSMTerraCoordinates(wsmEndpoint, workspaceId, containerResourceId)
} yield coordinates
terraInfo.map{terraCoordinates =>

Check warning on line 254 in filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala

View check run for this annotation

Codecov / codecov/patch

filesystems/blob/src/main/scala/cromwell/filesystems/blob/BlobFileSystemManager.scala#L250-L254

Added lines #L250 - L254 were not covered by tests
s"${terraCoordinates.wsmEndpoint}/api/workspaces/v1/${terraCoordinates.workspaceId.toString}/resources/controlled/azure/storageContainer/${terraCoordinates.containerResourceId.toString}/getSasToken"
s"${terraCoordinates.wsmEndpoint}/api/workspaces/v1/${terraCoordinates.workspaceId.toString}/resources/controlled/azure/storageContainer/${terraCoordinates.containerResourceId.toString}/getSasToken?sasExpirationDuration=$lifetimeSeconds"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ import wom.values.WomFile

import java.io.FileNotFoundException
import java.nio.file.FileAlreadyExistsException
import java.time.Duration
import java.time.temporal.ChronoUnit
import scala.concurrent.Future
import scala.util.{Failure, Success, Try}
sealed trait TesRunStatus {
Expand Down Expand Up @@ -159,7 +161,7 @@ object TesAsyncBackendJobExecutionActor {
// We use the first blob file in the list to determine the correct blob container.
blobFiles.headOption.map{blobPath =>
blobPath.getFilesystemManager.blobTokenGenerator match {
case wsmGenerator: WSMBlobSasTokenGenerator => wsmGenerator.getWSMSasFetchEndpoint(blobPath)
case wsmGenerator: WSMBlobSasTokenGenerator => wsmGenerator.getWSMSasFetchEndpoint(blobPath, Some(Duration.of(24, ChronoUnit.HOURS)))
case _ => Failure(new UnsupportedOperationException("Blob file does not have an associated WSMBlobSasTokenGenerator"))

Check warning on line 165 in supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala#L163-L165

Added lines #L163 - L165 were not covered by tests
}
}.getOrElse(Failure(new NoSuchElementException("Could not infer blob storage container from task inputs: No valid blob files provided.")))

Check warning on line 167 in supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala

View check run for this annotation

Codecov / codecov/patch

supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesAsyncBackendJobExecutionActor.scala#L167

Added line #L167 was not covered by tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@ import org.scalatest.flatspec.AnyFlatSpec
import org.scalatest.matchers.should.Matchers

import java.nio.file.Path
import scala.util.{Failure, Success, Try}
import java.time.Duration
import java.time.temporal.ChronoUnit
import scala.util.{Failure, Try}

class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers with MockSugar {
behavior of "TesAsyncBackendJobExecutionActor"
Expand Down Expand Up @@ -63,6 +65,7 @@ class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers wit
val testWorkspaceId = "e58ed763-928c-4155-0000-fdbaaadc15f3"
val testContainerResourceId = "e58ed763-928c-4155-1111-fdbaaadc15f3"

//path to a blob file
def generateMockBlobPath: BlobPath = {
val mockCromwellPath: cromwell.core.path.Path = mock[cromwell.core.path.Path]
mockCromwellPath.normalize() returns mockCromwellPath
Expand All @@ -74,48 +77,53 @@ class TesAsyncBackendJobExecutionActorSpec extends AnyFlatSpec with Matchers wit
val mockBlobPath = mock[BlobPath]
mockBlobPath.nioPath returns mockNioPath
mockBlobPath.toAbsolutePath returns mockCromwellPath
mockBlobPath.md5 returns "MOCK_MD5"
mockBlobPath.md5 returns "BLOB_MD5"

val mockTokenGenerator: WSMBlobSasTokenGenerator = mock[WSMBlobSasTokenGenerator]
val mockFsm: BlobFileSystemManager = mock[BlobFileSystemManager]
mockTokenGenerator.getWSMSasFetchEndpoint(mockBlobPath) returns Try(s"$testWsmEndpoint/api/workspaces/v1/$testWorkspaceId/resources/controlled/azure/storageContainer/$testContainerResourceId/getSasToken")
val expectedTokenDuration: Duration = Duration.of(24, ChronoUnit.HOURS)
mockTokenGenerator.getWSMSasFetchEndpoint(mockBlobPath, Some(expectedTokenDuration)) returns Try(s"$testWsmEndpoint/api/workspaces/v1/$testWorkspaceId/resources/controlled/azure/storageContainer/$testContainerResourceId/getSasToken?sasExpirationDuration=${expectedTokenDuration.getSeconds.toInt}")
mockFsm.blobTokenGenerator returns mockTokenGenerator
mockBlobPath.getFilesystemManager returns mockFsm

mockBlobPath
}

def mockPathGetter(pathString: String): Try[cromwell.core.path.Path] = {
val mockBlobPath = generateMockBlobPath
val mockCromwellPath: cromwell.core.path.Path = mock[cromwell.core.path.Path]
val foundBlobPath: Success[BlobPath] = Success(mockBlobPath)
val foundNonBlobPath: Success[cromwell.core.path.Path] = Success(mockCromwellPath)
if (pathString.equals(blobInput_0.url.get) || pathString.equals(blobInput_1.url.get)) return foundBlobPath
foundNonBlobPath
//Path to a file that isn't a blob file
def generateMockDefaultPath: cromwell.core.path.Path = {
val mockDefaultPath: cromwell.core.path.Path = mock[cromwell.core.path.Path]
mockDefaultPath.pathAsString returns someNotBlobUrl
mockDefaultPath.md5 returns "DEFAULT_MD5"
mockDefaultPath
}
def pathGetter(pathString: String): Try[cromwell.core.path.Path] = {
if(pathString.contains(someBlobUrl)) Try(generateMockBlobPath) else Try(generateMockDefaultPath)
}

def mockBlobConverter(pathToConvert: Try[cromwell.core.path.Path]): Try[BlobPath] = {
//using a stubbed md5 rather than matching on type because type matching of mocked types at runtime causes problems
if (pathToConvert.get.md5.equals("MOCK_MD5")) pathToConvert.asInstanceOf[Try[BlobPath]] else Failure(new Exception("failed"))
def blobConverter(pathToConvert: Try[cromwell.core.path.Path]): Try[BlobPath] = {
// Cromwell matches on (sub)type at runtime to determine if BlobPath or not.
// That doesn't work with mocks, so we use stubbed md5 for testing.
if(pathToConvert.get.md5.equals("BLOB_MD5")) Try(generateMockBlobPath) else Failure(new Exception("failed"))
}

it should "not return sas endpoint when no blob paths are provided" in {
val mockLogger: JobLogger = mock[JobLogger]
val emptyInputs: List[Input] = List()
val bloblessInputs: List[Input] = List(notBlobInput_1, notBlobInput_2)
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(emptyInputs, mockPathGetter, mockLogger, mockBlobConverter).isFailure shouldBe true
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(bloblessInputs, mockPathGetter, mockLogger, mockBlobConverter).isFailure shouldBe true
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(emptyInputs, pathGetter, mockLogger, blobConverter).isFailure shouldBe true
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(bloblessInputs, pathGetter, mockLogger, blobConverter).isFailure shouldBe true
}

it should "return a sas endpoint based on inputs when blob paths are provided" in {
val mockLogger: JobLogger = mock[JobLogger]
val expected = s"$testWsmEndpoint/api/workspaces/v1/$testWorkspaceId/resources/controlled/azure/storageContainer/$testContainerResourceId/getSasToken"
val expectedTokenLifetimeSeconds = 24 * 60 * 60 //assert that cromwell asks for 24h token duration.
val expected = s"$testWsmEndpoint/api/workspaces/v1/$testWorkspaceId/resources/controlled/azure/storageContainer/$testContainerResourceId/getSasToken?sasExpirationDuration=${expectedTokenLifetimeSeconds}"
val blobInput: List[Input] = List(blobInput_0)
val blobInputs: List[Input] = List(blobInput_0, blobInput_1)
val mixedInputs: List[Input] = List(notBlobInput_1, blobInput_0, blobInput_1)
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInput, mockPathGetter, mockLogger, mockBlobConverter).get shouldEqual expected
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInputs, mockPathGetter, mockLogger, mockBlobConverter).get shouldEqual expected
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(mixedInputs, mockPathGetter, mockLogger, mockBlobConverter).get shouldEqual expected
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInput, pathGetter, mockLogger, blobConverter).get shouldEqual expected
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(blobInputs, pathGetter, mockLogger, blobConverter).get shouldEqual expected
TesAsyncBackendJobExecutionActor.determineWSMSasEndpointFromInputs(mixedInputs, pathGetter, mockLogger, blobConverter).get shouldEqual expected
}

it should "contain expected strings in the bash script" in {
Expand Down

0 comments on commit 510aa31

Please sign in to comment.