From 4a54038933a89ea791e8ddad0ed977abf75bfa0c Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 1 Aug 2019 08:59:29 +0100 Subject: [PATCH] Refactor Uploader.upload(LocalFile,Bucket,Boolean,UploadEventListener,Int) (#129) * [storate-api] Remove unused tryCount * [storage-aws] Uploader Reduce the number of parameters --- .../thorp/core/UnversionedMirrorArchive.scala | 3 +- .../thorp/core/DummyStorageService.scala | 9 +++-- .../kemitix/thorp/storage/api/Storage.scala | 11 ++--- .../kemitix/thorp/storage/aws/S3Storage.scala | 8 ++-- .../kemitix/thorp/storage/aws/Uploader.scala | 40 ++++++++++--------- .../aws/AmazonS3ClientTestFixture.scala | 27 +++++++------ .../thorp/storage/aws/UploaderTest.scala | 19 +++------ 7 files changed, 54 insertions(+), 63 deletions(-) diff --git a/core/src/main/scala/net/kemitix/thorp/core/UnversionedMirrorArchive.scala b/core/src/main/scala/net/kemitix/thorp/core/UnversionedMirrorArchive.scala index 9fc065b..9f4edea 100644 --- a/core/src/main/scala/net/kemitix/thorp/core/UnversionedMirrorArchive.scala +++ b/core/src/main/scala/net/kemitix/thorp/core/UnversionedMirrorArchive.scala @@ -36,8 +36,7 @@ case class UnversionedMirrorArchive(syncTotals: SyncTotals) Storage.upload( localFile, bucket, - UploadEventListener(localFile, index, syncTotals, totalBytesSoFar), - 1) + UploadEventListener(localFile, index, syncTotals, totalBytesSoFar)) } object UnversionedMirrorArchive { diff --git a/core/src/test/scala/net/kemitix/thorp/core/DummyStorageService.scala b/core/src/test/scala/net/kemitix/thorp/core/DummyStorageService.scala index b2aeed6..fe20de5 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/DummyStorageService.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/DummyStorageService.scala @@ -20,10 +20,11 @@ case class DummyStorageService(s3ObjectData: S3ObjectsData, ): TaskR[Console, S3ObjectsData] = TaskR(s3ObjectData) - override def upload(localFile: LocalFile, - bucket: Bucket, - uploadEventListener: UploadEventListener, - tryCount: Int): UIO[StorageQueueEvent] = { + override def upload( + localFile: LocalFile, + bucket: Bucket, + uploadEventListener: UploadEventListener, + ): UIO[StorageQueueEvent] = { val (remoteKey, md5Hash) = uploadFiles(localFile.file) UIO(StorageQueueEvent.UploadQueueEvent(remoteKey, md5Hash)) } diff --git a/storage-api/src/main/scala/net/kemitix/thorp/storage/api/Storage.scala b/storage-api/src/main/scala/net/kemitix/thorp/storage/api/Storage.scala index ec388f7..ad5df92 100644 --- a/storage-api/src/main/scala/net/kemitix/thorp/storage/api/Storage.scala +++ b/storage-api/src/main/scala/net/kemitix/thorp/storage/api/Storage.scala @@ -21,7 +21,6 @@ object Storage { localFile: LocalFile, bucket: Bucket, uploadEventListener: UploadEventListener, - tryCount: Int ): ZIO[Storage with Config, Nothing, StorageQueueEvent] def copy( @@ -57,8 +56,8 @@ object Storage { override def upload( localFile: LocalFile, bucket: Bucket, - uploadEventListener: UploadEventListener, - tryCount: Int): ZIO[Storage, Nothing, StorageQueueEvent] = + uploadEventListener: UploadEventListener + ): ZIO[Storage, Nothing, StorageQueueEvent] = uploadResult override def copy( @@ -99,11 +98,9 @@ object Storage { final def upload( localFile: LocalFile, bucket: Bucket, - uploadEventListener: UploadEventListener, - tryCount: Int + uploadEventListener: UploadEventListener ): ZIO[Storage with Config, Nothing, StorageQueueEvent] = - ZIO.accessM( - _.storage upload (localFile, bucket, uploadEventListener, tryCount)) + ZIO.accessM(_.storage upload (localFile, bucket, uploadEventListener)) final def copy( bucket: Bucket, diff --git a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3Storage.scala b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3Storage.scala index acedb1f..7ba57e9 100644 --- a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3Storage.scala +++ b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3Storage.scala @@ -28,11 +28,9 @@ object S3Storage { localFile: LocalFile, bucket: Bucket, uploadEventListener: UploadEventListener, - tryCount: Int): ZIO[Config, Nothing, StorageQueueEvent] = - Uploader.upload(transferManager)(localFile, - bucket, - uploadEventListener, - 1) + ): ZIO[Config, Nothing, StorageQueueEvent] = + Uploader.upload(transferManager)( + Uploader.Request(localFile, bucket, uploadEventListener)) override def copy(bucket: Bucket, sourceKey: RemoteKey, diff --git a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/Uploader.scala b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/Uploader.scala index e3ad3db..fd01565 100644 --- a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/Uploader.scala +++ b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/Uploader.scala @@ -18,24 +18,24 @@ import zio.{UIO, ZIO} trait Uploader { - def upload(transferManager: => AmazonTransferManager)( + case class Request( localFile: LocalFile, bucket: Bucket, - uploadEventListener: UploadEventListener, - tryCount: Int - ): ZIO[Config, Nothing, StorageQueueEvent] = - transfer(transferManager)(localFile, bucket, uploadEventListener) - .catchAll(handleError(localFile.remoteKey)) + uploadEventListener: UploadEventListener + ) + + def upload(transferManager: => AmazonTransferManager)( + request: Request): ZIO[Config, Nothing, StorageQueueEvent] = + transfer(transferManager)(request) + .catchAll(handleError(request.localFile.remoteKey)) private def handleError(remoteKey: RemoteKey)(e: Throwable) = UIO(ErrorQueueEvent(Action.Upload(remoteKey.key), remoteKey, e)) private def transfer(transferManager: => AmazonTransferManager)( - localFile: LocalFile, - bucket: Bucket, - uploadEventListener: UploadEventListener + request: Request ) = - request(localFile, bucket, progressListener(uploadEventListener)) >>= + putObjectRequest(request) >>= dispatch(transferManager) private def dispatch(transferManager: AmazonTransferManager)( @@ -49,18 +49,20 @@ trait Uploader { MD5Hash(uploadResult.getETag))) } - private def request( - localFile: LocalFile, - bucket: Bucket, - listener: ProgressListener + private def putObjectRequest( + request: Request ) = { - val request = - new PutObjectRequest(bucket.name, localFile.remoteKey.key, localFile.file) - .withMetadata(metadata(localFile)) + val putRequest = + new PutObjectRequest(request.bucket.name, + request.localFile.remoteKey.key, + request.localFile.file) + .withMetadata(metadata(request.localFile)) for { batchMode <- Config.batchMode - r = if (batchMode) request - else request.withGeneralProgressListener(listener) + r = if (batchMode) putRequest + else + putRequest.withGeneralProgressListener( + progressListener(request.uploadEventListener)) } yield r } diff --git a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/AmazonS3ClientTestFixture.scala b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/AmazonS3ClientTestFixture.scala index 2be78df..de9d240 100644 --- a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/AmazonS3ClientTestFixture.scala +++ b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/AmazonS3ClientTestFixture.scala @@ -25,28 +25,31 @@ trait AmazonS3ClientTestFixture extends MockFactory { override def listObjects( bucket: Bucket, - prefix: RemoteKey): TaskR[Console, S3ObjectsData] = + prefix: RemoteKey + ): TaskR[Console, S3ObjectsData] = Lister.listObjects(client)(bucket, prefix) override def upload( localFile: LocalFile, bucket: Bucket, uploadEventListener: UploadEventListener, - tryCount: Int): ZIO[Config, Nothing, StorageQueueEvent] = - Uploader.upload(transferManager)(localFile, - bucket, - uploadEventListener, - 1) + ): ZIO[Config, Nothing, StorageQueueEvent] = + Uploader.upload(transferManager)( + Uploader.Request(localFile, bucket, uploadEventListener)) - override def copy(bucket: Bucket, - sourceKey: RemoteKey, - hash: MD5Hash, - targetKey: RemoteKey): UIO[StorageQueueEvent] = + override def copy( + bucket: Bucket, + sourceKey: RemoteKey, + hash: MD5Hash, + targetKey: RemoteKey + ): UIO[StorageQueueEvent] = Copier.copy(client)( Copier.Request(bucket, sourceKey, hash, targetKey)) - override def delete(bucket: Bucket, - remoteKey: RemoteKey): UIO[StorageQueueEvent] = + override def delete( + bucket: Bucket, + remoteKey: RemoteKey + ): UIO[StorageQueueEvent] = Deleter.delete(client)(bucket, remoteKey) override def shutdown: UIO[StorageQueueEvent] = { diff --git a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/UploaderTest.scala b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/UploaderTest.scala index fa039f1..271add8 100644 --- a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/UploaderTest.scala +++ b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/UploaderTest.scala @@ -27,7 +27,6 @@ class UploaderTest extends FreeSpec with MockFactory { val remoteKey = RemoteKey("aRemoteKey") val localFile = LocalFile(aFile, aSource, hashes, remoteKey) val bucket = Bucket("aBucket") - val tryCount = 1 val uploadResult = new UploadResult uploadResult.setKey(remoteKey.key) uploadResult.setETag(aHash.hash) @@ -47,8 +46,7 @@ class UploaderTest extends FreeSpec with MockFactory { invoke(fixture.amazonS3TransferManager)( localFile, bucket, - uploadEventListener, - tryCount + uploadEventListener ) assertResult(expected)(result) } @@ -66,8 +64,7 @@ class UploaderTest extends FreeSpec with MockFactory { invoke(fixture.amazonS3TransferManager)( localFile, bucket, - uploadEventListener, - tryCount + uploadEventListener ) assertResult(expected)(result) } @@ -85,8 +82,7 @@ class UploaderTest extends FreeSpec with MockFactory { invoke(fixture.amazonS3TransferManager)( localFile, bucket, - uploadEventListener, - tryCount + uploadEventListener ) assertResult(expected)(result) } @@ -94,19 +90,14 @@ class UploaderTest extends FreeSpec with MockFactory { def invoke(transferManager: AmazonTransferManager)( localFile: LocalFile, bucket: Bucket, - uploadEventListener: UploadEventListener, - tryCount: Int + uploadEventListener: UploadEventListener ) = { type TestEnv = Config val testEnv: TestEnv = Config.Live new DefaultRuntime {}.unsafeRunSync { Uploader .upload(transferManager)( - localFile, - bucket, - uploadEventListener, - tryCount - ) + Uploader.Request(localFile, bucket, uploadEventListener)) .provide(testEnv) }.toEither }