Changed ids underlying type to String and made Ids and Names — value-classes

11 files changed, 80 insertions, 73 deletions

diff --git a/src/main/scala/xyz/driver/core/auth.scala b/src/main/scala/xyz/driver/core/auth.scala
index e4d726b..67de21d 100644
--- a/src/main/scala/xyz/driver/core/auth.scala
+++ b/src/main/scala/xyz/driver/core/auth.scala
@@ -32,32 +32,32 @@ object auth {
   }
 
   case object ObserverRole extends Role {
-    val id          = Id(1L)
+    val id          = Id("1")
     val name        = Name("observer")
     val permissions = Set[Permission](CanSeeUser, CanSeeAssay, CanSeeReport)
   }
 
   case object PatientRole extends Role {
-    val id          = Id(2L)
+    val id          = Id("2")
     val name        = Name("patient")
     val permissions = Set.empty[Permission]
   }
 
   case object CuratorRole extends Role {
-    val id          = Id(3L)
+    val id          = Id("3")
     val name        = Name("curator")
     val permissions = ObserverRole.permissions ++ Set[Permission](CanEditReport, CanReviewReport)
   }
 
   case object PathologistRole extends Role {
-    val id   = Id(4L)
+    val id   = Id("4")
     val name = Name("pathologist")
     val permissions = ObserverRole.permissions ++
         Set[Permission](CanEditReport, CanSignOutReport, CanAmendReport, CanEditReviewingReport)
   }
 
   case object AdministratorRole extends Role {
-    val id   = Id(5L)
+    val id   = Id("5")
     val name = Name("administrator")
     val permissions = CuratorRole.permissions ++
         Set[Permission](CanCreateReport, CanShareReportWithPatient, CanAssignRoles)
diff --git a/src/main/scala/xyz/driver/core/core.scala b/src/main/scala/xyz/driver/core/core.scala
index b7fbeb6..fa0028b 100644
--- a/src/main/scala/xyz/driver/core/core.scala
+++ b/src/main/scala/xyz/driver/core/core.scala
@@ -3,10 +3,13 @@ package xyz.driver
 import scalaz.Equal
 
 package object core {
+
   import scala.language.reflectiveCalls
 
   def make[T](v: => T)(f: T => Unit): T = {
-    val value = v; f(value); value
+    val value = v
+    f(value)
+    value
   }
 
   def using[R <: { def close() }, P](r: => R)(f: R => P): P = {
@@ -17,30 +20,33 @@ package object core {
       resource.close()
     }
   }
+}
 
-  object tagging {
-    private[core] trait Tagged[+V, +Tag]
+package core {
+
+  final case class Id[+Tag](value: String) extends AnyVal {
+    def length: Int               = value.length
+    override def toString: String = value
   }
-  type @@[+V, +Tag] = V with tagging.Tagged[V, Tag]
 
-  type Id[+Tag] = Long @@ Tag
   object Id {
-    def apply[Tag](value: Long) = value.asInstanceOf[Id[Tag]]
+    implicit def idEqual[T]: Equal[Id[T]]       = Equal.equal[Id[T]](_ == _)
+    implicit def idOrdering[T]: Ordering[Id[T]] = Ordering.by[Id[T], String](_.value)
   }
-  implicit def idEqual[T]: Equal[Id[T]]       = Equal.equal[Id[T]](_ == _)
-  implicit def idOrdering[T]: Ordering[Id[T]] = Ordering.by(i => i: Long)
 
-  type Name[+Tag] = String @@ Tag
-  object Name {
-    def apply[Tag](value: String) = value.asInstanceOf[Name[Tag]]
+  final case class Name[+Tag](value: String) extends AnyVal {
+    def length: Int               = value.length
+    override def toString: String = value
   }
 
-  implicit def nameEqual[T]: Equal[Name[T]]       = Equal.equal[Name[T]](_ == _)
-  implicit def nameOrdering[T]: Ordering[Name[T]] = Ordering.by(n => n: String)
+  object Name {
+    implicit def nameEqual[T]: Equal[Name[T]]       = Equal.equal[Name[T]](_ == _)
+    implicit def nameOrdering[T]: Ordering[Name[T]] = Ordering.by(_.value)
+  }
 
   object revision {
     final case class Revision[T](id: String)
 
     implicit def revisionEqual[T]: Equal[Revision[T]] = Equal.equal[Revision[T]](_.id == _.id)
   }
-}
+}
\ No newline at end of file
diff --git a/src/main/scala/xyz/driver/core/database.scala b/src/main/scala/xyz/driver/core/database.scala
index 85a8cc4..a8ad477 100644
--- a/src/main/scala/xyz/driver/core/database.scala
+++ b/src/main/scala/xyz/driver/core/database.scala
@@ -37,12 +37,12 @@ object database {
     import database.profile.api._
 
     implicit def idColumnType[T] =
-      MappedColumnType.base[Id[T], Long](id => id: Long, Id[T](_))
+      MappedColumnType.base[Id[T], String](_.value, Id[T](_))
 
     implicit def nameColumnType[T] =
-      MappedColumnType.base[Name[T], String](name => name: String, Name[T](_))
+      MappedColumnType.base[Name[T], String](_.value, Name[T](_))
 
-    implicit val timeColumnType = MappedColumnType.base[Time, Long](time => time.millis, Time(_))
+    implicit val timeColumnType = MappedColumnType.base[Time, Long](_.millis, Time.apply)
   }
 
   trait DatabaseObject extends IdColumnTypes {
diff --git a/src/main/scala/xyz/driver/core/file.scala b/src/main/scala/xyz/driver/core/file.scala
index 38a2766..9cea9e5 100644
--- a/src/main/scala/xyz/driver/core/file.scala
+++ b/src/main/scala/xyz/driver/core/file.scala
@@ -59,7 +59,7 @@ object file {
 
     def upload(localSource: File, destination: Path): Future[Unit] = Future {
       checkSafeFileName(destination) {
-        val _ = s3.putObject(bucket, destination.toString, localSource).getETag
+        val _ = s3.putObject(bucket.value, destination.toString, localSource).getETag
       }
     }
 
@@ -72,20 +72,20 @@ object file {
         if (!tempDestinationFile.getParentFile.mkdirs()) {
           throw new Exception(s"Failed to create temp directory to download file `$tempDestinationFile`")
         } else {
-          Option(s3.getObject(new GetObjectRequest(bucket, filePath.toString), tempDestinationFile)).map { _ =>
+          Option(s3.getObject(new GetObjectRequest(bucket.value, filePath.toString), tempDestinationFile)).map { _ =>
             tempDestinationFile
           }
         }
       })
 
     def delete(filePath: Path): Future[Unit] = Future {
-      s3.deleteObject(bucket, filePath.toString)
+      s3.deleteObject(bucket.value, filePath.toString)
     }
 
     def list(path: Path): ListT[Future, FileLink] =
       ListT.listT(Future {
         import scala.collection.JavaConverters._
-        val req = new ListObjectsV2Request().withBucketName(bucket).withPrefix(path.toString).withMaxKeys(2)
+        val req = new ListObjectsV2Request().withBucketName(bucket.value).withPrefix(path.toString).withMaxKeys(2)
 
         def isInSubFolder(path: Path)(fileLink: FileLink) =
           fileLink.location.toString.replace(path.toString + "/", "").contains("/")
diff --git a/src/main/scala/xyz/driver/core/generators.scala b/src/main/scala/xyz/driver/core/generators.scala
index bb026a9..6bf579b 100644
--- a/src/main/scala/xyz/driver/core/generators.scala
+++ b/src/main/scala/xyz/driver/core/generators.scala
@@ -13,12 +13,18 @@ object generators {
   private val random = new Random
   import random._
 
-  private val DefaultMaxLength = 100
+  private val DefaultMaxLength = 10
   private val StringLetters    = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ ".toSet
 
-  def nextId[T](): Id[T] = Id[T](scala.math.abs(nextLong()))
+  def nextInt(maxValue: Int): Int = random.nextInt(maxValue)
 
-  def nextId[T](maxValue: Int): Id[T] = Id[T](scala.math.abs(nextInt(maxValue).toLong))
+  def nextBoolean(): Boolean = random.nextBoolean()
+
+  def nextDouble(): Double = random.nextDouble()
+
+  def nextId[T](): Id[T] = Id[T](nextString(DefaultMaxLength))
+
+  def nextId[T](maxLength: Int): Id[T] = Id[T](nextString(maxLength))
 
   def nextName[T](maxLength: Int = DefaultMaxLength): Name[T] = Name[T](nextString(maxLength))
 
@@ -29,7 +35,7 @@ object generators {
   def nextString(maxLength: Int = DefaultMaxLength): String =
     (oneOf[Char](StringLetters) +: arrayOf(oneOf[Char](StringLetters), maxLength - 1)).mkString
 
-  def nextOption[T](value: => T): Option[T] = if (nextBoolean) Option(value) else None
+  def nextOption[T](value: => T): Option[T] = if (nextBoolean()) Option(value) else None
 
   def nextPair[L, R](left: => L, right: => R): (L, R) = (left, right)
 
diff --git a/src/main/scala/xyz/driver/core/json.scala b/src/main/scala/xyz/driver/core/json.scala
index 3917eca..cc27944 100644
--- a/src/main/scala/xyz/driver/core/json.scala
+++ b/src/main/scala/xyz/driver/core/json.scala
@@ -12,18 +12,16 @@ import scala.reflect.runtime.universe._
 
 object json {
 
-  def IdInPath[T]: PathMatcher1[Id[T]] =
-    PathMatcher("""[+-]?\d*""".r) flatMap { string =>
-      try Some(Id[T](string.toLong))
-      catch { case _: IllegalArgumentException => None }
-    }
+  def IdInPath[T]: PathMatcher1[Id[T]] = new PathMatcher1[Id[T]] {
+    def apply(path: Path) = Matched(Path.Empty, Tuple1(Id[T](path.toString)))
+  }
 
   implicit def idFormat[T] = new RootJsonFormat[Id[T]] {
-    def write(id: Id[T]) = JsNumber(id)
+    def write(id: Id[T]) = JsString(id.value)
 
     def read(value: JsValue) = value match {
-      case JsNumber(id) => Id[T](id.toLong)
-      case _            => throw DeserializationException("Id expects number")
+      case JsString(id) => Id[T](id)
+      case _            => throw DeserializationException("Id expects string")
     }
   }
 
@@ -32,7 +30,7 @@ object json {
   }
 
   implicit def nameFormat[T] = new RootJsonFormat[Name[T]] {
-    def write(name: Name[T]) = JsString(name)
+    def write(name: Name[T]) = JsString(name.value)
 
     def read(value: JsValue): Name[T] = value match {
       case JsString(name) => Name[T](name)
diff --git a/src/test/scala/xyz/driver/core/AuthTest.scala b/src/test/scala/xyz/driver/core/AuthTest.scala
index e5e991b..f4d4d2a 100644
--- a/src/test/scala/xyz/driver/core/AuthTest.scala
+++ b/src/test/scala/xyz/driver/core/AuthTest.scala
@@ -19,7 +19,7 @@ class AuthTest extends FlatSpec with Matchers with MockitoSugar with ScalatestRo
     override def authStatus(context: ServiceRequestContext): OptionT[Future, User] = OptionT.optionT[Future] {
       if (context.contextHeaders.keySet.contains(AuthService.AuthenticationTokenHeader)) {
         Future.successful(Some(new User {
-          override def id: Id[User]     = Id[User](1L)
+          override def id: Id[User]     = Id[User]("1")
           override def roles: Set[Role] = Set(PathologistRole)
         }: User))
       } else {
@@ -33,9 +33,8 @@ class AuthTest extends FlatSpec with Matchers with MockitoSugar with ScalatestRo
   "'authorize' directive" should "throw error is auth token is not in the request" in {
 
     Get("/naive/attempt") ~>
-      authorize(CanSignOutReport) {
-        case user =>
-          complete("Never going to be here")
+      authorize(CanSignOutReport) { user =>
+        complete("Never going to be here")
       } ~>
       check {
         // handled shouldBe false
@@ -50,9 +49,8 @@ class AuthTest extends FlatSpec with Matchers with MockitoSugar with ScalatestRo
     Post("/administration/attempt").addHeader(
       RawHeader(AuthService.AuthenticationTokenHeader, referenceAuthToken.value)
     ) ~>
-      authorize(CanAssignRoles) {
-        case user =>
-          complete("Never going to get here")
+      authorize(CanAssignRoles) { user =>
+        complete("Never going to get here")
       } ~>
       check {
         handled shouldBe false
@@ -70,9 +68,8 @@ class AuthTest extends FlatSpec with Matchers with MockitoSugar with ScalatestRo
     Get("/valid/attempt/?a=2&b=5").addHeader(
       RawHeader(AuthService.AuthenticationTokenHeader, referenceAuthToken.value)
     ) ~>
-      authorize(CanSignOutReport) {
-        case user =>
-          complete("Alright, user \"" + user.id + "\" is authorized")
+      authorize(CanSignOutReport) { user =>
+        complete("Alright, user \"" + user.id + "\" is authorized")
       } ~>
       check {
         handled shouldBe true
diff --git a/src/test/scala/xyz/driver/core/CoreTest.scala b/src/test/scala/xyz/driver/core/CoreTest.scala
index f9a1aab..3eb9eaa 100644
--- a/src/test/scala/xyz/driver/core/CoreTest.scala
+++ b/src/test/scala/xyz/driver/core/CoreTest.scala
@@ -31,12 +31,12 @@ class CoreTest extends FlatSpec with Matchers with MockitoSugar {
 
   "Id" should "have equality and ordering working correctly" in {
 
-    (Id[String](1234213L) === Id[String](1234213L)) should be(true)
-    (Id[String](1234213L) === Id[String](213414L)) should be(false)
-    (Id[String](213414L) === Id[String](1234213L)) should be(false)
+    (Id[String]("1234213") === Id[String]("1234213")) should be(true)
+    (Id[String]("1234213") === Id[String]("213414")) should be(false)
+    (Id[String]("213414") === Id[String]("1234213")) should be(false)
 
-    Seq(Id[String](4L), Id[String](3L), Id[String](2L), Id[String](1L)).sorted should contain
-    theSameElementsInOrderAs(Seq(Id[String](1L), Id[String](2L), Id[String](3L), Id[String](4L)))
+    Seq(Id[String]("4"), Id[String]("3"), Id[String]("2"), Id[String]("1")).sorted should contain
+    theSameElementsInOrderAs(Seq(Id[String]("1"), Id[String]("2"), Id[String]("3"), Id[String]("4")))
   }
 
   "Name" should "have equality and ordering working correctly" in {
diff --git a/src/test/scala/xyz/driver/core/FileTest.scala b/src/test/scala/xyz/driver/core/FileTest.scala
index aba79f7..57af1c2 100644
--- a/src/test/scala/xyz/driver/core/FileTest.scala
+++ b/src/test/scala/xyz/driver/core/FileTest.scala
@@ -51,7 +51,7 @@ class FileTest extends FlatSpec with Matchers with MockitoSugar {
     val s3ObjectMetadataMock = mock[ObjectMetadata]
     val amazonS3Mock         = mock[AmazonS3]
     when(amazonS3Mock.listObjectsV2(any[ListObjectsV2Request]())).thenReturn(s3ResultsMock)
-    when(amazonS3Mock.putObject(testBucket, testFilePath.toString, sourceTestFile)).thenReturn(s3PutMock)
+    when(amazonS3Mock.putObject(testBucket.value, testFilePath.toString, sourceTestFile)).thenReturn(s3PutMock)
     when(amazonS3Mock.getObject(any[GetObjectRequest](), any[File]())).thenReturn(s3ObjectMetadataMock)
 
     val s3Storage = new S3Storage(amazonS3Mock, testBucket, scala.concurrent.ExecutionContext.global)
diff --git a/src/test/scala/xyz/driver/core/GeneratorsTest.scala b/src/test/scala/xyz/driver/core/GeneratorsTest.scala
index 0432b2a..4ec73ec 100644
--- a/src/test/scala/xyz/driver/core/GeneratorsTest.scala
+++ b/src/test/scala/xyz/driver/core/GeneratorsTest.scala
@@ -11,25 +11,25 @@ class GeneratorsTest extends FlatSpec with Matchers with Assertions {
     val generatedId2 = nextId[String]()
     val generatedId3 = nextId[Long]()
 
-    generatedId1 should be >= 0L
-    generatedId2 should be >= 0L
-    generatedId3 should be >= 0L
+    generatedId1.length should be >= 0
+    generatedId2.length should be >= 0
+    generatedId3.length should be >= 0
     generatedId1 should not be generatedId2
     generatedId2 should !==(generatedId3)
   }
 
   it should "be able to generate com.drivergrp.core.Id identifiers with max value" in {
 
-    val generatedLimitedId1 = nextId[String](10000)
-    val generatedLimitedId2 = nextId[String](1000)
-    val generatedLimitedId3 = nextId[Long](2000)
+    val generatedLimitedId1 = nextId[String](5)
+    val generatedLimitedId2 = nextId[String](4)
+    val generatedLimitedId3 = nextId[Long](3)
 
-    generatedLimitedId1 should be >= 0L
-    generatedLimitedId1 should be < 10000L
-    generatedLimitedId2 should be >= 0L
-    generatedLimitedId2 should be < 1000L
-    generatedLimitedId3 should be >= 0L
-    generatedLimitedId3 should be < 2000L
+    generatedLimitedId1.length should be >= 0
+    generatedLimitedId1.length should be < 6
+    generatedLimitedId2.length should be >= 0
+    generatedLimitedId2.length should be < 5
+    generatedLimitedId3.length should be >= 0
+    generatedLimitedId3.length should be < 4
     generatedLimitedId1 should not be generatedLimitedId2
     generatedLimitedId2 should !==(generatedLimitedId3)
   }
@@ -37,11 +37,11 @@ class GeneratorsTest extends FlatSpec with Matchers with Assertions {
   it should "be able to generate com.drivergrp.core.Name names" in {
 
     nextName[String]() should not be nextName[String]()
-    nextName[String]().length should be >= 0
+    nextName[String]().value.length should be >= 0
 
     val fixedLengthName = nextName[String](10)
     fixedLengthName.length should be <= 10
-    assert(!fixedLengthName.exists(_.isControl))
+    assert(!fixedLengthName.value.exists(_.isControl))
   }
 
   it should "be able to generate proper UUIDs" in {
@@ -82,7 +82,7 @@ class GeneratorsTest extends FlatSpec with Matchers with Assertions {
 
     val generatedPair = nextPair(nextId[Int](), nextName[Int]())
 
-    generatedPair._1 should be > 0L
+    generatedPair._1.length should be > 0
     generatedPair._2.length should be > 0
 
     nextPair(nextId[Int](), nextName[Int]()) should not be
@@ -98,7 +98,7 @@ class GeneratorsTest extends FlatSpec with Matchers with Assertions {
 
     val generatedTriad = nextTriad(nextId[Int](), nextName[Int](), nextBigDecimal())
 
-    generatedTriad._1 should be > 0L
+    generatedTriad._1.length should be > 0
     generatedTriad._2.length should be > 0
     generatedTriad._3 should be >= BigDecimal(0.00)
 
diff --git a/src/test/scala/xyz/driver/core/JsonTest.scala b/src/test/scala/xyz/driver/core/JsonTest.scala
index bcdcd5d..c113c59 100644
--- a/src/test/scala/xyz/driver/core/JsonTest.scala
+++ b/src/test/scala/xyz/driver/core/JsonTest.scala
@@ -9,10 +9,10 @@ class JsonTest extends FlatSpec with Matchers {
 
   "Json format for Id" should "read and write correct JSON" in {
 
-    val referenceId = Id[String](1312L)
+    val referenceId = Id[String]("1312-34A")
 
     val writtenJson = json.idFormat.write(referenceId)
-    writtenJson.prettyPrint should be("1312")
+    writtenJson.prettyPrint should be("\"1312-34A\"")
 
     val parsedId = json.idFormat.read(writtenJson)
     parsedId should be(referenceId)