more unit tests clean up

src/test/scala/Generator.scala

@@ -2,31 +2,31 @@ package org.broadinstitute.dsde.workbench.sam
 
 import akka.http.scaladsl.model.headers.{OAuth2BearerToken, RawHeader}
 import org.broadinstitute.dsde.workbench.model.google.ServiceAccountSubjectId
-import org.broadinstitute.dsde.workbench.model.{GoogleSubjectId, WorkbenchEmail, WorkbenchUser, WorkbenchUserId}
+import org.broadinstitute.dsde.workbench.model._
 import org.scalacheck._
 
 object Generator {
   val genNonPetEmail: Gen[WorkbenchEmail] = Gen.alphaStr.map(x => WorkbenchEmail(s"t$x@gmail.com"))
   val genPetEmail: Gen[WorkbenchEmail] = Gen.alphaStr.map(x => WorkbenchEmail(s"t$x@test.iam.gserviceaccount.com"))
-  val genGoogleSubjectId: Gen[GoogleSubjectId] = Gen.calendar.map(x => GoogleSubjectId(x.getTimeInMillis.toString))
   val genServiceAccountSubjectId: Gen[ServiceAccountSubjectId] = Gen.calendar.map(x => ServiceAccountSubjectId(x.getTimeInMillis.toString))
-  val genWorkbenchUserId: Gen[WorkbenchUserId] = Gen.calendar.map(x => WorkbenchUserId(x.getTimeInMillis.toString))
   val genOAuth2BearerToken: Gen[OAuth2BearerToken] = Gen.alphaStr.map(x => OAuth2BearerToken(x))
   val genUserInfoHeadersWithInvalidExpiresIn: Gen[List[RawHeader]] = for{
     email <- genNonPetEmail
-    gSid <- genGoogleSubjectId
     accessToken <- genOAuth2BearerToken
     expiresIn <- Gen.alphaStr
   } yield List(
     RawHeader(api.emailHeader, email.value),
-    RawHeader(api.googleSubjectIdHeader, gSid.value),
+    RawHeader(api.googleSubjectIdHeader, genRandom(System.currentTimeMillis())),
     RawHeader(api.accessTokenHeader, accessToken.value),
     RawHeader(api.expiresInHeader, expiresIn)
   )
+  val genUserInfo = for{
+    token <- genOAuth2BearerToken
+    email <- genNonPetEmail
+    expires <- Gen.calendar.map(_.getTimeInMillis)
+  } yield UserInfo(token, genWorkbenchUserId(System.currentTimeMillis()), email, expires)
 
   implicit val arbNonPetEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genNonPetEmail)
   implicit val arbPetEmail: Arbitrary[WorkbenchEmail] = Arbitrary(genPetEmail)
-  implicit val arbGoogleSubjectId: Arbitrary[GoogleSubjectId] = Arbitrary(genGoogleSubjectId)
-  implicit val arbWorkbenchUserId: Arbitrary[WorkbenchUserId] = Arbitrary(genWorkbenchUserId)
   implicit val arbOAuth2BearerToken: Arbitrary[OAuth2BearerToken] = Arbitrary(genOAuth2BearerToken)
 }

src/test/scala/org/broadinstitute/dsde/workbench/sam/TestSupport.scala

@@ -17,7 +17,7 @@ import org.broadinstitute.dsde.workbench.sam.model._
 import org.broadinstitute.dsde.workbench.sam.openam.MockAccessPolicyDAO
 import org.broadinstitute.dsde.workbench.sam.service._
 import org.scalatest.prop.{Configuration, PropertyChecks}
-import org.scalatest.{Matchers, PropSpec}
+import org.scalatest.{FlatSpec, Matchers}
 
 import scala.concurrent.duration.Duration
 import scala.concurrent.{Await, Awaitable, ExecutionContext}
@@ -29,7 +29,7 @@ trait TestSupport{
   def runAndWait[T](f: Awaitable[T]): T = Await.result(f, Duration.Inf)
 }
 
-trait PropertyBasedTesting extends PropSpec with PropertyChecks with Configuration with Matchers {
+trait PropertyBasedTesting extends FlatSpec with PropertyChecks with Configuration with Matchers {
   implicit override val generatorDrivenConfig: PropertyCheckConfiguration = PropertyCheckConfig(minSize = 3, maxSize = 10)
 }
 

src/test/scala/org/broadinstitute/dsde/workbench/sam/api/ResourceRoutesV1Spec.scala

@@ -9,7 +9,7 @@ import com.typesafe.config.ConfigFactory
 import net.ceedubs.ficus.Ficus._
 import org.broadinstitute.dsde.workbench.model.ErrorReportJsonSupport._
 import org.broadinstitute.dsde.workbench.model._
-import org.broadinstitute.dsde.workbench.sam.TestSupport
+import org.broadinstitute.dsde.workbench.sam.TestSupport.genGoogleSubjectId
 import org.broadinstitute.dsde.workbench.sam.config._
 import org.broadinstitute.dsde.workbench.sam.directory.MockDirectoryDAO
 import org.broadinstitute.dsde.workbench.sam.model.SamJsonSupport._
@@ -51,7 +51,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     val mockStatusService = new StatusService(directoryDAO, NoExtensions)
     val mockManagedGroupService = new ManagedGroupService(mockResourceService, resourceTypes, accessPolicyDAO, directoryDAO, NoExtensions, emailDomain)
 
-    mockUserService.createUser(CreateWorkbenchUserAPI(defaultUserInfo.userId, GoogleSubjectId(genRandom(System.currentTimeMillis())), defaultUserInfo.userEmail))
+    mockUserService.createUser(CreateWorkbenchUserAPI(defaultUserInfo.userId, genGoogleSubjectId(), defaultUserInfo.userEmail))
 
     new TestSamRoutes(mockResourceService, mockUserService, mockStatusService, mockManagedGroupService, userInfo, directoryDAO)
   }
@@ -153,7 +153,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
 
     val authDomainId = ResourceId("myAuthDomain")
     val otherUser = UserInfo(OAuth2BearerToken("magicString"), WorkbenchUserId("bugsBunny"), WorkbenchEmail("bugsford_bunnington@example.com"), 0)
-    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(otherUser.userId, GoogleSubjectId(genRandom(System.currentTimeMillis())), otherUser.userEmail)))
+    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(otherUser.userId, genGoogleSubjectId(), otherUser.userEmail)))
     runAndWait(samRoutes.managedGroupService.createManagedGroup(authDomainId, otherUser))
     val authDomain = Set(WorkbenchGroupName(authDomainId.value))
 
@@ -223,7 +223,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
   private def responsePayloadClue(str: String): String = s" -> Here is the response payload: $str"
 
   private def createUserResourcePolicy(members: AccessPolicyMembership, resourceType: ResourceType, samRoutes: TestSamRoutes, resourceId: ResourceId, policyName: AccessPolicyName): Unit = {
-    val user = CreateWorkbenchUserAPI(samRoutes.userInfo.userId, GoogleSubjectId(genRandom(System.currentTimeMillis())), samRoutes.userInfo.userEmail)
+    val user = CreateWorkbenchUserAPI(samRoutes.userInfo.userId, genGoogleSubjectId(), samRoutes.userInfo.userEmail)
     findOrCreateUser(user, samRoutes.userService)
 
     Post(s"/api/resources/v1/${resourceType.name}/${resourceId.value}") ~> samRoutes.route ~> check {
@@ -306,7 +306,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       status shouldEqual StatusCodes.NoContent
     }
 
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.userService.createUser(testUser))
 
@@ -325,7 +325,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       status shouldEqual StatusCodes.NoContent
     }
 
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.userService.createUser(testUser))
 
@@ -350,7 +350,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       status shouldEqual StatusCodes.NoContent
     }
 
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.userService.createUser(testUser))
 
@@ -372,7 +372,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       status shouldEqual StatusCodes.NoContent
     }
 
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.userService.createUser(testUser))
 
@@ -394,7 +394,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       status shouldEqual StatusCodes.NoContent
     }
 
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
     runAndWait(samRoutes.userService.createUser(testUser))
 
     val badEmail = WorkbenchEmail("null@bar.baz")
@@ -558,7 +558,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     val samRoutes = createSamRoutes(Map(resourceType.name -> resourceType))
 
     runAndWait(samRoutes.resourceService.createResourceType(resourceType))
-    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(WorkbenchUserId("user2"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("user2@example.com"))))
+    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(WorkbenchUserId("user2"), genGoogleSubjectId(), WorkbenchEmail("user2@example.com"))))
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), UserInfo(OAuth2BearerToken("accessToken"), WorkbenchUserId("user2"), WorkbenchEmail("user2@example.com"), 0)))
 
     //Verify resource exists by checking for conflict on recreate
@@ -595,7 +595,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.alterPolicies))),
       ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -614,7 +614,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
       Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.sharePolicy(AccessPolicyName("owner"))))),
       ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -629,7 +629,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // differs from happy case in that we don't create user
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.alterPolicies, ResourceActionPattern("can_compute", "", false)), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.alterPolicies))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -644,7 +644,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // differs from happy case in that owner role does not have alter_policies
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.alterPolicies, SamResourceActionPatterns.sharePolicy), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.sharePolicy(AccessPolicyName("splat"))))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -661,7 +661,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
     val testUser = UserInfo(OAuth2BearerToken("token"), WorkbenchUserId("testuser"), WorkbenchEmail("testuser@foo.com"), 0)
 
-    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(testUser.userId, GoogleSubjectId(genRandom(System.currentTimeMillis())), testUser.userEmail)))
+    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(testUser.userId, genGoogleSubjectId(), testUser.userEmail)))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), testUser))
 
@@ -674,7 +674,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // happy case
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.alterPolicies, ResourceActionPattern("can_compute", "", false)), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.alterPolicies))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -691,7 +691,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // happy case
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.sharePolicy, ResourceActionPattern("can_compute", "", false)), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.sharePolicy(AccessPolicyName("owner"))))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -708,7 +708,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // differs from happy case in that we don't create user
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.alterPolicies, ResourceActionPattern("can_compute", "", false)), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.alterPolicies))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -725,7 +725,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     // differs from happy case in that owner role does not have alter_policies
     val resourceType = ResourceType(ResourceTypeName("rt"), Set(SamResourceActionPatterns.alterPolicies, SamResourceActionPatterns.sharePolicy), Set(ResourceRole(ResourceRoleName("owner"), Set(SamResourceActions.sharePolicy(AccessPolicyName("splat"))))), ResourceRoleName("owner"))
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
-    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), GoogleSubjectId(genRandom(System.currentTimeMillis())), WorkbenchEmail("testuser@foo.com"))
+    val testUser = CreateWorkbenchUserAPI(WorkbenchUserId("testuser"), genGoogleSubjectId(), WorkbenchEmail("testuser@foo.com"))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), defaultUserInfo))
 
@@ -744,7 +744,7 @@ class ResourceRoutesV1Spec extends FlatSpec with Matchers with ScalatestRouteTes
     val samRoutes = TestSamRoutes(Map(resourceType.name -> resourceType))
     val testUser = UserInfo(OAuth2BearerToken("token"), WorkbenchUserId("testuser"), WorkbenchEmail("testuser@foo.com"), 0)
 
-    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(testUser.userId, GoogleSubjectId(genRandom(System.currentTimeMillis())), testUser.userEmail)))
+    runAndWait(samRoutes.userService.createUser(CreateWorkbenchUserAPI(testUser.userId, genGoogleSubjectId(), testUser.userEmail)))
 
     runAndWait(samRoutes.resourceService.createResource(resourceType, ResourceId("foo"), testUser))