From ee9febde6cbb23a976852b41accdacbc1badecfb Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Fri, 2 Jun 2017 19:09:46 -0700 Subject: Return map of authorized permissions --- src/test/scala/xyz/driver/core/AuthTest.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/test/scala/xyz') diff --git a/src/test/scala/xyz/driver/core/AuthTest.scala b/src/test/scala/xyz/driver/core/AuthTest.scala index bf776df..9018a3e 100644 --- a/src/test/scala/xyz/driver/core/AuthTest.scala +++ b/src/test/scala/xyz/driver/core/AuthTest.scala @@ -37,7 +37,7 @@ class AuthTest extends FlatSpec with Matchers with MockitoSugar with ScalatestRo override def userHasPermissions(user: User, permissions: Seq[Permission])( implicit ctx: ServiceRequestContext): Future[AuthorizationResult] = { - val authorized = permissions.forall(_ === TestRoleAllowedPermission) + val authorized = permissions.map(p => p -> (p === TestRoleAllowedPermission)).toMap Future.successful(AuthorizationResult(authorized, ctx.permissionsToken)) } } -- cgit v1.2.3 From f6a67a8400e12de6e0ae83d0b4c744e07fc4901c Mon Sep 17 00:00:00 2001 From: Jakob Odersky Date: Mon, 19 Jun 2017 11:40:34 -0700 Subject: Check for null values in gcs directory listing GCS: when listing, always assume the path is a directory GCS: fix unit test --- .../scala/xyz/driver/core/file/GcsStorage.scala | 22 ++++++++++++++-------- src/main/scala/xyz/driver/core/file/package.scala | 3 ++- src/test/scala/xyz/driver/core/FileTest.scala | 3 +-- 3 files changed, 17 insertions(+), 11 deletions(-) (limited to 'src/test/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/file/GcsStorage.scala b/src/main/scala/xyz/driver/core/file/GcsStorage.scala index 6c2746e..0d8b918 100644 --- a/src/main/scala/xyz/driver/core/file/GcsStorage.scala +++ b/src/main/scala/xyz/driver/core/file/GcsStorage.scala @@ -52,24 +52,30 @@ class GcsStorage(storageClient: Storage, bucketName: Name[Bucket], executionCont storageClient.delete(BlobId.of(bucketName.value, filePath.toString)) } - override def list(path: Path): ListT[Future, FileLink] = + override def list(directoryPath: Path): ListT[Future, FileLink] = ListT.listT(Future { val page = storageClient.list( bucketName.value, BlobListOption.currentDirectory(), - BlobListOption.prefix(path.toString) + BlobListOption.prefix(s"$directoryPath/") ) - page.iterateAll().asScala.map(blobToFileLink(path, _)).toList + page.iterateAll().asScala.map(blobToFileLink(directoryPath, _)).toList }) protected def blobToFileLink(path: Path, blob: Blob): FileLink = { + def nullError(property: String) = throw new IllegalStateException(s"Blob $blob at $path does not have $property") + val name = Option(blob.getName).getOrElse(nullError("a name")) + val generation = Option(blob.getGeneration).getOrElse(nullError("a generation")) + val updateTime = Option(blob.getUpdateTime).getOrElse(nullError("an update time")) + val size = Option(blob.getSize).getOrElse(nullError("a size")) + FileLink( - Name(blob.getName), - Paths.get(path.toString, blob.getName), - Revision(blob.getGeneration.toString), - Time(blob.getUpdateTime), - blob.getSize + Name(name), + Paths.get(path.toString, name), + Revision(generation.toString), + Time(updateTime), + size ) } diff --git a/src/main/scala/xyz/driver/core/file/package.scala b/src/main/scala/xyz/driver/core/file/package.scala index 9000894..7203207 100644 --- a/src/main/scala/xyz/driver/core/file/package.scala +++ b/src/main/scala/xyz/driver/core/file/package.scala @@ -37,7 +37,8 @@ package file { def delete(filePath: Path): Future[Unit] - def list(path: Path): ListT[Future, FileLink] + /** List contents of a directory */ + def list(directoryPath: Path): ListT[Future, FileLink] /** List of characters to avoid in S3 (I would say file names in general) * diff --git a/src/test/scala/xyz/driver/core/FileTest.scala b/src/test/scala/xyz/driver/core/FileTest.scala index a1b0329..c35eb5b 100644 --- a/src/test/scala/xyz/driver/core/FileTest.scala +++ b/src/test/scala/xyz/driver/core/FileTest.scala @@ -150,8 +150,7 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar { Iterator[Blob](blobMock).asJava, Iterator[Blob]().asJava ) - when( - gcsMock.list(testBucket.value, BlobListOption.currentDirectory(), BlobListOption.prefix(testDirPath.toString))) + when(gcsMock.list(testBucket.value, BlobListOption.currentDirectory(), BlobListOption.prefix(s"$testDirPath/"))) .thenReturn(pageMock) val filesBefore = Await.result(gcsStorage.list(testDirPath).run, 10 seconds) -- cgit v1.2.3 From c5bc3df97dd7670d6271efe440068b44f6521562 Mon Sep 17 00:00:00 2001 From: Zach Smith Date: Wed, 21 Jun 2017 16:31:21 -0700 Subject: Add minLength attribute to generators --- src/main/scala/xyz/driver/core/generators.scala | 29 ++++++++++++---------- .../scala/xyz/driver/core/GeneratorsTest.scala | 6 ++--- 2 files changed, 19 insertions(+), 16 deletions(-) (limited to 'src/test/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/generators.scala b/src/main/scala/xyz/driver/core/generators.scala index 8f3ff13..9242fd9 100644 --- a/src/main/scala/xyz/driver/core/generators.scala +++ b/src/main/scala/xyz/driver/core/generators.scala @@ -19,7 +19,7 @@ object generators { def nextToken(length: Int): String = List.fill(length)(oneOf(NonAmbigiousCharacters)).mkString - def nextInt(maxValue: Int): Int = random.nextInt(maxValue) + def nextInt(maxValue: Int, minValue: Int = 0): Int = random.nextInt(maxValue - minValue) + minValue def nextBoolean(): Boolean = random.nextBoolean() @@ -67,21 +67,24 @@ object generators { def oneOf[T](items: Set[T]): T = items.toSeq(nextInt(items.size)) - def arrayOf[T: ClassTag](generator: => T, maxLength: Int = DefaultMaxLength): Array[T] = - Array.fill(nextInt(maxLength))(generator) + def arrayOf[T: ClassTag](generator: => T, maxLength: Int = DefaultMaxLength, minLength: Int = 0): Array[T] = + Array.fill(nextInt(maxLength, minLength))(generator) - def seqOf[T](generator: => T, maxLength: Int = DefaultMaxLength): Seq[T] = - Seq.fill(nextInt(maxLength))(generator) + def seqOf[T](generator: => T, maxLength: Int = DefaultMaxLength, minLength: Int = 0): Seq[T] = + Seq.fill(nextInt(maxLength, minLength))(generator) - def vectorOf[T](generator: => T, maxLength: Int = DefaultMaxLength): Vector[T] = - Vector.fill(nextInt(maxLength))(generator) + def vectorOf[T](generator: => T, maxLength: Int = DefaultMaxLength, minLength: Int = 0): Vector[T] = + Vector.fill(nextInt(maxLength, minLength))(generator) - def listOf[T](generator: => T, maxLength: Int = DefaultMaxLength): List[T] = - List.fill(nextInt(maxLength))(generator) + def listOf[T](generator: => T, maxLength: Int = DefaultMaxLength, minLength: Int = 0): List[T] = + List.fill(nextInt(maxLength, minLength))(generator) - def setOf[T](generator: => T, maxLength: Int = DefaultMaxLength): Set[T] = - seqOf(generator, maxLength).toSet + def setOf[T](generator: => T, maxLength: Int = DefaultMaxLength, minLength: Int = 0): Set[T] = + seqOf(generator, maxLength, minLength).toSet - def mapOf[K, V](maxLength: Int, keyGenerator: => K, valueGenerator: => V): Map[K, V] = - seqOf(nextPair(keyGenerator, valueGenerator), maxLength).toMap + def mapOf[K, V](keyGenerator: => K, + valueGenerator: => V, + maxLength: Int = DefaultMaxLength, + minLength: Int = 0): Map[K, V] = + seqOf(nextPair(keyGenerator, valueGenerator), maxLength, minLength).toMap } diff --git a/src/test/scala/xyz/driver/core/GeneratorsTest.scala b/src/test/scala/xyz/driver/core/GeneratorsTest.scala index 4ec73ec..737cbcb 100644 --- a/src/test/scala/xyz/driver/core/GeneratorsTest.scala +++ b/src/test/scala/xyz/driver/core/GeneratorsTest.scala @@ -212,19 +212,19 @@ class GeneratorsTest extends FlatSpec with Matchers with Assertions { it should "be able to generate maps with keys and values generated by generators" in { - val generatedConstantMap = mapOf(10, "key", 123) + val generatedConstantMap = mapOf("key", 123, 10) generatedConstantMap.size should be <= 1 assert(generatedConstantMap.keys.forall(_ == "key")) assert(generatedConstantMap.values.forall(_ == 123)) - val generatedMap = mapOf(10, nextString(10), nextBigDecimal()) + val generatedMap = mapOf(nextString(10), nextBigDecimal(), 10) assert(generatedMap.keys.forall(_.length <= 10)) assert(generatedMap.values.forall(_ >= BigDecimal(0.00))) } it should "compose deeply" in { - val generatedNestedMap = mapOf(10, nextString(10), nextPair(nextBigDecimal(), nextOption(123))) + val generatedNestedMap = mapOf(nextString(10), nextPair(nextBigDecimal(), nextOption(123)), 10) generatedNestedMap.size should be <= 10 generatedNestedMap.keySet.size should be <= 10 -- cgit v1.2.3 From e41050b75308aab2736eea11b67bf9387a90dfe5 Mon Sep 17 00:00:00 2001 From: Jakob Odersky Date: Wed, 21 Jun 2017 15:06:20 -0700 Subject: Add file existence checking to file storage --- .../xyz/driver/core/file/FileSystemStorage.scala | 15 ++++++++----- .../scala/xyz/driver/core/file/GcsStorage.scala | 9 ++++++++ .../scala/xyz/driver/core/file/S3Storage.scala | 13 +++++++---- src/main/scala/xyz/driver/core/file/package.scala | 2 ++ src/test/scala/xyz/driver/core/FileTest.scala | 26 ++++++++++++++++++++++ 5 files changed, 56 insertions(+), 9 deletions(-) (limited to 'src/test/scala/xyz') diff --git a/src/main/scala/xyz/driver/core/file/FileSystemStorage.scala b/src/main/scala/xyz/driver/core/file/FileSystemStorage.scala index bfe6995..fab1307 100644 --- a/src/main/scala/xyz/driver/core/file/FileSystemStorage.scala +++ b/src/main/scala/xyz/driver/core/file/FileSystemStorage.scala @@ -1,7 +1,7 @@ package xyz.driver.core.file import java.io.File -import java.nio.file.{Path, Paths} +import java.nio.file.{Files, Path, Paths} import xyz.driver.core.{Name, Revision} import xyz.driver.core.time.Time @@ -12,7 +12,7 @@ import scalaz.{ListT, OptionT} class FileSystemStorage(executionContext: ExecutionContext) extends FileStorage { implicit private val execution = executionContext - def upload(localSource: File, destination: Path): Future[Unit] = Future { + override def upload(localSource: File, destination: Path): Future[Unit] = Future { checkSafeFileName(destination) { val destinationFile = destination.toFile @@ -28,12 +28,12 @@ class FileSystemStorage(executionContext: ExecutionContext) extends FileStorage } } - def download(filePath: Path): OptionT[Future, File] = + override def download(filePath: Path): OptionT[Future, File] = OptionT.optionT(Future { Option(new File(filePath.toString)).filter(file => file.exists() && file.isFile) }) - def delete(filePath: Path): Future[Unit] = Future { + override def delete(filePath: Path): Future[Unit] = Future { val file = new File(filePath.toString) if (file.delete()) () else { @@ -41,7 +41,7 @@ class FileSystemStorage(executionContext: ExecutionContext) extends FileStorage } } - def list(path: Path): ListT[Future, FileLink] = + override def list(path: Path): ListT[Future, FileLink] = ListT.listT(Future { val file = new File(path.toString) if (file.isDirectory) { @@ -54,4 +54,9 @@ class FileSystemStorage(executionContext: ExecutionContext) extends FileStorage } } else List.empty[FileLink] }) + + override def exists(path: Path): Future[Boolean] = Future { + Files.exists(path) + } + } diff --git a/src/main/scala/xyz/driver/core/file/GcsStorage.scala b/src/main/scala/xyz/driver/core/file/GcsStorage.scala index 0d8b918..deb8a0e 100644 --- a/src/main/scala/xyz/driver/core/file/GcsStorage.scala +++ b/src/main/scala/xyz/driver/core/file/GcsStorage.scala @@ -79,6 +79,15 @@ class GcsStorage(storageClient: Storage, bucketName: Name[Bucket], executionCont ) } + override def exists(path: Path): Future[Boolean] = Future { + val blob = Option( + storageClient.get( + bucketName.value, + path.toString + )) + blob.isDefined + } + override def signedFileUrl(filePath: Path, duration: Duration): OptionT[Future, URL] = OptionT.optionT(Future { Option(storageClient.get(bucketName.value, filePath.toString)).filterNot(_.getSize == 0).map { blob => diff --git a/src/main/scala/xyz/driver/core/file/S3Storage.scala b/src/main/scala/xyz/driver/core/file/S3Storage.scala index 933b01a..7df3db2 100644 --- a/src/main/scala/xyz/driver/core/file/S3Storage.scala +++ b/src/main/scala/xyz/driver/core/file/S3Storage.scala @@ -15,13 +15,13 @@ import scalaz.{ListT, OptionT} class S3Storage(s3: AmazonS3, bucket: Name[Bucket], executionContext: ExecutionContext) extends FileStorage { implicit private val execution = executionContext - def upload(localSource: File, destination: Path): Future[Unit] = Future { + override def upload(localSource: File, destination: Path): Future[Unit] = Future { checkSafeFileName(destination) { val _ = s3.putObject(bucket.value, destination.toString, localSource).getETag } } - def download(filePath: Path): OptionT[Future, File] = + override def download(filePath: Path): OptionT[Future, File] = OptionT.optionT(Future { val tempDir = System.getProperty("java.io.tmpdir") val randomFolderName = randomUUID().toString @@ -36,11 +36,11 @@ class S3Storage(s3: AmazonS3, bucket: Name[Bucket], executionContext: ExecutionC } }) - def delete(filePath: Path): Future[Unit] = Future { + override def delete(filePath: Path): Future[Unit] = Future { s3.deleteObject(bucket.value, filePath.toString) } - def list(path: Path): ListT[Future, FileLink] = + override def list(path: Path): ListT[Future, FileLink] = ListT.listT(Future { import scala.collection.JavaConverters._ val req = new ListObjectsV2Request().withBucketName(bucket.value).withPrefix(path.toString).withMaxKeys(2) @@ -63,4 +63,9 @@ class S3Storage(s3: AmazonS3, bucket: Name[Bucket], executionContext: ExecutionC } filterNot isInSubFolder(path) } toList }) + + override def exists(path: Path): Future[Boolean] = Future { + s3.doesObjectExist(bucket.value, path.toString) + } + } diff --git a/src/main/scala/xyz/driver/core/file/package.scala b/src/main/scala/xyz/driver/core/file/package.scala index 7203207..b2c679e 100644 --- a/src/main/scala/xyz/driver/core/file/package.scala +++ b/src/main/scala/xyz/driver/core/file/package.scala @@ -40,6 +40,8 @@ package file { /** List contents of a directory */ def list(directoryPath: Path): ListT[Future, FileLink] + def exists(path: Path): Future[Boolean] + /** List of characters to avoid in S3 (I would say file names in general) * * @see http://stackoverflow.com/questions/7116450/what-are-valid-s3-key-names-that-can-be-accessed-via-the-s3-rest-api diff --git a/src/test/scala/xyz/driver/core/FileTest.scala b/src/test/scala/xyz/driver/core/FileTest.scala index c35eb5b..246cd95 100644 --- a/src/test/scala/xyz/driver/core/FileTest.scala +++ b/src/test/scala/xyz/driver/core/FileTest.scala @@ -54,16 +54,25 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar { when(amazonS3Mock.listObjectsV2(any[ListObjectsV2Request]())).thenReturn(s3ResultsMock) when(amazonS3Mock.putObject(testBucket.value, testFilePath.toString, sourceTestFile)).thenReturn(s3PutMock) when(amazonS3Mock.getObject(any[GetObjectRequest](), any[File]())).thenReturn(s3ObjectMetadataMock) + when(amazonS3Mock.doesObjectExist(testBucket.value, testFilePath.toString)).thenReturn( + false, // before file is uploaded + true // after file is uploaded + ) val s3Storage = new S3Storage(amazonS3Mock, testBucket, scala.concurrent.ExecutionContext.global) val filesBefore = Await.result(s3Storage.list(testDirPath).run, 10 seconds) filesBefore shouldBe empty + val fileExistsBeforeUpload = Await.result(s3Storage.exists(testFilePath), 10 seconds) + fileExistsBeforeUpload should be(false) + Await.result(s3Storage.upload(sourceTestFile, testFilePath), 10 seconds) val filesAfterUpload = Await.result(s3Storage.list(testDirPath).run, 10 seconds) filesAfterUpload.size should be(1) + val fileExistsAfterUpload = Await.result(s3Storage.exists(testFilePath), 10 seconds) + fileExistsAfterUpload should be(true) val uploadedFileLine = filesAfterUpload.head uploadedFileLine.name should be(Name[File](testFileName)) uploadedFileLine.location should be(testFilePath) @@ -96,10 +105,17 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar { val filesBefore = Await.result(fileStorage.list(testDirPath).run, 10 seconds) filesBefore shouldBe empty + val fileExistsBeforeUpload = Await.result(fileStorage.exists(testFilePath), 10 seconds) + fileExistsBeforeUpload should be(false) + Await.result(fileStorage.upload(sourceTestFile, testFilePath), 10 seconds) val filesAfterUpload = Await.result(fileStorage.list(testDirPath).run, 10 seconds) filesAfterUpload.size should be(1) + + val fileExistsAfterUpload = Await.result(fileStorage.exists(testFilePath), 10 seconds) + fileExistsAfterUpload should be(true) + val uploadedFileLine = filesAfterUpload.head uploadedFileLine.name should be(Name[File]("uploadTestFile")) uploadedFileLine.location should be(testFilePath) @@ -152,10 +168,17 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar { ) when(gcsMock.list(testBucket.value, BlobListOption.currentDirectory(), BlobListOption.prefix(s"$testDirPath/"))) .thenReturn(pageMock) + when(gcsMock.get(testBucket.value, testFilePath.toString)).thenReturn( + null, // before file is uploaded + blobMock // after file is uploaded + ) val filesBefore = Await.result(gcsStorage.list(testDirPath).run, 10 seconds) filesBefore shouldBe empty + val fileExistsBeforeUpload = Await.result(gcsStorage.exists(testFilePath), 10 seconds) + fileExistsBeforeUpload should be(false) + when(gcsMock.get(testBucket.value)).thenReturn(bucketMock) when(gcsMock.get(testBucket.value, testFilePath.toString)).thenReturn(blobMock) when(bucketMock.create(org.mockito.Matchers.eq(testFileName), any[FileInputStream], any[BlobWriteOption])) @@ -166,6 +189,9 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar { val filesAfterUpload = Await.result(gcsStorage.list(testDirPath).run, 10 seconds) filesAfterUpload.size should be(1) + val fileExistsAfterUpload = Await.result(gcsStorage.exists(testFilePath), 10 seconds) + fileExistsAfterUpload should be(true) + val downloadedFile = Await.result(gcsStorage.download(testFilePath).run, 10 seconds) downloadedFile shouldBe defined downloadedFile.foreach { -- cgit v1.2.3