fix: slug finding in token creation flow to work when project removed

token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/PersistedSlugFinder.scala

@@ -18,7 +18,6 @@
 
 package io.renku.tokenrepository.repository.creation
 
-import cats.Id
 import cats.effect.MonadCancelThrow
 import io.renku.db.DbClient
 import io.renku.graph.model.projects
@@ -27,7 +26,7 @@ import io.renku.tokenrepository.repository.TokenRepositoryTypeSerializers
 import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes
 
 private trait PersistedSlugFinder[F[_]] {
-  def findPersistedProjectSlug(projectId: projects.GitLabId): F[projects.Slug]
+  def findPersistedProjectSlug(projectId: projects.GitLabId): F[Option[projects.Slug]]
 }
 
 private object PersistedSlugFinder {
@@ -43,7 +42,7 @@ private class PersistedSlugFinderImpl[F[_]: MonadCancelThrow: SessionResource: Q
   import io.renku.db.SqlStatement
   import skunk.implicits._
 
-  override def findPersistedProjectSlug(projectId: projects.GitLabId): F[projects.Slug] =
+  override def findPersistedProjectSlug(projectId: projects.GitLabId): F[Option[projects.Slug]] =
     SessionResource[F].useK(measureExecutionTime(query(projectId)))
 
   private def query(projectId: projects.GitLabId) =
@@ -56,5 +55,5 @@ private class PersistedSlugFinderImpl[F[_]: MonadCancelThrow: SessionResource: Q
           .query(projectSlugDecoder)
       )
       .arguments(projectId)
-      .build[Id](_.unique)
+      .build(_.option)
 }

token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/ProjectSlugFinder.scala

@@ -18,31 +18,30 @@
 
 package io.renku.tokenrepository.repository.creation
 
-import cats.data.OptionT
 import cats.effect.Async
 import cats.syntax.all._
 import eu.timepit.refined.auto._
 import io.renku.graph.model.projects
 import io.renku.http.client.{AccessToken, GitLabClient}
+import io.renku.http.tinytypes.TinyTypeURIEncoder._
 import org.typelevel.log4cats.Logger
 
 private trait ProjectSlugFinder[F[_]] {
-  def findProjectSlug(projectId: projects.GitLabId, accessToken: AccessToken): OptionT[F, projects.Slug]
+  def findProjectSlug(projectId: projects.GitLabId, accessToken: AccessToken): F[Option[projects.Slug]]
 }
 
 private class ProjectSlugFinderImpl[F[_]: Async: GitLabClient: Logger] extends ProjectSlugFinder[F] {
 
-  import org.http4s.circe.jsonOf
   import cats.effect._
   import cats.syntax.all._
   import io.circe._
   import org.http4s.Status.{Forbidden, NotFound, Ok, Unauthorized}
   import org.http4s._
+  import org.http4s.circe.jsonOf
   import org.http4s.implicits._
 
-  def findProjectSlug(projectId: projects.GitLabId, accessToken: AccessToken): OptionT[F, projects.Slug] = OptionT {
-    GitLabClient[F].get(uri"projects" / projectId.value, "single-project")(mapResponse)(accessToken.some)
-  }
+  def findProjectSlug(projectId: projects.GitLabId, accessToken: AccessToken): F[Option[projects.Slug]] =
+    GitLabClient[F].get(uri"projects" / projectId, "single-project")(mapResponse)(accessToken.some)
 
   private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[Option[projects.Slug]]] = {
     case (Ok, _, response)                           => response.as[projects.Slug].map(Option.apply)

token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokensCreator.scala

@@ -31,7 +31,7 @@ import io.renku.http.client.AccessToken.ProjectAccessToken
 import io.renku.http.client.{AccessToken, GitLabClient}
 import io.renku.tokenrepository.repository.AccessTokenCrypto.EncryptedAccessToken
 import io.renku.tokenrepository.repository.ProjectsTokensDB.SessionResource
-import io.renku.tokenrepository.repository.deletion.{TokenRemover, TokensRevoker}
+import io.renku.tokenrepository.repository.deletion.{DeletionResult, TokenRemover, TokensRevoker}
 import io.renku.tokenrepository.repository.fetching.PersistedTokensFinder
 import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes
 import org.typelevel.log4cats.Logger
@@ -83,11 +83,19 @@ private class TokensCreatorImpl[F[_]: MonadThrow: Logger](
     case None => Option.empty[AccessToken].pure[F]
     case Some(token) =>
       checkValid(projectId, token) >>= {
-        case true  => token.some.pure[F]
-        case false => tokenRemover.delete(projectId, userToken.some).as(Option.empty)
+        case true => token.some.pure[F]
+        case false =>
+          deleteAndLogSuccess(projectId, userToken, show"Token removed for $projectId as got invalidated in GL")
+            .as(Option.empty)
       }
   }
 
+  private def deleteAndLogSuccess(projectId: projects.GitLabId, mat: AccessToken, message: String) =
+    tokenRemover.delete(projectId, mat.some) >>= {
+      case DeletionResult.Deleted    => Logger[F].info(message)
+      case DeletionResult.NotExisted => ().pure[F]
+    }
+
   private def replaceSlugIfChangedOrRemove(
       projectId: projects.GitLabId,
       userToken: AccessToken
@@ -96,16 +104,19 @@ private class TokensCreatorImpl[F[_]: MonadThrow: Logger](
     case Some(token) =>
       projectSlugFinder
         .findProjectSlug(projectId, token)
-        .semiflatMap(actualSlug =>
-          findPersistedProjectSlug(projectId).flatMap {
-            case `actualSlug` => ().pure[F]
-            case _            => updateSlug(Project(projectId, actualSlug))
-          }
-        )
-        .cataF(
-          default = tokenRemover.delete(projectId, userToken.some).as(Option.empty),
-          _ => token.some.pure[F]
-        )
+        .flatMap {
+          case None =>
+            deleteAndLogSuccess(projectId,
+                                userToken,
+                                show"Token removed for $projectId as project does not exist in GL"
+            ).as(Option.empty)
+          case Some(actualGLSlug) =>
+            findPersistedProjectSlug(projectId) >>= {
+              case Some(`actualGLSlug`) => token.some.pure[F]
+              case Some(_)              => updateSlug(Project(projectId, actualGLSlug)).as(token.some)
+              case _                    => Option.empty[AccessToken].pure[F]
+            }
+        }
   }
 
   private def checkIfDue(projectId: projects.GitLabId): Option[AccessToken] => F[Option[AccessToken]] = {
@@ -123,7 +134,7 @@ private class TokensCreatorImpl[F[_]: MonadThrow: Logger](
     }
 
   private def findProjectSlug(projectId: projects.GitLabId, userToken: AccessToken): OptionT[F, Project] =
-    projectSlugFinder.findProjectSlug(projectId, userToken).map(Project(projectId, _))
+    OptionT(projectSlugFinder.findProjectSlug(projectId, userToken)).map(Project(projectId, _))
 
   private def createNewToken(userToken: AccessToken)(project: Project): OptionT[F, (Project, TokenCreationInfo)] =
     createProjectAccessToken(project.id, userToken).map(project -> _)

token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/DeleteTokenEndpoint.scala

@@ -40,10 +40,15 @@ class DeleteTokenEndpointImpl[F[_]: Async: Logger](tokenRemover: TokenRemover[F]
     with DeleteTokenEndpoint[F] {
 
   override def deleteToken(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Response[F]] =
-    (tokenRemover.delete(projectId, maybeAccessToken) >> NoContent())
-      .recoverWith(httpResult(projectId))
+    (tokenRemover.delete(projectId, maybeAccessToken).flatMap(logSuccess(projectId)) >> NoContent())
+      .recoverWith(errorHttpResult(projectId))
 
-  private def httpResult(projectId: GitLabId): PartialFunction[Throwable, F[Response[F]]] = {
+  private def logSuccess(projectId: GitLabId): DeletionResult => F[Unit] = {
+    case DeletionResult.Deleted    => Logger[F].info(show"Token removed for $projectId")
+    case DeletionResult.NotExisted => ().pure[F]
+  }
+
+  private def errorHttpResult(projectId: GitLabId): PartialFunction[Throwable, F[Response[F]]] = {
     case NonFatal(exception) =>
       val message = Message.Error.unsafeApply(s"Deleting token for projectId: $projectId failed")
       Logger[F].error(exception)(message.show) >> InternalServerError(message)

token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/DeletionResult.scala

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2023 Swiss Data Science Center (SDSC)
+ * A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and
+ * Eidgenössische Technische Hochschule Zürich (ETHZ).
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.renku.tokenrepository.repository.deletion
+
+sealed trait DeletionResult {
+  lazy val widen: DeletionResult = this
+}
+
+object DeletionResult {
+  case object Deleted    extends DeletionResult
+  case object NotExisted extends DeletionResult
+}

token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/PersistedTokenRemover.scala

@@ -25,25 +25,24 @@ import io.renku.graph.model.projects.GitLabId
 import io.renku.tokenrepository.repository.ProjectsTokensDB.SessionResource
 import io.renku.tokenrepository.repository.TokenRepositoryTypeSerializers
 import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes
-import org.typelevel.log4cats.Logger
 import skunk.data.Completion
 import skunk.implicits._
 
 private[repository] trait PersistedTokenRemover[F[_]] {
-  def delete(projectId: GitLabId): F[Unit]
+  def delete(projectId: GitLabId): F[DeletionResult]
 }
 
 private[repository] object PersistedTokenRemover {
-  def apply[F[_]: MonadCancelThrow: SessionResource: Logger: QueriesExecutionTimes]: PersistedTokenRemover[F] =
+  def apply[F[_]: MonadCancelThrow: SessionResource: QueriesExecutionTimes]: PersistedTokenRemover[F] =
     new PersistedTokenRemoverImpl[F]
 }
 
-private class PersistedTokenRemoverImpl[F[_]: MonadCancelThrow: SessionResource: Logger: QueriesExecutionTimes]
+private class PersistedTokenRemoverImpl[F[_]: MonadCancelThrow: SessionResource: QueriesExecutionTimes]
     extends DbClient[F](Some(QueriesExecutionTimes[F]))
     with PersistedTokenRemover[F]
     with TokenRepositoryTypeSerializers {
 
-  override def delete(projectId: GitLabId): F[Unit] =
+  override def delete(projectId: GitLabId): F[DeletionResult] =
     SessionResource[F].useK {
       query(projectId)
     }
@@ -58,14 +57,14 @@ private class PersistedTokenRemoverImpl[F[_]: MonadCancelThrow: SessionResource:
       .flatMapResult(failIfMultiUpdate(projectId))
   }
 
-  private def failIfMultiUpdate(projectId: GitLabId): Completion => F[Unit] = {
-    case Completion.Delete(0) => ().pure[F]
-    case Completion.Delete(1) => Logger[F].info(show"token removed for $projectId")
+  private def failIfMultiUpdate(projectId: GitLabId): Completion => F[DeletionResult] = {
+    case Completion.Delete(0) => DeletionResult.NotExisted.pure[F].widen
+    case Completion.Delete(1) => DeletionResult.Deleted.pure[F].widen
     case Completion.Delete(n) =>
       new RuntimeException(s"Deleting token for a projectId: $projectId removed $n records")
-        .raiseError[F, Unit]
+        .raiseError[F, DeletionResult]
     case completion =>
       new RuntimeException(s"Deleting token for a projectId: $projectId failed with completion code $completion")
-        .raiseError[F, Unit]
+        .raiseError[F, DeletionResult]
   }
 }

token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/TokenRemover.scala

@@ -28,22 +28,22 @@ import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes
 import org.typelevel.log4cats.Logger
 
 private[repository] trait TokenRemover[F[_]] {
-  def delete(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Unit]
+  def delete(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[DeletionResult]
 }
 
 private[repository] object TokenRemover {
   def apply[F[_]: Async: GitLabClient: SessionResource: Logger: QueriesExecutionTimes]: F[TokenRemover[F]] =
     TokensRevoker[F].map(new TokenRemoverImpl[F](PersistedTokenRemover[F], _))
 }
 
-private class TokenRemoverImpl[F[_]: MonadThrow: Logger](dbTokenRemover: PersistedTokenRemover[F],
-                                                         tokensRevoker: TokensRevoker[F]
+private class TokenRemoverImpl[F[_]: MonadThrow](dbTokenRemover: PersistedTokenRemover[F],
+                                                 tokensRevoker: TokensRevoker[F]
 ) extends TokenRemover[F] {
 
   import tokensRevoker._
 
-  override def delete(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Unit] =
-    dbTokenRemover.delete(projectId) >> revokeTokens(projectId, maybeAccessToken)
+  override def delete(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[DeletionResult] =
+    dbTokenRemover.delete(projectId).flatTap(_ => revokeTokens(projectId, maybeAccessToken))
 
   private def revokeTokens(projectId: GitLabId, maybeAccessToken: Option[AccessToken]) =
     maybeAccessToken match {

token-repository/src/test/scala/io/renku/tokenrepository/repository/InMemoryProjectsTokensDb.scala

@@ -20,9 +20,9 @@ package io.renku.tokenrepository.repository
 
 import cats.data.Kleisli
 import cats.effect.IO
+import cats.effect.unsafe.IORuntime
 import com.dimafeng.testcontainers._
 import io.renku.db.PostgresContainer
-import io.renku.testtools.IOSpec
 import io.renku.tokenrepository.repository.ProjectsTokensDB.SessionResource
 import natchez.Trace.Implicits.noop
 import org.scalatest.Suite
@@ -31,7 +31,9 @@ import skunk.codec.all._
 import skunk.implicits._
 
 trait InMemoryProjectsTokensDb extends ForAllTestContainer with TokenRepositoryTypeSerializers {
-  self: Suite with IOSpec =>
+  self: Suite =>
+
+  implicit val ioRuntime: IORuntime
 
   private val dbConfig = new ProjectsTokensDbConfigProvider[IO].get().unsafeRunSync()
 

token-repository/src/test/scala/io/renku/tokenrepository/repository/RepositoryGenerators.scala

@@ -19,6 +19,7 @@
 package io.renku.tokenrepository.repository
 
 import AccessTokenCrypto.EncryptedAccessToken
+import deletion.DeletionResult
 import eu.timepit.refined.api.RefType
 import io.renku.generators.Generators.Implicits._
 import io.renku.generators.Generators._
@@ -38,4 +39,7 @@ private object RepositoryGenerators {
 
   val accessTokenIds: Gen[AccessTokenId] =
     positiveInts().toGeneratorOf(v => AccessTokenId(v.value))
+
+  val deletionResults: Gen[DeletionResult] =
+    Gen.oneOf(DeletionResult.Deleted, DeletionResult.NotExisted)
 }

token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/PersistedSlugFinderSpec.scala

@@ -21,45 +21,33 @@ package io.renku.tokenrepository.repository.creation
 import cats.effect.IO
 import io.renku.generators.Generators.Implicits._
 import io.renku.graph.model.GraphModelGenerators._
-import io.renku.metrics.TestMetricsRegistry
 import io.renku.testtools.IOSpec
 import io.renku.tokenrepository.repository.InMemoryProjectsTokensDbSpec
 import io.renku.tokenrepository.repository.RepositoryGenerators.encryptedAccessTokens
-import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes
 import org.scalamock.scalatest.MockFactory
+import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should
-import org.scalatest.wordspec.AnyWordSpec
 
 class PersistedSlugFinderSpec
-    extends AnyWordSpec
+    extends AnyFlatSpec
     with IOSpec
     with InMemoryProjectsTokensDbSpec
     with should.Matchers
     with MockFactory {
 
-  "findPersistedProjectSlug" should {
+  it should "return Slug for the given project Id" in {
 
-    "return Slug for the given project Id" in new TestCase {
+    val projectId   = projectIds.generateOne
+    val projectSlug = projectSlugs.generateOne
 
-      val projectSlug = projectSlugs.generateOne
+    insert(projectId, projectSlug, encryptedAccessTokens.generateOne)
 
-      insert(projectId, projectSlug, encryptedAccessTokens.generateOne)
-
-      (finder findPersistedProjectSlug projectId).unsafeRunSync() shouldBe projectSlug
-    }
-
-    "fail if there's no Slug for the given Id" in new TestCase {
-      intercept[Exception] {
-        (finder findPersistedProjectSlug projectId).unsafeRunSync()
-      }
-    }
+    (finder findPersistedProjectSlug projectId).unsafeRunSync() shouldBe Some(projectSlug)
   }
 
-  private trait TestCase {
-    val projectId = projectIds.generateOne
-
-    private implicit val metricsRegistry:  TestMetricsRegistry[IO]   = TestMetricsRegistry[IO]
-    private implicit val queriesExecTimes: QueriesExecutionTimes[IO] = QueriesExecutionTimes[IO]().unsafeRunSync()
-    val finder = new PersistedSlugFinderImpl[IO]
+  it should "return None if there's no Slug with the given Id" in {
+    (finder findPersistedProjectSlug projectIds.generateOne).unsafeRunSync() shouldBe None
   }
+
+  private lazy val finder = new PersistedSlugFinderImpl[IO]
 }