Fetch UserState lazily after authentication and refactor

modules/core/shared/src/main/scala/scaladex/core/model/UserInfo.scala

@@ -9,18 +9,9 @@ import scaladex.core.util.Secret
  * @param name the real name of the user
  * @param avatarUrl the avatar icon
  */
-case class UserInfo(
-    login: String,
-    name: Option[String],
-    avatarUrl: String,
-    token: Secret
-) extends AvatarUrl
+case class UserInfo(login: String, name: Option[String], avatarUrl: String, token: Secret) extends AvatarUrl
 
-case class UserState(
-    repos: Set[Project.Reference],
-    orgs: Set[Project.Organization],
-    info: UserInfo
-) {
+case class UserState(repos: Set[Project.Reference], orgs: Set[Project.Organization], info: UserInfo) {
   def isAdmin(env: Env): Boolean = orgs.contains(Project.Organization("scalacenter")) || env.isLocal
   def canEdit(githubRepo: Project.Reference, env: Env): Boolean =
     isAdmin(env) || repos.contains(githubRepo)

modules/core/shared/src/main/scala/scaladex/core/service/GithubAuth.scala

@@ -2,9 +2,12 @@ package scaladex.core.service
 
 import scala.concurrent.Future
 
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
+import scaladex.core.util.Secret
 
 trait GithubAuth {
-  def getUserStateWithToken(token: String): Future[UserState]
-  def getUserStateWithOauth2(code: String): Future[UserState]
+  def getToken(code: String): Future[Secret]
+  def getUser(token: Secret): Future[UserInfo]
+  def getUserState(token: Secret): Future[Option[UserState]]
 }

modules/core/shared/src/main/scala/scaladex/core/service/GithubClient.scala

@@ -5,10 +5,12 @@ import scala.concurrent.Future
 import scaladex.core.model.GithubInfo
 import scaladex.core.model.GithubResponse
 import scaladex.core.model.Project
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 
 trait GithubClient {
   def getProjectInfo(ref: Project.Reference): Future[GithubResponse[(Project.Reference, GithubInfo)]]
+  def getUserInfo(): Future[GithubResponse[UserInfo]]
   def getUserState(): Future[GithubResponse[UserState]]
   def getUserOrganizations(login: String): Future[Seq[Project.Organization]]
   def getUserRepositories(login: String, filterPermissions: Seq[String]): Future[Seq[Project.Reference]]

modules/core/shared/src/main/scala/scaladex/core/service/WebDatabase.scala

@@ -14,6 +14,7 @@ import scaladex.core.model.Platform
 import scaladex.core.model.Project
 import scaladex.core.model.ProjectDependency
 import scaladex.core.model.SemanticVersion
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 import scaladex.core.web.ArtifactsPageParams
 
@@ -57,9 +58,10 @@ trait WebDatabase {
   def getProjectDependencies(ref: Project.Reference, version: SemanticVersion): Future[Seq[ProjectDependency]]
   def getProjectDependents(ref: Project.Reference): Future[Seq[ProjectDependency]]
 
-  // sessions
-  def insertSession(userId: UUID, userState: UserState): Future[Unit]
-  def getSession(userId: UUID): Future[Option[UserState]]
-  def getAllSessions(): Future[Seq[(UUID, UserState)]]
-  def deleteSession(userId: UUID): Future[Unit]
+  // users
+  def insertUser(userId: UUID, user: UserInfo): Future[Unit]
+  def updateUser(userId: UUID, userState: UserState): Future[Unit]
+  def getUser(userId: UUID): Future[Option[UserState]]
+  def getAllUsers(): Future[Seq[(UUID, UserInfo)]]
+  def deleteUser(userId: UUID): Future[Unit]
 }

modules/core/shared/src/test/scala/scaladex/core/test/InMemoryDatabase.scala

@@ -16,6 +16,7 @@ import scaladex.core.model.Project
 import scaladex.core.model.ProjectDependency
 import scaladex.core.model.ReleaseDependency
 import scaladex.core.model.SemanticVersion
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 import scaladex.core.service.SchedulerDatabase
 import scaladex.core.web.ArtifactsPageParams
@@ -139,10 +140,11 @@ class InMemoryDatabase extends SchedulerDatabase {
   override def updateArtifacts(allArtifacts: Seq[Artifact], newRef: Project.Reference): Future[Int] = ???
   override def getAllGroupIds(): Future[Seq[Artifact.GroupId]] = ???
   override def getAllMavenReferences(): Future[Seq[Artifact.MavenReference]] = ???
-  override def insertSession(userId: UUID, userState: UserState): Future[Unit] = ???
-  override def getSession(userId: UUID): Future[Option[UserState]] = ???
-  override def getAllSessions(): Future[Seq[(UUID, UserState)]] = ???
-  override def deleteSession(userId: UUID): Future[Unit] = ???
+  override def insertUser(userId: UUID, userInfo: UserInfo): Future[Unit] = ???
+  override def updateUser(userId: UUID, userInfo: UserState): Future[Unit] = ???
+  override def getUser(userId: UUID): Future[Option[UserState]] = ???
+  override def getAllUsers(): Future[Seq[(UUID, UserInfo)]] = ???
+  override def deleteUser(userId: UUID): Future[Unit] = ???
   override def updateArtifactReleaseDate(reference: Artifact.MavenReference, releaseDate: Instant): Future[Int] = ???
 
   override def updateGithubInfoAndStatus(

modules/core/shared/src/test/scala/scaladex/core/test/MockGithubAuth.scala

@@ -31,15 +31,18 @@ object MockGithubAuth extends GithubAuth {
     val userState: UserState = UserState(projects, organizations, info)
   }
 
-  private val users: Map[String, UserState] = Map(
-    Sonatype.token -> Sonatype.userState,
-    Admin.token -> Admin.userState,
-    Typelevel.token -> Typelevel.userState
+  private val users: Map[Secret, UserState] = Map(
+    Secret(Sonatype.token) -> Sonatype.userState,
+    Secret(Admin.token) -> Admin.userState,
+    Secret(Typelevel.token) -> Typelevel.userState
   )
 
-  override def getUserStateWithToken(token: String): Future[UserState] =
-    Future.successful(users(token))
-
-  override def getUserStateWithOauth2(code: String): Future[UserState] =
+  override def getToken(code: String): Future[Secret] =
     ???
+
+  override def getUser(token: Secret): Future[UserInfo] =
+    Future.successful(users(token).info)
+
+  override def getUserState(token: Secret): Future[Option[UserState]] =
+    Future.successful(users.get(token))
 }

modules/infra/src/main/resources/migrations/V18__alter_user_session.sql

@@ -0,0 +1,3 @@
+ALTER TABLE user_sessions
+  ALTER COLUMN repos SET DEFAULT '',
+  ALTER COLUMN orgs SET DEFAULT '';

modules/infra/src/main/scala/scaladex/infra/SqlDatabase.scala

@@ -20,6 +20,7 @@ import scaladex.core.model.Project
 import scaladex.core.model.ProjectDependency
 import scaladex.core.model.ReleaseDependency
 import scaladex.core.model.SemanticVersion
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 import scaladex.core.service.SchedulerDatabase
 import scaladex.core.web.ArtifactsPageParams
@@ -211,17 +212,20 @@ class SqlDatabase(datasource: HikariDataSource, xa: doobie.Transactor[IO]) exten
   override def getAllMavenReferences(): Future[Seq[Artifact.MavenReference]] =
     run(ArtifactTable.selectMavenReference.to[Seq])
 
-  override def insertSession(userId: UUID, userState: UserState): Future[Unit] =
-    run(UserSessionsTable.insertOrUpdate.run((userId, userState)).map(_ => ()))
+  override def insertUser(userId: UUID, userInfo: UserInfo): Future[Unit] =
+    run(UserSessionsTable.insert.run((userId, userInfo)).map(_ => ()))
 
-  override def getSession(userId: UUID): Future[Option[UserState]] =
-    run(UserSessionsTable.selectUserSessionById.to[Seq](userId)).map(_.headOption)
+  override def updateUser(userId: UUID, userState: UserState): Future[Unit] =
+    run(UserSessionsTable.update.run((userState, userId)).map(_ => ()))
 
-  override def getAllSessions(): Future[Seq[(UUID, UserState)]] =
-    run(UserSessionsTable.selectAllUserSessions.to[Seq])
+  override def getUser(userId: UUID): Future[Option[UserState]] =
+    run(UserSessionsTable.selectById.option(userId))
 
-  override def deleteSession(userId: UUID): Future[Unit] =
-    run(UserSessionsTable.deleteByUserId.run(userId).map(_ => ()))
+  override def getAllUsers(): Future[Seq[(UUID, UserInfo)]] =
+    run(UserSessionsTable.selectAll.to[Seq])
+
+  override def deleteUser(userId: UUID): Future[Unit] =
+    run(UserSessionsTable.deleteById.run(userId).map(_ => ()))
 
   override def getArtifacts(
       ref: Project.Reference,

modules/infra/src/main/scala/scaladex/infra/sql/UserSessionsTable.scala

@@ -5,6 +5,7 @@ import java.util.UUID
 import doobie.Query0
 import doobie.util.query.Query
 import doobie.util.update.Update
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 import scaladex.infra.sql.DoobieUtils.Mappings._
 import scaladex.infra.sql.DoobieUtils._
@@ -13,19 +14,21 @@ object UserSessionsTable {
 
   private[sql] val table = "user_sessions"
   private val userId = "user_id"
-  private val userStateFields = Seq("repos", "orgs")
   private val userInfoFields = Seq("login", "name", "avatar_url", "secret")
-  private val allFields = userId +: (userStateFields ++ userInfoFields)
+  private val userStateFields = Seq("repos", "orgs") ++ userInfoFields
 
-  val insertOrUpdate: Update[(UUID, UserState)] =
-    insertOrUpdateRequest(table, allFields, Seq(userId))
+  val insert: Update[(UUID, UserInfo)] =
+    insertRequest(table, userId +: userInfoFields)
 
-  val selectUserSessionById: Query[UUID, UserState] =
-    selectRequest(table, userStateFields ++ userInfoFields, Seq("user_id"))
+  val update: Update[(UserState, UUID)] =
+    updateRequest(table, userStateFields, Seq(userId))
 
-  val selectAllUserSessions: Query0[(UUID, UserState)] =
-    selectRequest(table, fields = allFields)
+  val selectById: Query[UUID, UserState] =
+    selectRequest(table, userStateFields, Seq("user_id"))
 
-  val deleteByUserId: Update[UUID] =
+  val selectAll: Query0[(UUID, UserInfo)] =
+    selectRequest(table, fields = userId +: userInfoFields)
+
+  val deleteById: Update[UUID] =
     deleteRequest(table, Seq(userId))
 }

modules/infra/src/test/scala/scaladex/infra/SqlDatabaseTests.scala

@@ -224,13 +224,16 @@ class SqlDatabaseTests extends AsyncFunSpec with BaseDatabaseSuite with Matchers
     } yield catsDependents shouldBe 0
   }
 
-  it("should insert and get user session from id") {
+  it("should insert, update and get user session from id") {
     val userId = UUID.randomUUID()
-    val userInfo = UserInfo("login", Some("name"), "", new Secret("token"))
+    val userInfo = UserInfo("login", Some("name"), "", Secret("token"))
     val userState = UserState(Set(Scalafix.reference), Set(Organization("scalacenter")), userInfo)
     for {
-      _ <- database.insertSession(userId, userState)
-      obtained <- database.getSession(userId)
+      _ <- database.insertUser(userId, userInfo)
+      state1 <- database.getUser(userId)
+      _ = state1 shouldBe Some(UserState(Set.empty, Set.empty, userInfo))
+      _ <- database.updateUser(userId, userState)
+      obtained <- database.getUser(userId)
     } yield obtained shouldBe Some(userState)
   }
 
@@ -239,32 +242,21 @@ class SqlDatabaseTests extends AsyncFunSpec with BaseDatabaseSuite with Matchers
     val secondUserId = UUID.randomUUID()
     val firstUserInfo = UserInfo("first login", Some("first name"), "", Secret("firstToken"))
     val secondUserInfo = UserInfo("second login", Some("second name"), "", Secret("secondToken"))
-    val firstUserState = UserState(Set(Scalafix.reference), Set(Organization("scalacenter")), firstUserInfo)
-    val secondUserState = UserState(Set(Cats.reference), Set(Organization("typelevel")), secondUserInfo)
     for {
-      _ <- database.insertSession(firstUserId, firstUserState)
-      _ <- database.insertSession(secondUserId, secondUserState)
-      storedUserStates <- database.getAllSessions()
-    } yield {
-      val userStateMap = storedUserStates.toMap
-      userStateMap.get(firstUserId).map(_.info.token.decode) shouldBe Some("firstToken")
-      userStateMap.get(secondUserId).map(_.info.token.decode) shouldBe Some("secondToken")
-    }
+      _ <- database.insertUser(firstUserId, firstUserInfo)
+      _ <- database.insertUser(secondUserId, secondUserInfo)
+      allUsers <- database.getAllUsers()
+    } yield allUsers should contain theSameElementsAs Seq(firstUserId -> firstUserInfo, secondUserId -> secondUserInfo)
   }
 
   it("should delete by user id") {
     val userId = UUID.randomUUID()
     val userInfo = UserInfo("login", Some("name"), "", new Secret("token"))
-    val userState = UserState(Set(Scalafix.reference), Set(Organization("scalacenter")), userInfo)
     for {
-      _ <- database.insertSession(userId, userState)
-      maybeUserStateBeforeDeletion <- database.getSession(userId)
-      _ <- database.deleteSession(userId)
-      maybeUserStateAfterDeletion <- database.getSession(userId)
-    } yield {
-      maybeUserStateBeforeDeletion shouldBe Some(userState)
-      maybeUserStateAfterDeletion shouldBe None
-    }
+      _ <- database.insertUser(userId, userInfo)
+      _ <- database.deleteUser(userId)
+      obtained <- database.getUser(userId)
+    } yield obtained shouldBe None
   }
 
   it("should return artifact from maven reference") {

modules/infra/src/test/scala/scaladex/infra/sql/UserSessionsTableTests.scala

@@ -5,8 +5,9 @@ import org.scalatest.matchers.should.Matchers
 import scaladex.infra.BaseDatabaseSuite
 
 class UserSessionsTableTests extends AnyFunSpec with BaseDatabaseSuite with Matchers {
-  it("check insertOrUpdate")(check(UserSessionsTable.insertOrUpdate))
-  it("check selectUserSessionById")(check(UserSessionsTable.selectUserSessionById))
-  it("check selectAllUserSessions")(check(UserSessionsTable.selectAllUserSessions))
-  it("check deleteByUserId")(check(UserSessionsTable.deleteByUserId))
+  it("check insert")(check(UserSessionsTable.insert))
+  it("check update")(check(UserSessionsTable.update))
+  it("check selectById")(check(UserSessionsTable.selectById))
+  it("check selectAll")(check(UserSessionsTable.selectAll))
+  it("check deleteById")(check(UserSessionsTable.deleteById))
 }

modules/server/src/main/scala/scaladex/server/GithubAuthImpl.scala

@@ -1,5 +1,6 @@
 package scaladex.server
 
+import scala.collection.concurrent.TrieMap
 import scala.concurrent.Future
 
 import akka.actor.ActorSystem
@@ -11,43 +12,29 @@ import akka.http.scaladsl.model.headers._
 import akka.http.scaladsl.unmarshalling.Unmarshal
 import com.typesafe.scalalogging.LazyLogging
 import scaladex.core.model.GithubResponse
+import scaladex.core.model.UserInfo
 import scaladex.core.model.UserState
 import scaladex.core.service.GithubAuth
+import scaladex.core.service.GithubClient
 import scaladex.core.util.Secret
 import scaladex.infra.GithubClientImpl
+import scaladex.server.config.OAuth2Config
 
 object Response {
   case class AccessToken(access_token: String) {
     val token: Secret = Secret(access_token)
   }
 }
 //todo: remove Json4sSupport
-class GithubAuthImpl(clientId: String, clientSecret: String, redirectUri: String)(implicit sys: ActorSystem)
+private class GithubAuthImpl(clientId: String, clientSecret: String, redirectUri: String)(implicit sys: ActorSystem)
     extends GithubAuth
     with Json4sSupport
     with LazyLogging {
   import sys.dispatcher
 
-  def getUserStateWithToken(token: String): Future[UserState] = getUserState(Secret(token))
+  private val githubClients: TrieMap[Secret, GithubClient] = TrieMap()
 
-  def getUserStateWithOauth2(code: String): Future[UserState] =
-    for {
-      token <- getTokenWithOauth2(code)
-      userState <- getUserState(token)
-    } yield userState
-
-  private def getUserState(token: Secret): Future[UserState] = {
-    val githubClient = new GithubClientImpl(token)
-    githubClient.getUserState().flatMap {
-      case GithubResponse.Ok(res)               => Future.successful(res)
-      case GithubResponse.MovedPermanently(res) => Future.successful(res)
-      case GithubResponse.Failed(errorCode, errorMessage) =>
-        val message = s"Call to GithubClient#getUserState failed with code: $errorCode, message: $errorMessage"
-        Future.failed(new Exception(message))
-    }
-  }
-
-  private def getTokenWithOauth2(code: String): Future[Secret] =
+  def getToken(code: String): Future[Secret] =
     Http()
       .singleRequest(
         HttpRequest(
@@ -64,4 +51,31 @@ class GithubAuthImpl(clientId: String, clientSecret: String, redirectUri: String
         )
       )
       .flatMap(response => Unmarshal(response).to[Response.AccessToken].map(_.token))
+
+  def getUser(token: Secret): Future[UserInfo] = {
+    val githubClient = githubClients.getOrElseUpdate(token, new GithubClientImpl(token))
+    githubClient.getUserInfo().map {
+      case GithubResponse.Ok(res)               => res
+      case GithubResponse.MovedPermanently(res) => res
+      case GithubResponse.Failed(errorCode, errorMessage) =>
+        val message = s"Failed to get user state: $errorCode, $errorMessage"
+        throw new Exception(message)
+    }
+  }
+
+  def getUserState(token: Secret): Future[Option[UserState]] = {
+    val githubClient = githubClients.getOrElseUpdate(token, new GithubClientImpl(token))
+    githubClient.getUserState().map {
+      case GithubResponse.Ok(userState)               => Some(userState)
+      case GithubResponse.MovedPermanently(userState) => Some(userState)
+      case GithubResponse.Failed(errorCode, errorMessage) =>
+        logger.warn(s"Failed to get user state: $errorCode, $errorMessage")
+        None
+    }
+  }
+}
+
+object GithubAuthImpl {
+  def apply(config: OAuth2Config)(implicit sys: ActorSystem): GithubAuth =
+    new GithubAuthImpl(config.clientId, config.clientSecret, config.redirectUri)
 }