From 8ec667343a8d7cdc04eb7ca1f5679f9ae99eee2c Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Thu, 6 Jun 2019 19:49:07 +0100 Subject: [PATCH] Improve Code Quality (#37) * [core] convert QuoteStripper to an object and move to core * [aws-lib]S3ClientUploader: use case matching instead of else if blocks * [aws-lib] put imports at top of file * [domain] remove redundant braces after class definition * [aws-lib] remove redundant braces after class definition * [core] avoid using head on a collection --- .../net/kemitix/s3thorp/aws/lib/QuoteStripper.scala | 7 ------- .../s3thorp/aws/lib/S3ClientMultiPartUploader.scala | 4 ++-- .../s3thorp/aws/lib/S3ClientObjectLister.scala | 4 ++-- .../s3thorp/aws/lib/S3ClientPutObjectUploader.scala | 4 ++-- .../kemitix/s3thorp/aws/lib/S3ClientUploader.scala | 11 +++++------ .../net/kemitix/s3thorp/aws/lib/ThorpS3Client.scala | 3 +-- .../lib/S3ClientMultiPartTransferManagerSuite.scala | 4 +--- .../aws/lib/S3ClientMultiPartUploaderSuite.scala | 2 +- .../net.kemitix.s3thorp.core/ActionGenerator.scala | 8 ++------ .../net.kemitix.s3thorp.core/QuoteStripper.scala | 7 +++++++ .../scala/net/kemitix/s3thorp/domain/Bucket.scala | 4 +--- .../net/kemitix/s3thorp/domain/HashModified.scala | 4 +--- .../net/kemitix/s3thorp/domain/KeyModified.scala | 4 +--- .../net/kemitix/s3thorp/domain/LastModified.scala | 4 +--- .../net/kemitix/s3thorp/domain/RemoteMetaData.scala | 4 +--- .../net/kemitix/s3thorp/domain/S3ObjectsData.scala | 7 ++----- 16 files changed, 30 insertions(+), 51 deletions(-) delete mode 100644 aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/QuoteStripper.scala create mode 100644 core/src/main/scala/net.kemitix.s3thorp.core/QuoteStripper.scala diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/QuoteStripper.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/QuoteStripper.scala deleted file mode 100644 index f9524a9..0000000 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/QuoteStripper.scala +++ /dev/null @@ -1,7 +0,0 @@ -package net.kemitix.s3thorp.aws.lib - -trait QuoteStripper { - - def stripQuotes: Char => Boolean = _ != '"' - -} diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploader.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploader.scala index c158e70..5531b0d 100644 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploader.scala +++ b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploader.scala @@ -6,6 +6,7 @@ import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model._ import net.kemitix.s3thorp.aws.api.S3Action.{ErroredS3Action, UploadS3Action} import net.kemitix.s3thorp.aws.api.{S3Action, UploadProgressListener} +import net.kemitix.s3thorp.core.QuoteStripper.stripQuotes import net.kemitix.s3thorp.core.MD5HashGenerator.md5FilePart import net.kemitix.s3thorp.domain.{Bucket, LocalFile, MD5Hash} @@ -14,8 +15,7 @@ import scala.util.control.NonFatal private class S3ClientMultiPartUploader(s3Client: AmazonS3) extends S3ClientUploader - with S3ClientMultiPartUploaderLogging - with QuoteStripper { + with S3ClientMultiPartUploaderLogging { def accepts(localFile: LocalFile) (implicit multiPartThreshold: Long): Boolean = diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientObjectLister.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientObjectLister.scala index 7fd39c0..647fa7e 100644 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientObjectLister.scala +++ b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientObjectLister.scala @@ -2,6 +2,7 @@ package net.kemitix.s3thorp.aws.lib import cats.effect.IO import com.github.j5ik2o.reactive.aws.s3.cats.S3CatsIOClient +import net.kemitix.s3thorp.core.QuoteStripper.stripQuotes import net.kemitix.s3thorp.domain._ import software.amazon.awssdk.services.s3.model.{ListObjectsV2Request, S3Object} @@ -9,8 +10,7 @@ import scala.collection.JavaConverters._ class S3ClientObjectLister(s3Client: S3CatsIOClient) extends S3ClientLogging - with S3ObjectsByHash - with QuoteStripper { + with S3ObjectsByHash { def listObjects(bucket: Bucket, prefix: RemoteKey) diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientPutObjectUploader.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientPutObjectUploader.scala index 0c2ef19..ec7d9d2 100644 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientPutObjectUploader.scala +++ b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientPutObjectUploader.scala @@ -5,12 +5,12 @@ import com.amazonaws.services.s3.AmazonS3 import com.amazonaws.services.s3.model.PutObjectRequest import net.kemitix.s3thorp.aws.api.S3Action.UploadS3Action import net.kemitix.s3thorp.aws.api.UploadProgressListener +import net.kemitix.s3thorp.core.QuoteStripper.stripQuotes import net.kemitix.s3thorp.domain.{Bucket, LocalFile, MD5Hash} class S3ClientPutObjectUploader(amazonS3: => AmazonS3) extends S3ClientUploader - with S3ClientLogging - with QuoteStripper { + with S3ClientLogging { override def accepts(localFile: LocalFile)(implicit multiPartThreshold: Long): Boolean = true diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientUploader.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientUploader.scala index 1143d85..0dde250 100644 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientUploader.scala +++ b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/S3ClientUploader.scala @@ -23,12 +23,11 @@ trait S3ClientUploader { def progressListener(uploadProgressListener: UploadProgressListener): ProgressListener = { new ProgressListener { override def progressChanged(event: ProgressEvent): Unit = { - if (event.getEventType.isTransferEvent) - TransferEvent(event.getEventType.name) - else if (event.getEventType equals ProgressEventType.RESPONSE_BYTE_TRANSFER_EVENT) - ByteTransferEvent(event.getEventType.name) - else - RequestEvent(event.getEventType.name, event.getBytes, event.getBytesTransferred) + event match { + case e if e.getEventType.isTransferEvent => TransferEvent(e.getEventType.name) + case e if e.getEventType equals ProgressEventType.RESPONSE_BYTE_TRANSFER_EVENT => ByteTransferEvent(e.getEventType.name) + case e => RequestEvent(e.getEventType.name, e.getBytes, e.getBytesTransferred) + } } } } diff --git a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/ThorpS3Client.scala b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/ThorpS3Client.scala index b2da44d..e45be77 100644 --- a/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/ThorpS3Client.scala +++ b/aws-lib/src/main/scala/net/kemitix/s3thorp/aws/lib/ThorpS3Client.scala @@ -13,8 +13,7 @@ class ThorpS3Client(ioS3Client: S3CatsIOClient, amazonS3Client: => AmazonS3, amazonS3TransferManager: => TransferManager) extends S3Client - with S3ClientLogging - with QuoteStripper { + with S3ClientLogging { // lazy val amazonS3Client = AmazonS3ClientBuilder.defaultClient // lazy val amazonS3TransferManager = TransferManagerBuilder.defaultTransferManager diff --git a/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartTransferManagerSuite.scala b/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartTransferManagerSuite.scala index d8f8309..ffd3031 100644 --- a/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartTransferManagerSuite.scala +++ b/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartTransferManagerSuite.scala @@ -4,12 +4,12 @@ import java.io.File import java.time.Instant import com.amazonaws.AmazonClientException +import com.amazonaws.event.ProgressListener import com.amazonaws.services.s3.model import com.amazonaws.services.s3.transfer.model.UploadResult import com.amazonaws.services.s3.transfer._ import net.kemitix.s3thorp.aws.api.S3Action.UploadS3Action import net.kemitix.s3thorp.aws.api.UploadProgressListener -import net.kemitix.s3thorp.aws.lib.S3ClientMultiPartTransferManager import net.kemitix.s3thorp.core.KeyGenerator.generateKey import net.kemitix.s3thorp.core.{MD5HashGenerator, Resource} import net.kemitix.s3thorp.domain._ @@ -115,8 +115,6 @@ class S3ClientMultiPartTransferManagerSuite override def getProgress: TransferProgress = ??? - import com.amazonaws.event.ProgressListener - override def addProgressListener(listener: ProgressListener): Unit = ??? override def removeProgressListener(listener: ProgressListener): Unit = ??? diff --git a/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploaderSuite.scala b/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploaderSuite.scala index ea63e35..33cf7f1 100644 --- a/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploaderSuite.scala +++ b/aws-lib/src/test/scala/net/kemitix/s3thorp/aws/lib/S3ClientMultiPartUploaderSuite.scala @@ -283,7 +283,7 @@ class S3ClientMultiPartUploaderSuite override def abortMultipartUpload(abortMultipartUploadRequest: AbortMultipartUploadRequest): Unit = { canceled set true } - }) {} + }) } private def invoke(uploader: S3ClientMultiPartUploader, theFile: LocalFile, progressListener: UploadProgressListener) = { diff --git a/core/src/main/scala/net.kemitix.s3thorp.core/ActionGenerator.scala b/core/src/main/scala/net.kemitix.s3thorp.core/ActionGenerator.scala index 38041bb..839d531 100644 --- a/core/src/main/scala/net.kemitix.s3thorp.core/ActionGenerator.scala +++ b/core/src/main/scala/net.kemitix.s3thorp.core/ActionGenerator.scala @@ -50,11 +50,7 @@ object ActionGenerator { private def copyFile(bucket: Bucket, localFile: LocalFile, matchByHash: Set[RemoteMetaData]): Stream[Action] = - Stream( - ToCopy( - bucket, - sourceKey = matchByHash.head.remoteKey, - hash = localFile.hash, - targetKey = localFile.remoteKey)) + matchByHash.headOption.map(_.remoteKey).toStream.map {sourceKey => + ToCopy(bucket, sourceKey, localFile.hash, localFile.remoteKey)} } diff --git a/core/src/main/scala/net.kemitix.s3thorp.core/QuoteStripper.scala b/core/src/main/scala/net.kemitix.s3thorp.core/QuoteStripper.scala new file mode 100644 index 0000000..0f6d8f3 --- /dev/null +++ b/core/src/main/scala/net.kemitix.s3thorp.core/QuoteStripper.scala @@ -0,0 +1,7 @@ +package net.kemitix.s3thorp.core + +object QuoteStripper { + + def stripQuotes: Char => Boolean = _ != '"' + +} diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/Bucket.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/Bucket.scala index f5269fc..b10a129 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/Bucket.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/Bucket.scala @@ -1,5 +1,3 @@ package net.kemitix.s3thorp.domain -final case class Bucket(name: String) { - -} +final case class Bucket(name: String) diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/HashModified.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/HashModified.scala index 35e6a16..24ba44b 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/HashModified.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/HashModified.scala @@ -1,6 +1,4 @@ package net.kemitix.s3thorp.domain final case class HashModified(hash: MD5Hash, - modified: LastModified) { - -} + modified: LastModified) diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/KeyModified.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/KeyModified.scala index bd5ae5a..de6139e 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/KeyModified.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/KeyModified.scala @@ -1,6 +1,4 @@ package net.kemitix.s3thorp.domain final case class KeyModified(key: RemoteKey, - modified: LastModified) { - -} + modified: LastModified) diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/LastModified.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/LastModified.scala index ee5d3a3..0a4fc76 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/LastModified.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/LastModified.scala @@ -2,6 +2,4 @@ package net.kemitix.s3thorp.domain import java.time.Instant -final case class LastModified(when: Instant) { - -} +final case class LastModified(when: Instant) diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/RemoteMetaData.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/RemoteMetaData.scala index 3d1035c..f499d16 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/RemoteMetaData.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/RemoteMetaData.scala @@ -2,6 +2,4 @@ package net.kemitix.s3thorp.domain final case class RemoteMetaData(remoteKey: RemoteKey, hash: MD5Hash, - lastModified: LastModified) { - -} + lastModified: LastModified) diff --git a/domain/src/main/scala/net/kemitix/s3thorp/domain/S3ObjectsData.scala b/domain/src/main/scala/net/kemitix/s3thorp/domain/S3ObjectsData.scala index 9ceddfe..30a5723 100644 --- a/domain/src/main/scala/net/kemitix/s3thorp/domain/S3ObjectsData.scala +++ b/domain/src/main/scala/net/kemitix/s3thorp/domain/S3ObjectsData.scala @@ -3,8 +3,5 @@ package net.kemitix.s3thorp.domain /** * A list of objects and their MD5 hash values. */ -final case class S3ObjectsData( - byHash: Map[MD5Hash, Set[KeyModified]], - byKey: Map[RemoteKey, HashModified]) { - -} +final case class S3ObjectsData(byHash: Map[MD5Hash, Set[KeyModified]], + byKey: Map[RemoteKey, HashModified])