From 985cc9f1476567f860d4e98e055ab2c10cd077d8 Mon Sep 17 00:00:00 2001 From: Paul Campbell Date: Wed, 24 Jul 2019 19:50:28 +0100 Subject: [PATCH] Don't use String as key in Map for hashes (#124) * Don't use String as key in Map for hashes * [domain] HashType remote redundant braces --- CHANGELOG.org | 1 + .../thorp/core/SimpleHashService.scala | 7 ++--- .../thorp/core/ActionGeneratorSuite.scala | 5 ++-- .../kemitix/thorp/core/DummyHashService.scala | 6 ++--- .../thorp/core/LocalFileStreamSuite.scala | 5 ++-- .../kemitix/thorp/core/PlanBuilderTest.scala | 5 ++-- .../thorp/core/S3MetaDataEnricherSuite.scala | 5 ++-- .../kemitix/thorp/core/SequencePlanTest.scala | 26 ++++++++++++------- .../net/kemitix/thorp/core/SyncSuite.scala | 11 ++++---- .../net/kemitix/thorp/domain/HashType.scala | 7 +++++ .../net/kemitix/thorp/domain/LocalFile.scala | 8 +++--- .../thorp/storage/api/HashService.scala | 4 +-- .../net/kemitix/thorp/storage/aws/ETag.scala | 5 ++++ .../thorp/storage/aws/S3HashService.scala | 9 ++++--- .../storage/aws/StorageServiceSuite.scala | 5 ++-- 15 files changed, 69 insertions(+), 40 deletions(-) create mode 100644 domain/src/main/scala/net/kemitix/thorp/domain/HashType.scala create mode 100644 storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/ETag.scala diff --git a/CHANGELOG.org b/CHANGELOG.org index d3f4d19..996eab3 100644 --- a/CHANGELOG.org +++ b/CHANGELOG.org @@ -18,6 +18,7 @@ The format is based on [[https://keepachangelog.com/en/1.0.0/][Keep a Changelog] - Replace cats-effect with zio (#117) - Replace Monocle with local SimpleLens implementation (#121) + - Don't use String as key in Map for hashes (#124) ** Dependencies diff --git a/core/src/main/scala/net/kemitix/thorp/core/SimpleHashService.scala b/core/src/main/scala/net/kemitix/thorp/core/SimpleHashService.scala index 6991af6..16b4a6e 100644 --- a/core/src/main/scala/net/kemitix/thorp/core/SimpleHashService.scala +++ b/core/src/main/scala/net/kemitix/thorp/core/SimpleHashService.scala @@ -2,7 +2,8 @@ package net.kemitix.thorp.core import java.nio.file.Path -import net.kemitix.thorp.domain.MD5Hash +import net.kemitix.thorp.domain.HashType.MD5 +import net.kemitix.thorp.domain.{HashType, MD5Hash} import net.kemitix.thorp.storage.api.HashService import zio.Task @@ -10,9 +11,9 @@ case class SimpleHashService() extends HashService { override def hashLocalObject( path: Path - ): Task[Map[String, MD5Hash]] = + ): Task[Map[HashType, MD5Hash]] = for { md5 <- MD5HashGenerator.md5File(path) - } yield Map("md5" -> md5) + } yield Map(MD5 -> md5) } diff --git a/core/src/test/scala/net/kemitix/thorp/core/ActionGeneratorSuite.scala b/core/src/test/scala/net/kemitix/thorp/core/ActionGeneratorSuite.scala index ea5172f..146c280 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/ActionGeneratorSuite.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/ActionGeneratorSuite.scala @@ -3,6 +3,7 @@ package net.kemitix.thorp.core import java.time.Instant import net.kemitix.thorp.core.Action.{DoNothing, ToCopy, ToUpload} +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain._ import org.scalatest.FunSpec @@ -143,7 +144,7 @@ class ActionGeneratorSuite extends FunSpec { } } - private def md5HashMap(theHash: MD5Hash) = { - Map("md5" -> theHash) + private def md5HashMap(theHash: MD5Hash): Map[HashType, MD5Hash] = { + Map(MD5 -> theHash) } } diff --git a/core/src/test/scala/net/kemitix/thorp/core/DummyHashService.scala b/core/src/test/scala/net/kemitix/thorp/core/DummyHashService.scala index 31b7941..f2b2186 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/DummyHashService.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/DummyHashService.scala @@ -2,14 +2,14 @@ package net.kemitix.thorp.core import java.nio.file.Path -import net.kemitix.thorp.domain.MD5Hash +import net.kemitix.thorp.domain.{HashType, MD5Hash} import net.kemitix.thorp.storage.api.HashService import zio.Task -case class DummyHashService(hashes: Map[Path, Map[String, MD5Hash]]) +case class DummyHashService(hashes: Map[Path, Map[HashType, MD5Hash]]) extends HashService { - override def hashLocalObject(path: Path): Task[Map[String, MD5Hash]] = + override def hashLocalObject(path: Path): Task[Map[HashType, MD5Hash]] = Task(hashes(path)) } diff --git a/core/src/test/scala/net/kemitix/thorp/core/LocalFileStreamSuite.scala b/core/src/test/scala/net/kemitix/thorp/core/LocalFileStreamSuite.scala index 9388a01..077f3aa 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/LocalFileStreamSuite.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/LocalFileStreamSuite.scala @@ -2,6 +2,7 @@ package net.kemitix.thorp.core import java.nio.file.Paths +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain._ import net.kemitix.thorp.storage.api.HashService import org.scalatest.FunSpec @@ -13,8 +14,8 @@ class LocalFileStreamSuite extends FunSpec { private val sourcePath = source.toPath private val hashService: HashService = DummyHashService( Map( - file("root-file") -> Map("md5" -> MD5HashData.Root.hash), - file("subdir/leaf-file") -> Map("md5" -> MD5HashData.Leaf.hash) + file("root-file") -> Map(MD5 -> MD5HashData.Root.hash), + file("subdir/leaf-file") -> Map(MD5 -> MD5HashData.Leaf.hash) )) private def file(filename: String) = diff --git a/core/src/test/scala/net/kemitix/thorp/core/PlanBuilderTest.scala b/core/src/test/scala/net/kemitix/thorp/core/PlanBuilderTest.scala index e55d9e6..9fd0d75 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/PlanBuilderTest.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/PlanBuilderTest.scala @@ -5,6 +5,7 @@ import java.nio.file.Path import net.kemitix.thorp.console._ import net.kemitix.thorp.core.Action.{DoNothing, ToCopy, ToDelete, ToUpload} +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain._ import net.kemitix.thorp.storage.api.{HashService, StorageService} import org.scalatest.FreeSpec @@ -433,7 +434,7 @@ class PlanBuilderTest extends FreeSpec with TemporaryFolder { def md5Hash(file: File) = { runtime .unsafeRunSync { - hashService.hashLocalObject(file.toPath).map(_.get("md5")) + hashService.hashLocalObject(file.toPath).map(_.get(MD5)) } .toEither .toOption @@ -477,7 +478,7 @@ class PlanBuilderTest extends FreeSpec with TemporaryFolder { case ToUpload(_, lf, _) => ("upload", lf.remoteKey.key, - lf.hashes("md5").hash, + lf.hashes(MD5).hash, lf.source.toString, lf.file.toString) case ToDelete(_, remoteKey, _) => ("delete", remoteKey.key, "", "", "") diff --git a/core/src/test/scala/net/kemitix/thorp/core/S3MetaDataEnricherSuite.scala b/core/src/test/scala/net/kemitix/thorp/core/S3MetaDataEnricherSuite.scala index abed045..3dc1a74 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/S3MetaDataEnricherSuite.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/S3MetaDataEnricherSuite.scala @@ -3,6 +3,7 @@ package net.kemitix.thorp.core import java.time.Instant import net.kemitix.thorp.core.S3MetaDataEnricher.{getMetadata, getS3Status} +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain._ import org.scalatest.FunSpec @@ -171,8 +172,8 @@ class S3MetaDataEnricherSuite extends FunSpec { } } - private def md5HashMap(theHash: MD5Hash) = { - Map("md5" -> theHash) + private def md5HashMap(theHash: MD5Hash): Map[HashType, MD5Hash] = { + Map(MD5 -> theHash) } describe("getS3Status") { diff --git a/core/src/test/scala/net/kemitix/thorp/core/SequencePlanTest.scala b/core/src/test/scala/net/kemitix/thorp/core/SequencePlanTest.scala index 0f8df6f..c3363b7 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/SequencePlanTest.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/SequencePlanTest.scala @@ -3,22 +3,28 @@ package net.kemitix.thorp.core import java.io.File import net.kemitix.thorp.core.Action._ -import net.kemitix.thorp.domain.{Bucket, LocalFile, MD5Hash, RemoteKey} +import net.kemitix.thorp.domain.{ + Bucket, + HashType, + LocalFile, + MD5Hash, + RemoteKey +} import org.scalatest.FreeSpec class SequencePlanTest extends FreeSpec { "sort" - { "a list of assorted actions" - { - val bucket = Bucket("aBucket") - val remoteKey1 = RemoteKey("remoteKey1") - val remoteKey2 = RemoteKey("targetHash") - val hash = MD5Hash("aHash") - val hashes: Map[String, MD5Hash] = Map() - val size = 1024 - val file1 = new File("aFile") - val file2 = new File("aFile") - val source = new File("source") + val bucket = Bucket("aBucket") + val remoteKey1 = RemoteKey("remoteKey1") + val remoteKey2 = RemoteKey("targetHash") + val hash = MD5Hash("aHash") + val hashes = Map[HashType, MD5Hash]() + val size = 1024 + val file1 = new File("aFile") + val file2 = new File("aFile") + val source = new File("source") val localFile1 = LocalFile(file1, source, hashes, remoteKey1) val localFile2 = diff --git a/core/src/test/scala/net/kemitix/thorp/core/SyncSuite.scala b/core/src/test/scala/net/kemitix/thorp/core/SyncSuite.scala index f3d00e1..0b3f1ea 100644 --- a/core/src/test/scala/net/kemitix/thorp/core/SyncSuite.scala +++ b/core/src/test/scala/net/kemitix/thorp/core/SyncSuite.scala @@ -7,6 +7,7 @@ import java.time.Instant import net.kemitix.thorp.console import net.kemitix.thorp.console.MyConsole import net.kemitix.thorp.core.Action.{ToCopy, ToDelete, ToUpload} +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain.MD5HashData.{Leaf, Root} import net.kemitix.thorp.domain.StorageQueueEvent.{ CopyQueueEvent, @@ -43,8 +44,8 @@ class SyncSuite extends FunSpec { private val hashService = DummyHashService( Map( - file("root-file") -> Map("md5" -> MD5HashData.Root.hash), - file("subdir/leaf-file") -> Map("md5" -> MD5HashData.Leaf.hash) + file("root-file") -> Map(MD5 -> MD5HashData.Root.hash), + file("subdir/leaf-file") -> Map(MD5 -> MD5HashData.Leaf.hash) )) private val configOptions = ConfigOptions( @@ -62,8 +63,8 @@ class SyncSuite extends FunSpec { localFile: LocalFile): (String, String, File) = (bucket.name, remoteKey.key, localFile.file) - private def md5HashMap(md5Hash: MD5Hash): Map[String, MD5Hash] = - Map("md5" -> md5Hash) + private def md5HashMap(md5Hash: MD5Hash): Map[HashType, MD5Hash] = + Map(MD5 -> md5Hash) private def file(filename: String) = sourcePath.resolve(Paths.get(filename)) @@ -207,7 +208,7 @@ class SyncSuite extends FunSpec { batchMode: Boolean, uploadEventListener: UploadEventListener, tryCount: Int): Task[UploadQueueEvent] = - Task(UploadQueueEvent(localFile.remoteKey, localFile.hashes("md5"))) + Task(UploadQueueEvent(localFile.remoteKey, localFile.hashes(MD5))) override def copy(bucket: Bucket, sourceKey: RemoteKey, diff --git a/domain/src/main/scala/net/kemitix/thorp/domain/HashType.scala b/domain/src/main/scala/net/kemitix/thorp/domain/HashType.scala new file mode 100644 index 0000000..1c0d70b --- /dev/null +++ b/domain/src/main/scala/net/kemitix/thorp/domain/HashType.scala @@ -0,0 +1,7 @@ +package net.kemitix.thorp.domain + +trait HashType + +object HashType { + case object MD5 extends HashType +} diff --git a/domain/src/main/scala/net/kemitix/thorp/domain/LocalFile.scala b/domain/src/main/scala/net/kemitix/thorp/domain/LocalFile.scala index 8f91a4b..10d8be2 100644 --- a/domain/src/main/scala/net/kemitix/thorp/domain/LocalFile.scala +++ b/domain/src/main/scala/net/kemitix/thorp/domain/LocalFile.scala @@ -3,10 +3,12 @@ package net.kemitix.thorp.domain import java.io.File import java.nio.file.Path +import net.kemitix.thorp.domain.HashType.MD5 + final case class LocalFile( file: File, source: File, - hashes: Map[String, MD5Hash], + hashes: Map[HashType, MD5Hash], remoteKey: RemoteKey ) { @@ -19,7 +21,7 @@ final case class LocalFile( def matches(other: MD5Hash): Boolean = hashes.values.exists(other equals _) - def md5base64: Option[String] = hashes.get("md5").map(_.hash64) + def md5base64: Option[String] = hashes.get(MD5).map(_.hash64) } @@ -27,7 +29,7 @@ object LocalFile { def resolve( path: String, - md5Hashes: Map[String, MD5Hash], + md5Hashes: Map[HashType, MD5Hash], source: Path, pathToKey: Path => RemoteKey ): LocalFile = { diff --git a/storage-api/src/main/scala/net/kemitix/thorp/storage/api/HashService.scala b/storage-api/src/main/scala/net/kemitix/thorp/storage/api/HashService.scala index 6b2c022..95d8b6d 100644 --- a/storage-api/src/main/scala/net/kemitix/thorp/storage/api/HashService.scala +++ b/storage-api/src/main/scala/net/kemitix/thorp/storage/api/HashService.scala @@ -2,7 +2,7 @@ package net.kemitix.thorp.storage.api import java.nio.file.Path -import net.kemitix.thorp.domain.MD5Hash +import net.kemitix.thorp.domain.{HashType, MD5Hash} import zio.Task /** @@ -10,6 +10,6 @@ import zio.Task */ trait HashService { - def hashLocalObject(path: Path): Task[Map[String, MD5Hash]] + def hashLocalObject(path: Path): Task[Map[HashType, MD5Hash]] } diff --git a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/ETag.scala b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/ETag.scala new file mode 100644 index 0000000..4b71bc9 --- /dev/null +++ b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/ETag.scala @@ -0,0 +1,5 @@ +package net.kemitix.thorp.storage.aws + +import net.kemitix.thorp.domain.HashType + +object ETag extends HashType diff --git a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3HashService.scala b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3HashService.scala index 644ca33..665e8ac 100644 --- a/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3HashService.scala +++ b/storage-aws/src/main/scala/net/kemitix/thorp/storage/aws/S3HashService.scala @@ -3,7 +3,8 @@ package net.kemitix.thorp.storage.aws import java.nio.file.Path import net.kemitix.thorp.core.MD5HashGenerator -import net.kemitix.thorp.domain.MD5Hash +import net.kemitix.thorp.domain.HashType.MD5 +import net.kemitix.thorp.domain.{HashType, MD5Hash} import net.kemitix.thorp.storage.api.HashService import zio.Task @@ -17,14 +18,14 @@ trait S3HashService extends HashService { */ override def hashLocalObject( path: Path - ): Task[Map[String, MD5Hash]] = + ): Task[Map[HashType, MD5Hash]] = for { md5 <- MD5HashGenerator.md5File(path) etag <- ETagGenerator.eTag(path).map(MD5Hash(_)) } yield Map( - "md5" -> md5, - "etag" -> etag + MD5 -> md5, + ETag -> etag ) } diff --git a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/StorageServiceSuite.scala b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/StorageServiceSuite.scala index 170ec8c..9722d30 100644 --- a/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/StorageServiceSuite.scala +++ b/storage-aws/src/test/scala/net/kemitix/thorp/storage/aws/StorageServiceSuite.scala @@ -5,6 +5,7 @@ import java.time.Instant import com.amazonaws.services.s3.model.PutObjectRequest import com.amazonaws.services.s3.transfer.model.UploadResult import net.kemitix.thorp.core.{KeyGenerator, Resource, S3MetaDataEnricher} +import net.kemitix.thorp.domain.HashType.MD5 import net.kemitix.thorp.domain.MD5HashData.Root import net.kemitix.thorp.domain.StorageQueueEvent.UploadQueueEvent import net.kemitix.thorp.domain._ @@ -113,8 +114,8 @@ class StorageServiceSuite extends FunSpec with MockFactory { } - private def md5HashMap(hash: MD5Hash) = - Map("md5" -> hash) + private def md5HashMap(hash: MD5Hash): Map[HashType, MD5Hash] = + Map(MD5 -> hash) val batchMode: Boolean = true