fix: project clean-up process not to log error for 401 on webhook rem…

…oval (#884)
SwissDataScienceCenter · Feb 28, 2022 · 02742b3 · 02742b3
1 parent 7f4d387
commit 02742b3
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 9 deletions.
diff --git a/...ventlog/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemover.scala b/...ventlog/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/...log/events/categories/statuschange/projectCleaner/ProjectWebhookAndTokenRemoverSpec.scala b/...log/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 {