From 80c1a82b587710eb33a4dd96564826c101353430 Mon Sep 17 00:00:00 2001
From: Jakub Chrobasik <jakub.chrobasik@gmail.com>
Date: Mon, 28 Feb 2022 11:34:23 +0100
Subject: [PATCH] fix: project clean-up process not to log error for 401 on
 webhook removal

---
 .../ProjectWebhookAndTokenRemover.scala       | 11 +++----
 .../ProjectWebhookAndTokenRemoverSpec.scala   | 30 ++++++++++++++++---
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/event-log/src/main/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemover.scala b/event-log/src/main/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemover.scala
index ea09a55b06..c954752ba5 100644
--- a/event-log/src/main/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemover.scala
+++ b/event-log/src/main/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemover.scala
@@ -27,7 +27,7 @@ import io.renku.graph.webhookservice.WebhookServiceUrl
 import io.renku.http.client.{AccessToken, RestClient}
 import org.http4s.EntityDecoder
 import org.http4s.Method.DELETE
-import org.http4s.Status.{NoContent, NotFound, Ok}
+import org.http4s.Status.{Forbidden, NoContent, NotFound, Ok, Unauthorized}
 import org.http4s.circe.jsonOf
 import org.typelevel.log4cats.Logger
 
@@ -49,9 +49,10 @@ private class ProjectWebhookAndTokenRemoverImpl[F[_]: Async: Logger](accessToken
 ) extends RestClient[F, ProjectWebhookAndTokenRemover[F]](Throttler.noThrottling[F])
     with ProjectWebhookAndTokenRemover[F] {
 
+  import AccessTokenFinder.projectIdToPath
+
   override def removeWebhookAndToken(project: Project): F[Unit] =
-    accessTokenFinder
-      .findAccessToken(project.id)(AccessTokenFinder.projectIdToPath) flatMap {
+    accessTokenFinder.findAccessToken(project.id) >>= {
       case Some(token) => removeProjectWebhook(project, token) >> removeProjectTokens(project)
       case None        => ().pure[F]
     }
@@ -69,9 +70,9 @@ private class ProjectWebhookAndTokenRemoverImpl[F[_]: Async: Logger](accessToken
   private implicit val tokenEntityDecoder: EntityDecoder[F, AccessToken] = jsonOf[F, AccessToken]
 
   private def mapWebhookResponse(project: Project): ResponseMapping[Unit] = {
-    case (Ok | NotFound, _, _) => ().pure[F]
+    case (Ok | NotFound | Unauthorized | Forbidden, _, _) => ().pure[F]
     case (status, _, _) =>
-      new Exception(s"Removing project webhook failed with status: $status for project: ${project.show}")
+      new Exception(show"Removing project webhook failed with status: $status for project: $project")
         .raiseError[F, Unit]
   }
 
diff --git a/event-log/src/test/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemoverSpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemoverSpec.scala
index 6258cfc1cf..1c950b20ba 100644
--- a/event-log/src/test/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemoverSpec.scala
+++ b/event-log/src/test/scala/io/renku/eventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemoverSpec.scala
@@ -33,7 +33,7 @@ import io.renku.interpreters.TestLogger
 import io.renku.stubbing.ExternalServiceStubbing
 import io.renku.testtools.IOSpec
 import org.http4s.Status
-import org.http4s.Status.{InternalServerError, Unauthorized}
+import org.http4s.Status.{Forbidden, InternalServerError, NotFound, ServiceUnavailable, Unauthorized}
 import org.scalamock.scalatest.MockFactory
 import org.scalatest.matchers.should
 import org.scalatest.wordspec.AnyWordSpec
@@ -47,7 +47,7 @@ class ProjectWebhookAndTokenRemoverSpec
 
   "removeWebhookAndToken" should {
 
-    "remove the token and the web hook of the specified project" in new TestCase {
+    "remove the token and the webhook of the specified project" in new TestCase {
       (accesTokenFinder
         .findAccessToken[projects.Id](_: projects.Id)(_: projects.Id => String))
         .expects(project.id, AccessTokenFinder.projectIdToPath)
@@ -65,6 +65,28 @@ class ProjectWebhookAndTokenRemoverSpec
       webhookAndTokenRemover.removeWebhookAndToken(project).unsafeRunSync() shouldBe ()
     }
 
+    NotFound :: Unauthorized :: Forbidden :: Nil foreach { status =>
+      s"remove the token even if the webhook removal returned $status" in new TestCase {
+        (accesTokenFinder
+          .findAccessToken[projects.Id](_: projects.Id)(_: projects.Id => String))
+          .expects(project.id, AccessTokenFinder.projectIdToPath)
+          .returns(accessToken.some.pure[IO])
+        stubFor {
+          delete(s"/projects/${project.id}/webhooks")
+            .withAccessToken(accessToken.some)
+            .willReturn(aResponse().withStatus(status.code))
+        }
+        stubFor {
+          delete(s"/projects/${project.id}/tokens")
+            .willReturn(noContent())
+        }
+
+        webhookAndTokenRemover.removeWebhookAndToken(project).unsafeRunSync() shouldBe ()
+
+        logger.expectNoLogs()
+      }
+    }
+
     "do nothing when the tokenFinder returns no token for the project" in new TestCase {
       (accesTokenFinder
         .findAccessToken[projects.Id](_: projects.Id)(_: projects.Id => String))
@@ -93,11 +115,11 @@ class ProjectWebhookAndTokenRemoverSpec
       stubFor {
         delete(s"/projects/${project.id}/webhooks")
           .withAccessToken(accessToken.some)
-          .willReturn(unauthorized())
+          .willReturn(serviceUnavailable())
       }
       intercept[Exception] {
         webhookAndTokenRemover.removeWebhookAndToken(project).unsafeRunSync()
-      }.getMessage shouldBe s"DELETE $webhookServiceUrl/projects/${project.id}/webhooks returned ${Status.Unauthorized}; error: Removing project webhook failed with status: $Unauthorized for project: ${project.show}"
+      }.getMessage shouldBe s"DELETE $webhookServiceUrl/projects/${project.id}/webhooks returned $ServiceUnavailable; error: Removing project webhook failed with status: $ServiceUnavailable for project: ${project.show}"
     }
 
     "fail when the token deletion fails" in new TestCase {