diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/CommitHistoryChangesSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/CommitHistoryChangesSpec.scala index dd96783d5c..08e316f3ac 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/CommitHistoryChangesSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/CommitHistoryChangesSpec.scala @@ -125,7 +125,7 @@ class CommitHistoryChangesSpec sleep((1 second).toMillis) - `check no hook exists`(project.id, user.accessToken) + `check hook cannot be found`(project.id, user.accessToken) Then("the project and its datasets should be removed from the knowledge-graph") diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/EventsProcessingStatusSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/EventsProcessingStatusSpec.scala deleted file mode 100644 index fdc4a8ccca..0000000000 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/EventsProcessingStatusSpec.scala +++ /dev/null @@ -1,95 +0,0 @@ -/* - * 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.graph.acceptancetests - -import data._ -import data.Project.Statistics.CommitsCount -import eu.timepit.refined.api.Refined -import eu.timepit.refined.auto._ -import eu.timepit.refined.numeric.Positive -import flows.{AccessTokenPresence, TSProvisioning} -import io.circe.Json -import io.renku.generators.CommonGraphGenerators.authUsers -import io.renku.generators.Generators.Implicits._ -import io.renku.graph.model.EventsGenerators.commitIds -import io.renku.graph.model.events.EventStatusProgress -import io.renku.graph.model.testentities.generators.EntitiesGenerators._ -import org.http4s.Status._ -import org.scalatest.concurrent.Eventually -import testing.AcceptanceTestPatience -import tooling.{AcceptanceSpec, ApplicationServices, ModelImplicits} - -class EventsProcessingStatusSpec - extends AcceptanceSpec - with ModelImplicits - with ApplicationServices - with TSProvisioning - with AccessTokenPresence - with Eventually - with AcceptanceTestPatience { - - private val numberOfEvents: Int Refined Positive = 5 - - Feature("Status of processing for a given project") { - - Scenario("As a user I would like to see processing status of my project") { - - val user = authUsers.generateOne - val project = - dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), - CommitsCount(numberOfEvents.value) - ).map(addMemberWithId(user.id)).generateOne - - gitLabStub.addAuthenticated(user) - - When("there's no webhook for a given project in GitLab") - Then("the status endpoint should return OK with 'activated' = false") - - val response = webhookServiceClient.fetchProcessingStatus(project.id, user.accessToken) - response.status shouldBe Ok - response.jsonBody.hcursor.downField("activated").as[Boolean].value shouldBe false - - When("a webhook created for the project") - And("there are events under processing") - val allCommitIds = commitIds.generateNonEmptyList(min = numberOfEvents, max = numberOfEvents) - gitLabStub.setupProject(project, allCommitIds.toList: _*) - mockCommitDataOnTripleGenerator(project, toPayloadJsonLD(project), allCommitIds) - `data in the Triples Store`(project, allCommitIds, user.accessToken) - - Then("the status endpoint should return OK with some progress info") - eventually { - val response = webhookServiceClient.fetchProcessingStatus(project.id, user.accessToken) - - response.status shouldBe Ok - - val responseJson = response.jsonBody.hcursor - responseJson.downField("activated").as[Boolean].value shouldBe true - - val progressObjCursor = responseJson.downField("progress").as[Json].fold(throw _, identity).hcursor - progressObjCursor.downField("done").as[Int].value shouldBe EventStatusProgress.Stage.Final.value - progressObjCursor.downField("total").as[Int].value shouldBe EventStatusProgress.Stage.Final.value - progressObjCursor.downField("percentage").as[Float].value shouldBe 100f - - val detailsObjCursor = responseJson.downField("details").as[Json].fold(throw _, identity).hcursor - detailsObjCursor.downField("status").as[String].value shouldBe "success" - detailsObjCursor.downField("message").as[String].value shouldBe "triples store" - } - } - } -} diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/ProjectStatusResourceSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/ProjectStatusResourceSpec.scala new file mode 100644 index 0000000000..53b7412bb6 --- /dev/null +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/ProjectStatusResourceSpec.scala @@ -0,0 +1,129 @@ +/* + * 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.graph.acceptancetests + +import eu.timepit.refined.api.Refined +import eu.timepit.refined.auto._ +import eu.timepit.refined.numeric.Positive +import io.circe.Json +import io.renku.generators.CommonGraphGenerators.authUsers +import io.renku.generators.Generators.Implicits._ +import io.renku.graph.acceptancetests.data.Project.Statistics.CommitsCount +import io.renku.graph.acceptancetests.data._ +import io.renku.graph.acceptancetests.flows.{AccessTokenPresence, TSProvisioning} +import io.renku.graph.acceptancetests.testing.AcceptanceTestPatience +import io.renku.graph.acceptancetests.tooling.{AcceptanceSpec, ApplicationServices, ModelImplicits} +import io.renku.graph.model.EventsGenerators.commitIds +import io.renku.graph.model.events.EventStatusProgress +import io.renku.graph.model.testentities.generators.EntitiesGenerators._ +import org.http4s.Status._ +import org.scalatest.concurrent.Eventually + +class ProjectStatusResourceSpec + extends AcceptanceSpec + with ModelImplicits + with ApplicationServices + with TSProvisioning + with AccessTokenPresence + with Eventually + with AcceptanceTestPatience { + + private val numberOfEvents: Int Refined Positive = 5 + + Feature("Project Status API for a given project") { + + val memberUser = authUsers.generateOne + val project = + dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), + CommitsCount(numberOfEvents.value) + ).map(addMemberWithId(memberUser.id)).generateOne + + Scenario("Call be a user who is not a member of the project") { + + gitLabStub.addProject(project) + + When("there's no webhook for a given project in GitLab") + And("project hasn't been activated (no trace of it in the EL)") + + Given("the user is authenticated") + val someAuthUser = authUsers.generateOne + gitLabStub.addAuthenticated(someAuthUser) + + When("the user who is not a member of the project is calling the Status API") + Then("the Status API should return NOT_FOUND") + webhookServiceClient.`GET projects/:id/events/status`(project.id, someAuthUser.accessToken).status shouldBe NotFound + + When("a member of the project activates it") + gitLabStub.addAuthenticated(memberUser) + val commitId = commitIds.generateOne + gitLabStub.replaceCommits(project.id, commitId) + // making the triples generation be happy and not throwing exceptions to the logs + `GET /projects/:id/commits/:id returning OK with some triples`(project, commitId) + + webhookServiceClient.`POST projects/:id/webhooks`(project.id, memberUser.accessToken).status shouldBe Created + + Then("the non-member user should get OK with 'activated' = true") + eventually { + val authUserResponse = webhookServiceClient.`GET projects/:id/events/status`(project.id, someAuthUser.accessToken) + authUserResponse.status shouldBe Ok + authUserResponse.jsonBody.hcursor.downField("activated").as[Boolean].value shouldBe true + } + } + + Scenario("Call be a user who is a member of the project") { + + When("there's no webhook for a given project in GitLab") + gitLabStub.addProject(project) + + Given("the member is authenticated") + gitLabStub.addAuthenticated(memberUser) + + Then("the member of the project calling the Status API should get OK with 'activated' = false") + val response = webhookServiceClient.`GET projects/:id/events/status`(project.id, memberUser.accessToken) + response.status shouldBe Ok + response.jsonBody.hcursor.downField("activated").as[Boolean].value shouldBe false + + When("there's a webhook created for the project") + And("there are events under processing") + val allCommitIds = commitIds.generateNonEmptyList(min = numberOfEvents, max = numberOfEvents) + gitLabStub.setupProject(project, allCommitIds.toList: _*) + mockCommitDataOnTripleGenerator(project, toPayloadJsonLD(project), allCommitIds) + `data in the Triples Store`(project, allCommitIds, memberUser.accessToken) + + Then("the Status API should return OK with some status info") + eventually { + val response = webhookServiceClient.`GET projects/:id/events/status`(project.id, memberUser.accessToken) + + response.status shouldBe Ok + + val responseJson = response.jsonBody.hcursor + responseJson.downField("activated").as[Boolean].value shouldBe true + + val progressObjCursor = responseJson.downField("progress").as[Json].fold(throw _, identity).hcursor + progressObjCursor.downField("done").as[Int].value shouldBe EventStatusProgress.Stage.Final.value + progressObjCursor.downField("total").as[Int].value shouldBe EventStatusProgress.Stage.Final.value + progressObjCursor.downField("percentage").as[Float].value shouldBe 100f + + val detailsObjCursor = responseJson.downField("details").as[Json].fold(throw _, identity).hcursor + detailsObjCursor.downField("status").as[String].value shouldBe "success" + detailsObjCursor.downField("message").as[String].value shouldBe "triples store" + } + } + } +} diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookCreationSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookCreationSpec.scala index ec155c8191..ecc710adcd 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookCreationSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookCreationSpec.scala @@ -18,45 +18,24 @@ package io.renku.graph.acceptancetests -import data.Project.Statistics.CommitsCount -import data._ -import flows.AccessTokenPresence import io.circe.syntax.EncoderOps import io.renku.generators.CommonGraphGenerators.authUsers import io.renku.generators.Generators.Implicits._ +import io.renku.graph.acceptancetests.data.Project.Statistics.CommitsCount +import io.renku.graph.acceptancetests.data._ +import io.renku.graph.acceptancetests.flows.AccessTokenPresence +import io.renku.graph.acceptancetests.tooling.{AcceptanceSpec, ApplicationServices, ModelImplicits} import io.renku.graph.model.EventsGenerators.commitIds import io.renku.graph.model.testentities.cliShapedPersons import io.renku.graph.model.testentities.generators.EntitiesGenerators._ import io.renku.http.client.AccessToken import org.http4s.Status._ -import tooling.{AcceptanceSpec, ApplicationServices, ModelImplicits} class WebhookCreationSpec extends AcceptanceSpec with ModelImplicits with ApplicationServices with AccessTokenPresence { Feature("A Graph Services hook can be created for a project") { - Scenario("Graph Services hook is present on the project in GitLab") { - - val user = authUsers.generateOne - val project = - dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), - CommitsCount.zero - ).map(addMemberWithId(user.id)).generateOne - - Given("api user is authenticated") - gitLabStub.addAuthenticated(user) - - Given("project is present in GitLab with has Graph Services hook") - gitLabStub.setupProject(project) - - When("user does POST webhook-service/projects/:id/webhooks") - val response = webhookServiceClient.POST(s"projects/${project.id}/webhooks", user.accessToken) - - Then("he should get OK response back") - response.status shouldBe Ok - } - - Scenario("No Graph Services webhook on the project in GitLab") { + Scenario("No Graph Services webhook present on the project in GitLab") { val user = authUsers.generateOne val project = @@ -71,15 +50,13 @@ class WebhookCreationSpec extends AcceptanceSpec with ModelImplicits with Applic gitLabStub.addProject(project) Given("some Commit exists for the project in GitLab") - givenAccessTokenPresentFor(project, user.accessToken) val commitId = commitIds.generateOne gitLabStub.replaceCommits(project.id, commitId) - // making the triples generation be happy and not throwing exceptions to the logs `GET /projects/:id/commits/:id returning OK with some triples`(project, commitId) - When("user does POST webhook-service/projects/:id/webhooks") - val response = webhookServiceClient.POST(s"projects/${project.id}/webhooks", user.accessToken) + When("the user does POST webhook-service/projects/:id/webhooks") + val response = webhookServiceClient.`POST projects/:id/webhooks`(project.id, user.accessToken) Then("he should get CREATED response back") response.status shouldBe Created @@ -93,5 +70,26 @@ class WebhookCreationSpec extends AcceptanceSpec with ModelImplicits with Applic .asInstanceOf[AccessToken] .asJson } + + Scenario("Graph Services hook is present on the project in GitLab") { + + val user = authUsers.generateOne + val project = + dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), + CommitsCount.zero + ).map(addMemberWithId(user.id)).generateOne + + Given("api user is authenticated") + gitLabStub.addAuthenticated(user) + + Given("project is present in GitLab with has Graph Services hook") + gitLabStub.setupProject(project) + + When("user does POST webhook-service/projects/:id/webhooks") + val response = webhookServiceClient.`POST projects/:id/webhooks`(project.id, user.accessToken) + + Then("he should get OK response back") + response.status shouldBe Ok + } } } diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookValidationEndpointSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookValidationEndpointSpec.scala index ae627a73f3..d7c611e5c9 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookValidationEndpointSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/WebhookValidationEndpointSpec.scala @@ -18,7 +18,7 @@ package io.renku.graph.acceptancetests -import io.circe.syntax.EncoderOps +import io.circe.syntax._ import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.graph.acceptancetests.data.Project.Statistics.CommitsCount @@ -34,11 +34,11 @@ class WebhookValidationEndpointSpec extends AcceptanceSpec with ApplicationServi Scenario("There's a Graph Services hook on a Public project in GitLab") { + val user = authUsers.generateOne val project = dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), CommitsCount.zero - ).generateOne - val user = authUsers.generateOne + ).map(addMemberWithId(user.id)).generateOne Given("api user is authenticated") gitLabStub.addAuthenticated(user) @@ -55,11 +55,11 @@ class WebhookValidationEndpointSpec extends AcceptanceSpec with ApplicationServi Scenario("There's no Graph Services hook on a Public project in GitLab") { + val user = authUsers.generateOne val project = dataProjects(renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons).modify(removeMembers()), CommitsCount.zero - ).generateOne - val user = authUsers.generateOne + ).map(addMemberWithId(user.id)).generateOne Given("api user is authenticated") gitLabStub.addAuthenticated(user) diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/flows/TSProvisioning.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/flows/TSProvisioning.scala index 34dfcc8f64..0306202084 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/flows/TSProvisioning.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/flows/TSProvisioning.scala @@ -79,21 +79,16 @@ trait TSProvisioning def `wait for events to be processed`(projectId: projects.GitLabId, accessToken: AccessToken): Assertion = eventually { - val response = fetchProcessingStatus(projectId, accessToken) + val response = webhookServiceClient.`GET projects/:id/events/status`(projectId, accessToken) response.status shouldBe Ok response.jsonBody.hcursor.downField("activated").as[Boolean].value shouldBe true response.jsonBody.hcursor.downField("progress").downField("percentage").as[Double].value shouldBe 100d } - def `check no hook exists`(projectId: projects.GitLabId, accessToken: AccessToken): Assertion = eventually { - val response = fetchProcessingStatus(projectId, accessToken) - response.status shouldBe Ok - response.jsonBody.hcursor.downField("activated").as[Boolean].value shouldBe false + def `check hook cannot be found`(projectId: projects.GitLabId, accessToken: AccessToken): Assertion = eventually { + webhookServiceClient.`GET projects/:id/events/status`(projectId, accessToken).status shouldBe NotFound } - private def fetchProcessingStatus(projectId: projects.GitLabId, accessToken: AccessToken) = - webhookServiceClient.fetchProcessingStatus(projectId, accessToken) - def `wait for the Fast Tract event`(projectId: projects.GitLabId)(implicit ioRuntime: IORuntime): Unit = eventually { val sleepTime = 1 second diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/CrossEntitiesSearchSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/CrossEntitiesSearchSpec.scala index 7c5f9db7b9..ed4e9b6df2 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/CrossEntitiesSearchSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/CrossEntitiesSearchSpec.scala @@ -65,7 +65,7 @@ class CrossEntitiesSearchSpec extends AcceptanceSpec with ApplicationServices wi .modify(replaceDSName(sentenceContaining(commonPhrase).generateAs[datasets.Name])) ) .generateOne - val project = dataProjects(testProject).generateOne + val project = dataProjects(testProject).map(addMemberWithId(user.id)).generateOne Scenario("As a user I would like to be able to do cross-entity search by calling a REST endpoint") { diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/DatasetsResourcesSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/DatasetsResourcesSpec.scala index 362fcc7caa..bab0de0c83 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/DatasetsResourcesSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/DatasetsResourcesSpec.scala @@ -66,8 +66,12 @@ class DatasetsResourcesSpec creatorGen = cliShapedPersons ) .generateOne + val creatorPerson = cliShapedPersons.generateOne val project = - dataProjects(testProject).map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)).generateOne + dataProjects(testProject) + .map(replaceCreatorFrom(creatorPerson, creator.id)) + .map(addMemberFrom(creatorPerson, creator.id) >>> addMemberWithId(user.id)) + .generateOne Scenario("As a user I would like to find project's datasets by calling a REST endpoint") { @@ -180,8 +184,12 @@ class DatasetsResourcesSpec .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons))) .generateOne + val creatorPerson = cliShapedPersons.generateOne val privateProject = - dataProjects(testPrivateProject).map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)).generateOne + dataProjects(testPrivateProject) + .map(replaceCreatorFrom(creatorPerson, creator.id)) + .map(addMemberFrom(creatorPerson, creator.id)) + .generateOne Given("there's a private project in KG") val commitId = commitIds.generateOne @@ -212,13 +220,13 @@ class DatasetsResourcesSpec .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeTitleContaining(text))) .generateOne - val project1 = dataProjects(testProject1).generateOne + val project1 = dataProjects(testProject1).map(addMemberWithId(creator.id)).generateOne val (dataset2, testProject2) = renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeDescContaining(text))) .generateOne - val project2 = dataProjects(testProject2).generateOne + val project2 = dataProjects(testProject2).map(addMemberWithId(creator.id)).generateOne val (dataset3, testProject3) = renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) .modify(removeMembers()) .addDataset( @@ -226,27 +234,28 @@ class DatasetsResourcesSpec .modify(_.makeCreatorNameContaining(text)(creatorUsernameUpdaterInternal(cliShapedPersons))) ) .generateOne - val project3 = dataProjects(testProject3).generateOne + val project3 = dataProjects(testProject3).map(addMemberWithId(creator.id)).generateOne val (dataset4, testProject4 ::~ testProject4Fork) = renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeKeywordsContaining(text))) .forkOnce(creatorGen = cliShapedPersons) .generateOne .bimap(identity, _.bimap(removeMembers(), removeMembers())) - val project4 = dataProjects(testProject4).generateOne - val project4Fork = dataProjects(testProject4Fork).generateOne + val project4 = dataProjects(testProject4).map(addMemberWithId(creator.id)).generateOne + val project4Fork = dataProjects(testProject4Fork).map(addMemberWithId(creator.id)).generateOne val (dataset5WithoutText, testProject5) = renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons))) .generateOne - val project5 = dataProjects(testProject5).generateOne + val project5 = dataProjects(testProject5).map(addMemberWithId(creator.id)).generateOne val (_, testProject6Private) = renkuProjectEntities(visibilityPrivate, creatorGen = cliShapedPersons) .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeTitleContaining(text))) .generateOne + val project6CreatorPerson = cliShapedPersons.generateOne val project6Private = dataProjects(testProject6Private) - .map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)) - .map(addMemberWithId(user.id)) + .map(replaceCreatorFrom(project6CreatorPerson, creator.id)) + .map(addMemberFrom(project6CreatorPerson, creator.id)) .generateOne Given("some datasets with title, description, name and author containing some arbitrary chosen text") @@ -376,16 +385,20 @@ class DatasetsResourcesSpec .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeTitleContaining(text))) .generateOne + val project1CreatorPerson = cliShapedPersons.generateOne val project1 = dataProjects(testProject1) - .map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)) + .map(replaceCreatorFrom(project1CreatorPerson, creator.id)) + .map(addMemberWithId(user.id) >>> addMemberFrom(project1CreatorPerson, creator.id)) .generateOne val (_, testProject2Private) = renkuProjectEntities(visibilityPrivate, creatorGen = cliShapedPersons) .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeTitleContaining(text))) .generateOne + val project2CreatorPerson = cliShapedPersons.generateOne val project2Private = dataProjects(testProject2Private) - .map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)) + .map(replaceCreatorFrom(project2CreatorPerson, creator.id)) + .map(addMemberFrom(project2CreatorPerson, creator.id)) .generateOne val (dataset3PrivateWithAccess, testProject3PrivateWithAccess) = @@ -393,9 +406,10 @@ class DatasetsResourcesSpec .modify(removeMembers()) .addDataset(datasetEntities(provenanceInternal(cliShapedPersons)).modify(_.makeTitleContaining(text))) .generateOne + val project3CreatorPerson = cliShapedPersons.generateOne val project3PrivateWithAccess = dataProjects(testProject3PrivateWithAccess) - .map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)) - .map(addMemberWithId(user.id)) + .map(replaceCreatorFrom(project3CreatorPerson, creator.id)) + .map(addMemberWithId(user.id) >>> addMemberFrom(project3CreatorPerson, creator.id)) .generateOne Given("some datasets with title, description, name and author containing some arbitrary chosen text") @@ -438,7 +452,7 @@ class DatasetsResourcesSpec .addDataset(datasetEntities(provenanceInternal(cliShapedPersons))) .generateOne - val project = dataProjects(testProject).generateOne + val project = dataProjects(testProject).map(addMemberWithId(creator.id)).generateOne Given("some data in the Triples Store") gitLabStub.addAuthenticated(creator) @@ -464,9 +478,10 @@ class DatasetsResourcesSpec .addDataset(datasetEntities(provenanceInternal(cliShapedPersons))) .generateOne + val projectCreatorPerson = cliShapedPersons.generateOne val project = dataProjects(testProject) - .map(replaceCreatorFrom(cliShapedPersons.generateOne, creator.id)) - .map(addMemberWithId(user.id)) + .map(replaceCreatorFrom(projectCreatorPerson, creator.id)) + .map(addMemberWithId(user.id) >>> addMemberFrom(projectCreatorPerson, creator.id)) .generateOne Given("I am authenticated") diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/LineageResourcesSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/LineageResourcesSpec.scala index e20137fb06..100622489b 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/LineageResourcesSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/LineageResourcesSpec.scala @@ -20,12 +20,13 @@ package io.renku.graph.acceptancetests package knowledgegraph import cats.syntax.all._ -import data._ -import flows.TSProvisioning import io.circe.literal._ import io.circe.{ACursor, Json} import io.renku.generators.CommonGraphGenerators.authUsers import io.renku.generators.Generators.Implicits._ +import io.renku.graph.acceptancetests.data._ +import io.renku.graph.acceptancetests.flows.TSProvisioning +import io.renku.graph.acceptancetests.tooling.{AcceptanceSpec, ApplicationServices} import io.renku.graph.model import io.renku.graph.model.EventsGenerators.commitIds import io.renku.graph.model.Schemas.prov @@ -36,11 +37,14 @@ import io.renku.graph.model.testentities.{LineageExemplarData, NodeDef, cliShape import io.renku.http.client.AccessToken import io.renku.http.client.UrlEncoder.urlEncode import org.http4s.Status.{NotFound, Ok} -import tooling.{AcceptanceSpec, ApplicationServices} class LineageResourcesSpec extends AcceptanceSpec with ApplicationServices with TSProvisioning with TSData { Feature("GET knowledge-graph/projects///files//lineage to find a file's lineage") { + + val user = authUsers.generateOne + val token = user.accessToken + val (exemplarData, project) = { val lineageData = LineageExemplarData( renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) @@ -54,7 +58,7 @@ class LineageResourcesSpec extends AcceptanceSpec with ApplicationServices with .generateOne, personGen = cliShapedPersons ) - (lineageData, dataProjects(lineageData.project).generateOne) + (lineageData, dataProjects(lineageData.project).map(addMemberWithId(user.id)).generateOne) } /** Expected data structure when looking for the grid_plot file @@ -70,8 +74,6 @@ class LineageResourcesSpec extends AcceptanceSpec with ApplicationServices with * grid_plot */ Scenario("As a user I would like to find a public project's lineage") { - val user = authUsers.generateOne - val token = user.accessToken Given("some data in the Triples Store") val commitId = commitIds.generateOne @@ -141,7 +143,10 @@ class LineageResourcesSpec extends AcceptanceSpec with ApplicationServices with ) val commitId = commitIds.generateOne val project = - dataProjects(privateExemplarData.project).map(replaceCreatorFrom(creatorPerson, creator.id)).generateOne + dataProjects(privateExemplarData.project) + .map(replaceCreatorFrom(creatorPerson, creator.id)) + .map(addMemberFrom(creatorPerson, creator.id)) + .generateOne Given("I am authenticated") gitLabStub.addAuthenticated(creator) diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectDatasetTagsResourceSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectDatasetTagsResourceSpec.scala index 98304a5e6b..6b31efc658 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectDatasetTagsResourceSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectDatasetTagsResourceSpec.scala @@ -20,20 +20,25 @@ package io.renku.graph.acceptancetests package knowledgegraph import cats.syntax.all._ -import data._ -import flows.TSProvisioning import io.circe.Json import io.renku.generators.CommonGraphGenerators.authUsers import io.renku.generators.Generators.Implicits._ +import io.renku.graph.acceptancetests.data._ +import io.renku.graph.acceptancetests.flows.TSProvisioning +import io.renku.graph.acceptancetests.tooling.{AcceptanceSpec, ApplicationServices} import io.renku.graph.model.EventsGenerators.commitIds import io.renku.graph.model.RenkuTinyTypeGenerators.{personEmails, personGitLabIds} import io.renku.graph.model.publicationEvents import io.renku.graph.model.testentities._ import io.renku.tinytypes.json.TinyTypeDecoders._ import org.http4s.Status.Ok -import tooling.{AcceptanceSpec, ApplicationServices} +import org.scalatest.EitherValues -class ProjectDatasetTagsResourceSpec extends AcceptanceSpec with ApplicationServices with TSProvisioning { +class ProjectDatasetTagsResourceSpec + extends AcceptanceSpec + with ApplicationServices + with TSProvisioning + with EitherValues { Feature("GET knowledge-graph/projects///datasets/:dsName/tags to find project dataset's tags") { @@ -58,7 +63,7 @@ class ProjectDatasetTagsResourceSpec extends AcceptanceSpec with ApplicationServ val project = dataProjects(testProject) .map(replaceCreatorFrom(creator, creatorGitLabId)) - .map(addMemberFrom(creator, creatorGitLabId)) + .map(addMemberFrom(creator, creatorGitLabId) >>> addMemberWithId(user.id)) .generateOne ds -> project @@ -78,11 +83,11 @@ class ProjectDatasetTagsResourceSpec extends AcceptanceSpec with ApplicationServ Then("he should get OK response with the relevant tags") response.status shouldBe Ok - val Right(tags) = response.jsonBody.as[List[Json]] + val tags = response.jsonBody.as[List[Json]].value tags .map(_.hcursor.downField("name").as[publicationEvents.Name]) .sequence - .fold(fail(_), identity) shouldBe dataset.publicationEvents.sortBy(_.startDate).reverse.map(_.name) + .value shouldBe dataset.publicationEvents.sortBy(_.startDate).reverse.map(_.name) } } } diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectsResourcesSpec.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectsResourcesSpec.scala index b241a82296..0157b62971 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectsResourcesSpec.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/knowledgegraph/ProjectsResourcesSpec.scala @@ -30,7 +30,7 @@ import io.renku.graph.model.RenkuTinyTypeGenerators.{personEmails, personGitLabI import io.renku.graph.model.projects.Visibility import io.renku.graph.model.testentities._ import io.renku.http.rest.Links -import io.renku.http.server.EndpointTester.{jsonEntityDecoder, JsonOps} +import io.renku.http.server.EndpointTester.{JsonOps, jsonEntityDecoder} import org.http4s.Status._ import org.scalatest.EitherValues @@ -57,7 +57,7 @@ class ProjectsResourcesSpec val parentDataProject = dataProjects(parent) .map(replaceCreatorFrom(creator, creatorGitLabId)) - .map(addMemberFrom(creator, creatorGitLabId)) + .map(addMemberFrom(creator, creatorGitLabId) >>> addMemberWithId(user.id)) .generateOne val childDataProject = diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabApiStub.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabApiStub.scala index f6a7e32587..848cb72712 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabApiStub.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabApiStub.scala @@ -18,11 +18,6 @@ package io.renku.graph.acceptancetests.stubs.gitlab -import GitLabApiStub.State -import GitLabStateGenerators._ -import GitLabStateQueries._ -import GitLabStateUpdates._ -import JsonEncoders._ import cats.data.{EitherT, NonEmptyList, OptionT} import cats.effect._ import cats.syntax.all._ @@ -33,11 +28,16 @@ import io.circe.syntax._ import io.renku.generators.Generators.Implicits._ import io.renku.graph.acceptancetests.data.Project import io.renku.graph.acceptancetests.data.Project.Permissions.AccessLevel +import io.renku.graph.acceptancetests.stubs.gitlab.GitLabApiStub.State import io.renku.graph.acceptancetests.stubs.gitlab.GitLabAuth.AuthedReq.{AuthedProject, AuthedUser} +import io.renku.graph.acceptancetests.stubs.gitlab.GitLabStateGenerators._ +import io.renku.graph.acceptancetests.stubs.gitlab.GitLabStateQueries._ +import io.renku.graph.acceptancetests.stubs.gitlab.GitLabStateUpdates._ +import io.renku.graph.acceptancetests.stubs.gitlab.JsonEncoders._ import io.renku.graph.model -import io.renku.graph.model.{persons, projects} import io.renku.graph.model.events.CommitId import io.renku.graph.model.testentities.Person +import io.renku.graph.model.{persons, projects} import io.renku.http.client.AccessToken.ProjectAccessToken import io.renku.http.client.UserAccessToken import org.http4s._ @@ -125,6 +125,11 @@ final class GitLabApiStub[F[_]: Async: Logger](private val stateRef: Ref[F, Stat .map(_.toList.flatMap(_.members.toList)) .flatMap(Ok(_)) + case GET -> Root / ProjectId(projectId) / "members" / "all" / UserGitLabId(userId) => + query(findProjectById(projectId, maybeAuthedReq)) + .map(_.toList.flatMap(_.members.toList).find(_.gitLabId == userId)) + .flatMap(OkOrNotFound(_)) + case GET -> Root / ProjectId(id) / "repository" / "commits" => query(commitsFor(id, maybeAuthedReq)).flatMap(Ok(_)) diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabStubIOSyntax.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabStubIOSyntax.scala index 190097ffcc..3b8f500ce4 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabStubIOSyntax.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/GitLabStubIOSyntax.scala @@ -55,6 +55,11 @@ trait GitLabStubIOSyntax { self: IOSpec => def addProject(project: Project): Unit = self.update(GitLabStateUpdates.addProject(project)).unsafeRunSync() + /** Adds a webhook to a gitlab project. The assumption is the project is already added to GitLab + */ + def addWebhook(projectId: projects.GitLabId): Unit = + self.update(GitLabStateUpdates.addWebhook(projectId, webhookUri)).unsafeRunSync() + /** Adds the given project and associates the given commits. It also installs the `webhook/events` webhook to * integrate with the triples generator. */ diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/JsonEncoders.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/JsonEncoders.scala index 038975291f..670f26c4b2 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/JsonEncoders.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/stubs/gitlab/JsonEncoders.scala @@ -42,24 +42,28 @@ trait JsonEncoders { } implicit val projectMemberEncoder: Encoder[ProjectMember] = Encoder.instance { pm => - Map("id" -> pm.gitLabId.asJson, "username" -> pm.username.asJson, "name" -> pm.name.asJson).asJson + Map("id" -> pm.gitLabId.asJson, + "username" -> pm.username.asJson, + "name" -> pm.name.asJson, + "access_level" -> 40.asJson, + "state" -> "active".asJson + ).asJson } - implicit val pushEventEncoder: Encoder[PushEvent] = - Encoder.instance { ev => - Map( - "project_id" -> ev.projectId.asJson, - "push_data" -> Json.obj( - "commit_from" -> Json.Null, - "commit_to" -> ev.commitId.asJson - ), - "author" -> - Json.obj( - "id" -> ev.authorId.asJson, - "name" -> ev.authorName.asJson - ) - ).asJson - } + implicit val pushEventEncoder: Encoder[PushEvent] = Encoder.instance { ev => + Map( + "project_id" -> ev.projectId.asJson, + "push_data" -> Json.obj( + "commit_from" -> Json.Null, + "commit_to" -> ev.commitId.asJson + ), + "author" -> + Json.obj( + "id" -> ev.authorId.asJson, + "name" -> ev.authorName.asJson + ) + ).asJson + } implicit val commitDataEncoder: Encoder[CommitData] = Encoder.instance { c => diff --git a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/tooling/ServicesClients.scala b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/tooling/ServicesClients.scala index 7ad8dc9c83..7149224186 100644 --- a/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/tooling/ServicesClients.scala +++ b/acceptance-tests/src/test/scala/io/renku/graph/acceptancetests/tooling/ServicesClients.scala @@ -65,8 +65,13 @@ object WebhookServiceClient { } yield response }.unsafeRunSync() - def fetchProcessingStatus(projectId: projects.GitLabId, accessToken: AccessToken)(implicit + def `POST projects/:id/webhooks`(projectId: projects.GitLabId, accessToken: AccessToken)(implicit ior: IORuntime + ): ClientResponse = + POST((uri"projects" / projectId / "webhooks").renderString, accessToken) + + def `GET projects/:id/events/status`(projectId: projects.GitLabId, accessToken: AccessToken)(implicit + ior: IORuntime ): ClientResponse = GET((uri"projects" / projectId / "events" / "status").renderString, accessToken) } diff --git a/event-log/README.md b/event-log/README.md index decc28e237..06ae555099 100644 --- a/event-log/README.md +++ b/event-log/README.md @@ -189,12 +189,12 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "RollbackToNew", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" - }, - "subCategory": "RollbackToNew" + } } ``` @@ -207,14 +207,14 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "ToFailure", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" }, "message": "", - "newStatus": , - "subCategory": "ToFailure" + "newStatus": } ``` @@ -227,12 +227,12 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "RollbackToTriplesGenerated", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" - }, - "newStatus": "RollbackToTriplesGenerated" + } } ``` @@ -245,10 +245,10 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "RedoProjectTransformation", "project": { "path": "namespace/project-name" - }, - "subCategory": "RedoProjectTransformation" + } } ``` @@ -261,12 +261,12 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "ToAwaitingDeletion", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" - }, - "subCategory": "ToAwaitingDeletion" + } } ``` @@ -279,12 +279,12 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "ToTriplesGenerated", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" }, - "subCategory": "ToTriplesGenerated", "processingTime": "PT2.023S" } ``` @@ -300,12 +300,12 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "ToTriplesStore", "id": "df654c3b1bd105a29d658f78f6380a842feac879", "project": { "id": 12, "path": "namespace/project-name" }, - "subCategory": "ToTriplesStore", "processingTime": "PT2.023S" } ``` @@ -319,11 +319,11 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "ProjectEventsToNew", "project": { "id": 12, "path": "namespace/project-name" - }, - "subCategory": "ProjectEventsToNew" + } } ``` @@ -336,11 +336,11 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", + "subCategory": "RollbackToAwaitingDeletion", "project": { "id": 12, "path": "namespace/project-name" - }, - "subCategory": "RollbackToAwaitingDeletion" + } } ``` @@ -353,7 +353,7 @@ payload needed for processing. ```json { "categoryName": "EVENTS_STATUS_CHANGE", - "subCategory": "AllEventsToNew" + "subCategory": "AllEventsToNew" } ``` diff --git a/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisation.scala b/event-log/src/main/scala/io/renku/eventlog/events/producers/ProjectPrioritisation.scala similarity index 86% rename from event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisation.scala rename to event-log/src/main/scala/io/renku/eventlog/events/producers/ProjectPrioritisation.scala index a0b334a750..16c0cee8ee 100644 --- a/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisation.scala +++ b/event-log/src/main/scala/io/renku/eventlog/events/producers/ProjectPrioritisation.scala @@ -17,15 +17,14 @@ */ package io.renku.eventlog.events.producers -package awaitinggeneration -import ProjectPrioritisation._ import cats.MonadThrow import cats.data.NonEmptyList import cats.syntax.all._ import eu.timepit.refined.api.Refined import eu.timepit.refined.numeric.NonNegative -import DefaultSubscribers.DefaultSubscribers +import io.renku.eventlog.events.producers.DefaultSubscribers.DefaultSubscribers +import io.renku.eventlog.events.producers.ProjectPrioritisation._ import io.renku.graph.model.events.EventDate import io.renku.graph.model.projects import io.renku.tinytypes.{BigDecimalTinyType, TinyTypeFactory} @@ -42,30 +41,30 @@ private class ProjectPrioritisationImpl[F[_]: DefaultSubscribers] extends Projec private val FreeSpotsRatio = .15 override def prioritise(projects: List[ProjectInfo], totalOccupancy: Long): List[(ProjectIds, Priority)] = - ((rejectProjectsAboveOccupancyThreshold _).tupled >>> + (rejectProjectsAboveOccupancyThreshold >>> findPrioritiesBasedOnLatestEventDate >>> correctPrioritiesUsingOccupancyPerProject)(projects, totalOccupancy) - private def rejectProjectsAboveOccupancyThreshold(projects: List[ProjectInfo], - totalOccupancy: Long - ): List[ProjectInfo] = - DefaultSubscribers[F].getTotalCapacity - .map(_.value) - .map { totalCapacity => - val freeSpots = - if ((totalCapacity * FreeSpotsRatio).ceil.intValue() < 2) 2 - else (totalCapacity * FreeSpotsRatio).ceil.intValue() - projects match { - case Nil => Nil - case projects => - projects.foldLeft(List.empty[ProjectInfo]) { (acc, project) => - if (project.currentOccupancy.value == 0) acc :+ project - else if (totalOccupancy < totalCapacity - freeSpots) acc :+ project - else acc - } + private lazy val rejectProjectsAboveOccupancyThreshold: ((List[ProjectInfo], Long)) => List[ProjectInfo] = { + case (projects, totalOccupancy) => + DefaultSubscribers[F].getTotalCapacity + .map { totalCapacity => + val freeSpots = + if ((totalCapacity * FreeSpotsRatio).ceil.intValue() < 2) 2 + else (totalCapacity * FreeSpotsRatio).ceil.intValue() + + projects match { + case Nil => Nil + case projects => + projects.foldLeft(List.empty[ProjectInfo]) { (acc, project) => + if (project.currentOccupancy.value == 0) acc :+ project + else if (totalOccupancy < totalCapacity.value - freeSpots) acc :+ project + else acc + } + } } - } - .getOrElse(projects) + .getOrElse(projects) + } private lazy val findPrioritiesBasedOnLatestEventDate : List[ProjectInfo] => List[(ProjectIds, Priority, Int Refined NonNegative)] = { diff --git a/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinder.scala b/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinder.scala index c75a06747b..c9927a0beb 100644 --- a/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinder.scala +++ b/event-log/src/main/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinder.scala @@ -30,8 +30,8 @@ import io.renku.db.implicits._ import io.renku.eventlog.EventLogDB.SessionResource import io.renku.eventlog.TypeSerializers import io.renku.eventlog.events.producers -import io.renku.eventlog.events.producers.EventFinder -import io.renku.eventlog.events.producers.awaitinggeneration.ProjectPrioritisation.{Priority, ProjectInfo} +import io.renku.eventlog.events.producers.{EventFinder, ProjectPrioritisation} +import io.renku.eventlog.events.producers.ProjectPrioritisation.{Priority, ProjectInfo} import io.renku.eventlog.events.producers.DefaultSubscribers.DefaultSubscribers import io.renku.eventlog.metrics.{EventStatusGauges, QueriesExecutionTimes} import io.renku.graph.model.events.{CompoundEventId, EventDate, EventId, EventStatus, ExecutionDate} diff --git a/event-log/src/main/scala/io/renku/eventlog/events/producers/model.scala b/event-log/src/main/scala/io/renku/eventlog/events/producers/model.scala index fbff39f2bb..26972063cf 100644 --- a/event-log/src/main/scala/io/renku/eventlog/events/producers/model.scala +++ b/event-log/src/main/scala/io/renku/eventlog/events/producers/model.scala @@ -34,6 +34,8 @@ final class TotalCapacity private (val value: Int) extends AnyVal with IntTinyTy if (c > 0) FreeCapacity(c) else FreeCapacity(0) } + + def *(v: Double): Double = value * v } object TotalCapacity extends TinyTypeFactory[TotalCapacity](new TotalCapacity(_)) with NonNegativeInt[TotalCapacity] { implicit val decoder: Decoder[TotalCapacity] = intDecoder(TotalCapacity) diff --git a/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinder.scala b/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinder.scala index 05a23e0c5b..66c0a1f0dc 100644 --- a/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinder.scala +++ b/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinder.scala @@ -19,8 +19,7 @@ package io.renku.eventlog.events.producers package triplesgenerated -import ProjectPrioritisation.{Priority, ProjectInfo} -import cats.MonadThrow +import cats.Id import cats.data._ import cats.effect.Async import cats.syntax.all._ @@ -31,6 +30,8 @@ import io.renku.db.implicits._ import io.renku.db.{DbClient, SqlStatement} import io.renku.eventlog.EventLogDB.SessionResource import io.renku.eventlog.events.producers +import io.renku.eventlog.events.producers.DefaultSubscribers.DefaultSubscribers +import io.renku.eventlog.events.producers.ProjectPrioritisation.{Priority, ProjectInfo} import io.renku.eventlog.metrics.{EventStatusGauges, QueriesExecutionTimes} import io.renku.graph.model.events.EventStatus._ import io.renku.graph.model.events.{CompoundEventId, EventDate, EventId, EventStatus, ExecutionDate} @@ -47,7 +48,7 @@ import scala.util.Random private class EventFinderImpl[F[_]: Async: SessionResource: QueriesExecutionTimes: EventStatusGauges]( now: () => Instant = () => Instant.now, projectsFetchingLimit: Int Refined Positive, - projectPrioritisation: ProjectPrioritisation, + projectPrioritisation: ProjectPrioritisation[F], pickRandomlyFrom: List[ProjectIds] => Option[ProjectIds] = ids => ids.get((Random nextInt ids.size).toLong) ) extends DbClient(Some(QueriesExecutionTimes[F])) with producers.EventFinder[F, TriplesGeneratedEvent] @@ -57,18 +58,24 @@ private class EventFinderImpl[F[_]: Async: SessionResource: QueriesExecutionTime override def popEvent(): F[Option[TriplesGeneratedEvent]] = SessionResource[F].useK { for { - (maybeProject, maybeTriplesGeneratedEvent) <- findEventAndUpdateForProcessing() + (maybeProject, maybeTriplesGeneratedEvent) <- findEventAndUpdateForProcessing _ <- Kleisli.liftF(maybeUpdateMetrics(maybeProject, maybeTriplesGeneratedEvent)) } yield maybeTriplesGeneratedEvent } - private def findEventAndUpdateForProcessing() = for { - maybeProject <- findProjectsWithEventsInQueue.map(prioritise).map(selectProject) - maybeIdAndProjectAndBody <- - maybeProject.map(findNewestEvent).getOrElse(Kleisli.pure(Option.empty[TriplesGeneratedEvent])) + private def findEventAndUpdateForProcessing = for { + maybeProject <- selectCandidateProject + maybeIdAndProjectAndBody <- maybeProject + .map(findNewestEvent) + .getOrElse(Kleisli.pure(Option.empty[TriplesGeneratedEvent])) maybeBody <- markAsTransformingTriples(maybeIdAndProjectAndBody) } yield maybeProject -> maybeBody + private def selectCandidateProject = for { + projects <- findProjectsWithEventsInQueue + totalOccupancy <- findTotalOccupancy + } yield ((prioritise _).tupled >>> selectProject)(projects, totalOccupancy) + private def findProjectsWithEventsInQueue = measureExecutionTime { SqlStatement .named(s"${SubscriptionCategory.categoryName.value.toLowerCase} - find projects") @@ -115,6 +122,16 @@ private class EventFinderImpl[F[_]: Async: SessionResource: QueriesExecutionTime .build(_.toList) } + private def findTotalOccupancy = measureExecutionTime { + SqlStatement + .named(s"${SubscriptionCategory.categoryName.value.toLowerCase} - find total occupancy") + .select[Void, Long]( + sql"""SELECT COUNT(event_id) FROM event WHERE status = '#${TransformingTriples.value}'""".query(int8) + ) + .arguments(Void) + .build[Id](_.unique) + } + private def findNewestEvent(idAndPath: ProjectIds) = measureExecutionTime { val executionDate = ExecutionDate(now()) SqlStatement @@ -163,10 +180,10 @@ private class EventFinderImpl[F[_]: Async: SessionResource: QueriesExecutionTime case None => Kleisli.pure(Option.empty[TriplesGeneratedEvent]) case Some(event @ TriplesGeneratedEvent(id, _, _)) => - measureExecutionTime(updateStatus(id)) map toNoneIfEventAlreadyTaken(event) + updateStatus(id) map toNoneIfEventAlreadyTaken(event) } - private def updateStatus(commitEventId: CompoundEventId) = + private def updateStatus(commitEventId: CompoundEventId) = measureExecutionTime { SqlStatement .named(s"${SubscriptionCategory.categoryName.value.toLowerCase} - update status") .command[EventStatus ~ ExecutionDate ~ EventId ~ projects.GitLabId ~ EventStatus]( @@ -185,6 +202,7 @@ private class EventFinderImpl[F[_]: Async: SessionResource: QueriesExecutionTime TransformingTriples ) .build + } private def toNoneIfEventAlreadyTaken(event: TriplesGeneratedEvent): Completion => Option[TriplesGeneratedEvent] = { case Completion.Update(1) => Some(event) @@ -204,11 +222,8 @@ private object EventFinder { private val ProjectsFetchingLimit: Int Refined Positive = 10 - def apply[F[_]: Async: SessionResource: QueriesExecutionTimes: EventStatusGauges] - : F[producers.EventFinder[F, TriplesGeneratedEvent]] = MonadThrow[F].catchNonFatal { - new EventFinderImpl[F]( - projectsFetchingLimit = ProjectsFetchingLimit, - projectPrioritisation = new ProjectPrioritisation() - ) - } + def apply[F[_]: Async: DefaultSubscribers: SessionResource: QueriesExecutionTimes: EventStatusGauges] + : F[producers.EventFinder[F, TriplesGeneratedEvent]] = + ProjectPrioritisation[F] + .map(pp => new EventFinderImpl(projectsFetchingLimit = ProjectsFetchingLimit, projectPrioritisation = pp)) } diff --git a/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisation.scala b/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisation.scala deleted file mode 100644 index 9e4a67616b..0000000000 --- a/event-log/src/main/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisation.scala +++ /dev/null @@ -1,93 +0,0 @@ -/* - * 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.eventlog.events.producers -package triplesgenerated - -import ProjectPrioritisation.{Priority, ProjectInfo} -import eu.timepit.refined.api.Refined -import eu.timepit.refined.numeric.NonNegative -import io.renku.eventlog.events.producers.ProjectIds -import io.renku.graph.model.events.EventDate -import io.renku.graph.model.projects -import io.renku.tinytypes.{BigDecimalTinyType, TinyTypeFactory} - -import java.time.Duration - -private class ProjectPrioritisation { - import ProjectPrioritisation.Priority._ - - def prioritise(projects: List[ProjectInfo]): List[(ProjectIds, Priority)] = - findPrioritiesBasedOnMostRecentActivity(projects) - .map { case (projectIds, priority, _) => (projectIds, priority) } - - private lazy val findPrioritiesBasedOnMostRecentActivity - : List[ProjectInfo] => List[(ProjectIds, Priority, Int Refined NonNegative)] = { - case Nil => Nil - case ProjectInfo(projectId, projectPath, _, currentOccupancy) :: Nil => - List((ProjectIds(projectId, projectPath), MaxPriority, currentOccupancy)) - case projects => - val ProjectInfo(_, _, latestEventDate, _) = projects.head - val ProjectInfo(_, _, oldestEventDate, _) = projects.last - val maxDistance = BigDecimal(oldestEventDate.distance(from = latestEventDate)) - - def findPriority(eventDate: EventDate): Priority = maxDistance match { - case maxDistance if maxDistance == 0 => MaxPriority - case maxDistance => - Priority.safeApply( - BigDecimal(oldestEventDate.distance(from = eventDate).toDouble) / maxDistance - ) - } - projects.map { case ProjectInfo(projectId, projectPath, eventDate, currentOccupancy) => - (ProjectIds(projectId, projectPath), findPriority(eventDate), currentOccupancy) - } - } - - private implicit class EventDateOps(eventDate: EventDate) { - def distance(from: EventDate): Long = Duration.between(from.value, eventDate.value).toHours - } -} - -private object ProjectPrioritisation { - - final case class ProjectInfo(id: projects.GitLabId, - path: projects.Path, - latestEventDate: EventDate, - currentOccupancy: Int Refined NonNegative - ) - final class Priority private (val value: BigDecimal) extends AnyVal with BigDecimalTinyType - object Priority extends TinyTypeFactory[Priority](new Priority(_)) { - - addConstraint( - value => (value >= .1) && (value <= BigDecimal(1)), - value => s"$value is not valid $typeName as it has to be >= 0.1 && <= 1" - ) - - val MaxPriority: Priority = Priority(1.0) - val MinPriority: Priority = Priority(0.1) - - private def apply(value: Double): Priority = Priority(BigDecimal(value)) - - def safeApply(value: BigDecimal): Priority = value match { - case value if value <= .1 => MinPriority - case value if value >= 1.0 => MaxPriority - case value => Priority(value) - } - def safeApply(value: Double): Priority = safeApply(BigDecimal(value)) - } -} diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/Generators.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/Generators.scala index b1e8e20014..b1ebf1ef42 100644 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/Generators.scala +++ b/event-log/src/test/scala/io/renku/eventlog/events/producers/Generators.scala @@ -31,9 +31,9 @@ import org.scalacheck.Gen object Generators { - val totalCapacities: Gen[TotalCapacity] = positiveInts() map (v => TotalCapacity(v.value)) - val freeCapacities: Gen[FreeCapacity] = positiveInts() map (v => FreeCapacity(v.value)) - val usedCapacities: Gen[UsedCapacity] = positiveInts() map (v => UsedCapacity(v.value)) + implicit val totalCapacities: Gen[TotalCapacity] = positiveInts() map (v => TotalCapacity(v.value)) + val freeCapacities: Gen[FreeCapacity] = positiveInts() map (v => FreeCapacity(v.value)) + val usedCapacities: Gen[UsedCapacity] = positiveInts() map (v => UsedCapacity(v.value)) private[producers] final case class TestSubscriber(url: SubscriberUrl, id: SubscriberId, capacity: SubscriberCapacity) extends Subscription.Subscriber diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisationSpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/ProjectPrioritisationSpec.scala similarity index 99% rename from event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisationSpec.scala rename to event-log/src/test/scala/io/renku/eventlog/events/producers/ProjectPrioritisationSpec.scala index de54554533..06fe06b9a0 100644 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/ProjectPrioritisationSpec.scala +++ b/event-log/src/test/scala/io/renku/eventlog/events/producers/ProjectPrioritisationSpec.scala @@ -17,15 +17,14 @@ */ package io.renku.eventlog.events.producers -package awaitinggeneration import cats.syntax.all._ import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import eu.timepit.refined.numeric.NonNegative import io.renku.eventlog.events.producers.DefaultSubscribers.DefaultSubscribers -import io.renku.generators.Generators._ import io.renku.generators.Generators.Implicits._ +import io.renku.generators.Generators._ import io.renku.graph.model.EventContentGenerators._ import io.renku.graph.model.GraphModelGenerators._ import io.renku.graph.model.events.EventDate diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/TotalCapacitySpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/TotalCapacitySpec.scala index 4293ab18eb..1118d32035 100644 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/TotalCapacitySpec.scala +++ b/event-log/src/test/scala/io/renku/eventlog/events/producers/TotalCapacitySpec.scala @@ -23,8 +23,9 @@ import io.renku.generators.Generators.{ints, nonNegativeInts} import io.renku.generators.Generators.Implicits._ import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec +import org.scalatestplus.scalacheck.ScalaCheckPropertyChecks -class TotalCapacitySpec extends AnyWordSpec with should.Matchers { +class TotalCapacitySpec extends AnyWordSpec with should.Matchers with ScalaCheckPropertyChecks { "-" should { @@ -44,4 +45,13 @@ class TotalCapacitySpec extends AnyWordSpec with should.Matchers { (totalCapacity - usedCapacity) shouldBe FreeCapacity(0) } } + + "*" should { + + "multiple the Capacity's value by the given value" in { + forAll { (totalCapacity: TotalCapacity, value: Double) => + totalCapacity * value shouldBe totalCapacity.value * value + } + } + } } diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinderSpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinderSpec.scala index 03b84630bd..d1318e2a12 100644 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinderSpec.scala +++ b/event-log/src/test/scala/io/renku/eventlog/events/producers/awaitinggeneration/EventFinderSpec.scala @@ -19,12 +19,11 @@ package io.renku.eventlog.events.producers package awaitinggeneration -import ProjectPrioritisation.Priority.MaxPriority import cats.effect.IO -import cats.syntax.all._ import eu.timepit.refined.auto._ import io.renku.eventlog.InMemoryEventLogDbSpec -import io.renku.eventlog.events.producers.awaitinggeneration.ProjectPrioritisation.{Priority, ProjectInfo} +import io.renku.eventlog.events.producers.ProjectPrioritisation.Priority.MaxPriority +import io.renku.eventlog.events.producers.ProjectPrioritisation.{Priority, ProjectInfo} import io.renku.eventlog.metrics.TestEventStatusGauges._ import io.renku.eventlog.metrics.{EventStatusGauges, QueriesExecutionTimes, TestEventStatusGauges} import io.renku.generators.Generators.Implicits._ @@ -39,6 +38,7 @@ import io.renku.metrics.TestMetricsRegistry import io.renku.testtools.IOSpec import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory +import org.scalatest.OptionValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -50,12 +50,14 @@ private class EventFinderSpec with IOSpec with InMemoryEventLogDbSpec with MockFactory - with should.Matchers { + with should.Matchers + with OptionValues { "popEvent" should { s"find the most recent event in status $New or $GenerationRecoverableFailure " + s"and mark it as $GeneratingTriples" in new TestCase { + val projectId = projectIds.generateOne val projectPath = projectPaths.generateOne @@ -81,9 +83,7 @@ private class EventFinderSpec returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - finder.popEvent().unsafeRunSync() shouldBe Some( - AwaitingGenerationEvent(event2Id, projectPath, event2Body) - ) + finder.popEvent().unsafeRunSync().value shouldBe AwaitingGenerationEvent(event2Id, projectPath, event2Body) gauges.awaitingGeneration.getValue(projectPath).unsafeRunSync() shouldBe -1 gauges.underGeneration.getValue(projectPath).unsafeRunSync() shouldBe 1 @@ -97,6 +97,7 @@ private class EventFinderSpec s"find the most recent event in status $New or $GenerationRecoverableFailure " + "if there are multiple latest events with the same date" in new TestCase { + val projectId = projectIds.generateOne val projectPath = projectPaths.generateOne val latestEventDate = eventDates.generateOne @@ -124,9 +125,9 @@ private class EventFinderSpec returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - finder.popEvent().unsafeRunSync() should { - be(AwaitingGenerationEvent(event1Id, projectPath, event1Body).some) or - be(AwaitingGenerationEvent(event2Id, projectPath, event2Body).some) + finder.popEvent().unsafeRunSync().value should { + be(AwaitingGenerationEvent(event1Id, projectPath, event1Body)) or + be(AwaitingGenerationEvent(event2Id, projectPath, event2Body)) } gauges.awaitingGeneration.getValue(projectPath).unsafeRunSync() shouldBe -1 @@ -137,20 +138,20 @@ private class EventFinderSpec } // 2nd event with the same event date - gauges.awaitingGeneration.getValue(projectPath).unsafeRunSync() shouldBe -2 - gauges.underGeneration.getValue(projectPath).unsafeRunSync() shouldBe 2 - givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, latestEventDate, currentOccupancy = 1)), totalOccupancy = 1, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - finder.popEvent().unsafeRunSync() should { - be(AwaitingGenerationEvent(event1Id, projectPath, event1Body).some) or - be(AwaitingGenerationEvent(event2Id, projectPath, event2Body).some) + finder.popEvent().unsafeRunSync().value should { + be(AwaitingGenerationEvent(event1Id, projectPath, event1Body)) or + be(AwaitingGenerationEvent(event2Id, projectPath, event2Body)) } + gauges.awaitingGeneration.getValue(projectPath).unsafeRunSync() shouldBe -2 + gauges.underGeneration.getValue(projectPath).unsafeRunSync() shouldBe 2 + findEvents(EventStatus.GeneratingTriples).noBatchDate should contain theSameElementsAs List( event1Id -> executionDate, event2Id -> executionDate @@ -165,6 +166,7 @@ private class EventFinderSpec Set(GenerationNonRecoverableFailure, TransformationNonRecoverableFailure, Skipped) foreach { latestEventStatus => s"find the most recent event in status $New or $GenerationRecoverableFailure " + s"if the latest event is in status $latestEventStatus" in new TestCase { + val projectId = projectIds.generateOne val projectPath = projectPaths.generateOne @@ -190,9 +192,7 @@ private class EventFinderSpec returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - finder.popEvent().unsafeRunSync() shouldBe Some( - AwaitingGenerationEvent(event1Id, projectPath, event1Body) - ) + finder.popEvent().unsafeRunSync().value shouldBe AwaitingGenerationEvent(event1Id, projectPath, event1Body) gauges.awaitingGeneration.getValue(projectPath).unsafeRunSync() shouldBe -1 gauges.underGeneration.getValue(projectPath).unsafeRunSync() shouldBe 1 @@ -349,10 +349,10 @@ private class EventFinderSpec val currentTime = mockFunction[Instant] currentTime.expects().returning(now).anyNumberOfTimes() - implicit val gauges: EventStatusGauges[IO] = TestEventStatusGauges[IO] - val projectPrioritisation = mock[ProjectPrioritisation[IO]] + implicit val gauges: EventStatusGauges[IO] = TestEventStatusGauges[IO] private implicit val metricsRegistry: TestMetricsRegistry[IO] = TestMetricsRegistry[IO] implicit val queriesExecTimes: QueriesExecutionTimes[IO] = QueriesExecutionTimes[IO]().unsafeRunSync() + val projectPrioritisation = mock[ProjectPrioritisation[IO]] def givenPrioritisation(takes: List[ProjectInfo], totalOccupancy: Long, returns: List[(ProjectIds, Priority)]) = (projectPrioritisation.prioritise _) diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinderSpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinderSpec.scala index a44cf0567f..db85f32a89 100644 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinderSpec.scala +++ b/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/EventFinderSpec.scala @@ -23,6 +23,8 @@ import cats.effect.IO import cats.syntax.all._ import eu.timepit.refined.auto._ import io.renku.eventlog.InMemoryEventLogDbSpec +import io.renku.eventlog.events.producers.ProjectPrioritisation.Priority.MaxPriority +import io.renku.eventlog.events.producers.ProjectPrioritisation.{Priority, ProjectInfo} import io.renku.eventlog.metrics.TestEventStatusGauges._ import io.renku.eventlog.metrics.{EventStatusGauges, QueriesExecutionTimes, TestEventStatusGauges} import io.renku.generators.Generators.Implicits._ @@ -37,10 +39,9 @@ import io.renku.metrics.TestMetricsRegistry import io.renku.testtools.IOSpec import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory +import org.scalatest.OptionValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -import triplesgenerated.ProjectPrioritisation.Priority.MaxPriority -import triplesgenerated.ProjectPrioritisation.{Priority, ProjectInfo} import java.time.Instant @@ -49,12 +50,12 @@ private class EventFinderSpec with IOSpec with InMemoryEventLogDbSpec with MockFactory - with should.Matchers { + with should.Matchers + with OptionValues { "popEvent" should { - "return an event with the latest event date " + - s"and status $TriplesGenerated or $TransformationRecoverableFailure " + + s"return the most recent event in status $TriplesGenerated or $TransformationRecoverableFailure " + s"and mark it as $TransformingTriples" in new TestCase { val projectId = projectIds.generateOne @@ -78,17 +79,18 @@ private class EventFinderSpec givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, latestEventDate, 0)), + totalOccupancy = 0, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEventId, actualPath, actualPayload)) = finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEventId, actualPath, actualPayload) = finder.popEvent().unsafeRunSync().value actualEventId shouldBe event1Id actualPath shouldBe projectPath actualPayload.value should contain theSameElementsAs eventPayload1.value findEvents(TransformingTriples).noBatchDate shouldBe List((event1Id, executionDate)) - givenPrioritisation(takes = Nil, returns = Nil) + givenPrioritisation(takes = Nil, totalOccupancy = 1, returns = Nil) finder.popEvent().unsafeRunSync() shouldBe None @@ -123,10 +125,11 @@ private class EventFinderSpec // 1st event with the same event date givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, latestEventDate, 0)), + totalOccupancy = 0, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEventId, actualPath, actualPayload)) = finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEventId, actualPath, actualPayload) = finder.popEvent().unsafeRunSync().value if (actualEventId == event1Id) { actualEventId shouldBe event1Id actualPath shouldBe projectPath @@ -141,17 +144,18 @@ private class EventFinderSpec be(List((event1Id, executionDate))) or be(List((event2Id, executionDate))) } - gauges.awaitingDeletion.getValue(projectPath).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(projectPath).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(projectPath).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(projectPath).unsafeRunSync() shouldBe 1d // 2nd event with the same event date givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, latestEventDate, 1)), + totalOccupancy = 1, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(nextActualEventId, nextActualPath, nextActualPayload)) = - finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(nextActualEventId, nextActualPath, nextActualPayload) = + finder.popEvent().unsafeRunSync().value if (nextActualEventId == event1Id) { nextActualEventId shouldBe event1Id nextActualPath shouldBe projectPath @@ -166,11 +170,11 @@ private class EventFinderSpec event2Id -> executionDate ) - gauges.awaitingDeletion.getValue(projectPath).unsafeRunSync() shouldBe -2d - gauges.underDeletion.getValue(projectPath).unsafeRunSync() shouldBe 2d + gauges.awaitingTransformation.getValue(projectPath).unsafeRunSync() shouldBe -2d + gauges.underTransformation.getValue(projectPath).unsafeRunSync() shouldBe 2d // no more events left - givenPrioritisation(takes = Nil, returns = Nil) + givenPrioritisation(takes = Nil, totalOccupancy = 2, returns = Nil) finder.popEvent().unsafeRunSync() shouldBe None } @@ -194,18 +198,19 @@ private class EventFinderSpec givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, latestEventDate, 0)), + totalOccupancy = 0, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEvent1Id, actualEvent1Path, actualEvent1Payload)) = - finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEvent1Id, actualEvent1Path, actualEvent1Payload) = + finder.popEvent().unsafeRunSync().value actualEvent1Id shouldBe event1Id actualEvent1Path shouldBe projectPath actualEvent1Payload.value should contain theSameElementsAs eventPayload1.value - gauges.awaitingDeletion.getValue(projectPath).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(projectPath).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(projectPath).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(projectPath).unsafeRunSync() shouldBe 1d findEvents(TransformingTriples).noBatchDate shouldBe List((event1Id, executionDate)) @@ -218,23 +223,25 @@ private class EventFinderSpec givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, newerLatestEventDate, 1)), + totalOccupancy = 1, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEvent2Id, actualEvent2Path, actualEvent2Payload)) = - finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEvent2Id, actualEvent2Path, actualEvent2Payload) = + finder.popEvent().unsafeRunSync().value actualEvent2Id shouldBe event2Id actualEvent2Path shouldBe projectPath actualEvent2Payload.value should contain theSameElementsAs eventPayload2.value - gauges.awaitingDeletion.getValue(projectPath).unsafeRunSync() shouldBe -2d - gauges.underDeletion.getValue(projectPath).unsafeRunSync() shouldBe 2d + gauges.awaitingTransformation.getValue(projectPath).unsafeRunSync() shouldBe -2d + gauges.underTransformation.getValue(projectPath).unsafeRunSync() shouldBe 2d findEvents(TransformingTriples).noBatchDate shouldBe List((event1Id, executionDate), (event2Id, executionDate)) } s"skip events in $TriplesGenerated status which do not have payload - within a project" in new TestCase { + val projectId = projectIds.generateOne val projectPath = projectPaths.generateOne @@ -259,23 +266,25 @@ private class EventFinderSpec givenPrioritisation( takes = List(ProjectInfo(projectId, projectPath, event1Date, 0)), + totalOccupancy = 0, returns = List(ProjectIds(projectId, projectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEventId, actualEventPath, actualEventPayload)) = - finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEventId, actualEventPath, actualEventPayload) = + finder.popEvent().unsafeRunSync().value actualEventId shouldBe event2Id actualEventPath shouldBe projectPath actualEventPayload.value should contain theSameElementsAs eventPayload2.value - gauges.awaitingDeletion.getValue(projectPath).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(projectPath).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(projectPath).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(projectPath).unsafeRunSync() shouldBe 1d findEvents(TransformingTriples).noBatchDate shouldBe List((event2Id, executionDate)) } s"skip projects with events in $TriplesGenerated status which do not have payload" in new TestCase { + val event1ProjectId = projectIds.generateOne val event1ProjectPath = projectPaths.generateOne @@ -303,19 +312,20 @@ private class EventFinderSpec givenPrioritisation( takes = List(ProjectInfo(event2ProjectId, event2ProjectPath, event2Date, 0)), + totalOccupancy = 0, returns = List(ProjectIds(event2ProjectId, event2ProjectPath) -> MaxPriority) ) - val Some(TriplesGeneratedEvent(actualEventId, actualEventPath, actualEventPayload)) = - finder.popEvent().unsafeRunSync() + val TriplesGeneratedEvent(actualEventId, actualEventPath, actualEventPayload) = + finder.popEvent().unsafeRunSync().value actualEventId shouldBe event2Id actualEventPath shouldBe event2ProjectPath actualEventPayload.value should contain theSameElementsAs eventPayload2.value findEvents(TransformingTriples).noBatchDate shouldBe List((event2Id, executionDate)) - gauges.awaitingDeletion.getValue(event2ProjectPath).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(event2ProjectPath).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(event2ProjectPath).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(event2ProjectPath).unsafeRunSync() shouldBe 1d } "return no event when execution date is in the future " + @@ -349,7 +359,7 @@ private class EventFinderSpec findEvents(TransformingTriples) shouldBe List.empty - givenPrioritisation(takes = Nil, returns = Nil) + givenPrioritisation(takes = Nil, totalOccupancy = 0, returns = Nil) finder.popEvent().unsafeRunSync() shouldBe None } @@ -358,14 +368,15 @@ private class EventFinderSpec val events = readyStatuses .generateNonEmptyList(min = 2) - .map(status => createEvent(status)) + .map(createEvent(_)) .toList findEvents(TransformingTriples) shouldBe List.empty - events foreach { case (eventId, _, eventDate, projectPath, _) => + events.sortBy(_._3).reverse.zipWithIndex foreach { case ((eventId, _, eventDate, projectPath, _), idx) => givenPrioritisation( takes = List(ProjectInfo(eventId.projectId, projectPath, eventDate, 0)), + totalOccupancy = idx, returns = List(ProjectIds(eventId.projectId, projectPath) -> MaxPriority) ) } @@ -375,8 +386,8 @@ private class EventFinderSpec } events foreach { case (_, _, _, path, _) => - gauges.awaitingDeletion.getValue(path).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(path).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(path).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(path).unsafeRunSync() shouldBe 1d } findEvents(status = TransformingTriples).eventIdsOnly should contain theSameElementsAs events.map(_._1) @@ -400,12 +411,12 @@ private class EventFinderSpec val eventsGroupedByProjects = events.groupBy(_._1.projectId) - eventsGroupedByProjects foreach { - case (projectId, (_, _, _, projectPath, _) :: _) => + eventsGroupedByProjects.zipWithIndex foreach { + case ((projectId, (_, _, _, projectPath, _) :: _), idx) => (projectPrioritisation.prioritise _) - .expects(*) + .expects(*, idx) .returning(List(ProjectIds(projectId, projectPath) -> MaxPriority)) - case (_, Nil) => () + case ((_, Nil), _) => () } eventsGroupedByProjects foreach { _ => @@ -414,15 +425,15 @@ private class EventFinderSpec eventsGroupedByProjects foreach { case (_, list) => val path = list.head._4 - gauges.awaitingDeletion.getValue(path).unsafeRunSync() shouldBe -1d - gauges.underDeletion.getValue(path).unsafeRunSync() shouldBe 1d + gauges.awaitingTransformation.getValue(path).unsafeRunSync() shouldBe -1d + gauges.underTransformation.getValue(path).unsafeRunSync() shouldBe 1d } findEvents(TransformingTriples).eventIdsOnly should contain theSameElementsAs eventsGroupedByProjects.map { case (_, projectEvents) => projectEvents.maxBy(_._3)._1 }.toList - givenPrioritisation(takes = Nil, returns = Nil) + givenPrioritisation(takes = Nil, totalOccupancy = eventsGroupedByProjects.size, returns = Nil) eventLogFind.popEvent().unsafeRunSync() shouldBe None } @@ -434,14 +445,14 @@ private class EventFinderSpec val currentTime = mockFunction[Instant] currentTime.expects().returning(now).anyNumberOfTimes() - val projectPrioritisation = mock[ProjectPrioritisation] implicit val gauges: EventStatusGauges[IO] = TestEventStatusGauges[IO] private implicit val metricsRegistry: TestMetricsRegistry[IO] = TestMetricsRegistry[IO] implicit val queriesExecTimes: QueriesExecutionTimes[IO] = QueriesExecutionTimes[IO]().unsafeRunSync() + val projectPrioritisation = mock[ProjectPrioritisation[IO]] - def givenPrioritisation(takes: List[ProjectInfo], returns: List[(ProjectIds, Priority)]) = + def givenPrioritisation(takes: List[ProjectInfo], totalOccupancy: Long, returns: List[(ProjectIds, Priority)]) = (projectPrioritisation.prioritise _) - .expects(takes) + .expects(takes, totalOccupancy) .returning(returns) } diff --git a/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisationSpec.scala b/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisationSpec.scala deleted file mode 100644 index c9149b9ed5..0000000000 --- a/event-log/src/test/scala/io/renku/eventlog/events/producers/triplesgenerated/ProjectPrioritisationSpec.scala +++ /dev/null @@ -1,91 +0,0 @@ -/* - * 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.eventlog.events.producers -package triplesgenerated - -import eu.timepit.refined.auto._ -import io.renku.graph.model.EventContentGenerators._ -import io.renku.generators.Generators.Implicits._ -import io.renku.generators.Generators._ -import io.renku.graph.model.GraphModelGenerators._ -import io.renku.graph.model.events.EventDate -import org.scalacheck.Gen -import org.scalatest.matchers.should -import org.scalatest.wordspec.AnyWordSpec - -import scala.concurrent.duration._ - -private class ProjectPrioritisationSpec extends AnyWordSpec with should.Matchers { - import ProjectPrioritisation._ - import Priority._ - - "prioritize" should { - - "return an empty list if there are no projects" in { - projectPrioritisation.prioritise(Nil) shouldBe Nil - } - - "put priority MaxPriority if the list contains only one project" in { - val project = projectInfos.generateOne - - projectPrioritisation.prioritise(List(project)) shouldBe List((project.toIdsAndPath, MaxPriority)) - } - - "compute the priority using the event date only " in { - val project1 = projectInfos.generateOne - val project2 = projectInfos.generateOne.copy( - latestEventDate = project1.latestEventDate.plus(durations(min = 61 minutes, max = 5 hours).generateOne), - currentOccupancy = 0 - ) - val project3 = projectInfos.generateOne.copy( - latestEventDate = project2.latestEventDate.plus(durations(min = 61 minutes, max = 5 hours).generateOne), - currentOccupancy = 0 - ) - - val (projectIdAndPath1, priority1) :: (projectIdAndPath2, priority2) :: (projectIdAndPath3, priority3) :: Nil = - projectPrioritisation.prioritise(List(project1, project2, project3)) - - projectIdAndPath1.id shouldBe project1.id - projectIdAndPath2.id shouldBe project2.id - projectIdAndPath3.id shouldBe project3.id - - priority1.value shouldBe BigDecimal(1.0) - priority2.value should (be < BigDecimal(1.0) and be > BigDecimal(0.1)) - priority3.value shouldBe BigDecimal(0.1) - } - } - - private lazy val projectPrioritisation = new ProjectPrioritisation() - - private implicit lazy val projectInfos: Gen[ProjectInfo] = for { - projectId <- projectIds - projectPath <- projectPaths - latestEvenDate <- eventDates - currentOccupancy <- nonNegativeInts(20) - } yield ProjectInfo(projectId, projectPath, latestEvenDate, currentOccupancy) - - private implicit class EventDateOps(eventDate: EventDate) { - def plus(duration: FiniteDuration): EventDate = - EventDate(eventDate.value plusMillis duration.toMillis) - } - - private implicit class ProjectInfoOps(projectInfo: ProjectInfo) { - lazy val toIdsAndPath = ProjectIds(projectInfo.id, projectInfo.path) - } -} diff --git a/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala b/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala index efc974148b..69919a3635 100644 --- a/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala +++ b/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala @@ -23,7 +23,8 @@ import cats.syntax.all._ import com.typesafe.config.Config import io.renku.tinytypes._ import pureconfig._ -import pureconfig.error.{CannotConvert, ConfigReaderFailures} +import pureconfig.error.{CannotConvert, ConfigReaderFailures, FailureReason} +import scodec.bits.ByteVector abstract class ConfigLoader[F[_]: MonadThrow] { @@ -76,4 +77,11 @@ object ConfigLoader { } .getOrElse(Left(CannotConvert(stringValue, ttApply.getClass.toString, "Not an int value"))) } + + implicit def base64ByteVectorReader: ConfigReader[ByteVector] = + ConfigReader.fromString { str => + ByteVector + .fromBase64Descriptive(str) + .leftMap(err => new FailureReason { override lazy val description: String = s"Cannot read base64: $err" }) + } } diff --git a/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala b/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala index 50a824c0b8..7d768030e0 100644 --- a/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala +++ b/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala @@ -19,10 +19,12 @@ package io.renku.crypto import cats.MonadThrow -import eu.timepit.refined.W -import eu.timepit.refined.api.Refined -import eu.timepit.refined.collection.MinSize +import cats.syntax.all._ +import io.renku.config.ConfigLoader.base64ByteVectorReader import io.renku.crypto.AesCrypto.Secret +import pureconfig.ConfigReader +import pureconfig.error.FailureReason +import scodec.bits.ByteVector import java.nio.charset.StandardCharsets.UTF_8 import java.util.Base64 @@ -30,42 +32,62 @@ import javax.crypto.Cipher import javax.crypto.Cipher.{DECRYPT_MODE, ENCRYPT_MODE} import javax.crypto.spec.{IvParameterSpec, SecretKeySpec} -abstract class AesCrypto[F[_]: MonadThrow, NONENCRYPTED, ENCRYPTED]( - secret: Secret -) { +abstract class AesCrypto[F[_]: MonadThrow, NONENCRYPTED, ENCRYPTED](secret: Secret) { - private val base64Decoder = Base64.getDecoder - private val base64Encoder = Base64.getEncoder - private val algorithm = "AES/CBC/PKCS5Padding" - private val key = new SecretKeySpec(base64Decoder.decode(secret.value).takeWhile(_ != 10), "AES") - private val ivSpec = new IvParameterSpec(new Array[Byte](16)) - private val encryptingCipher = cipher(ENCRYPT_MODE) - private val decryptingCipher = cipher(DECRYPT_MODE) + private val base64Decoder = Base64.getDecoder + private val base64Encoder = Base64.getEncoder + private val algorithm = "AES/CBC/PKCS5Padding" + private val ivSpec = new IvParameterSpec(new Array[Byte](16)) def encrypt(nonEncrypted: NONENCRYPTED): F[ENCRYPTED] def decrypt(encrypted: ENCRYPTED): F[NONENCRYPTED] private def cipher(mode: Int): Cipher = { val c = Cipher.getInstance(algorithm) - c.init(mode, key, ivSpec) + c.init(mode, new SecretKeySpec(secret.value.toArray, "AES"), ivSpec) c } protected def encryptAndEncode(toEncryptAndEncode: String): F[String] = MonadThrow[F].catchNonFatal { - new String( - base64Encoder.encode(encryptingCipher.doFinal(toEncryptAndEncode.getBytes(UTF_8))), - UTF_8 - ) + base64Encoder.encodeToString(cipher(ENCRYPT_MODE).doFinal(toEncryptAndEncode.getBytes(UTF_8))) } protected def decodeAndDecrypt(toDecodeAndDecrypt: String): F[String] = MonadThrow[F].catchNonFatal { new String( - decryptingCipher.doFinal(base64Decoder.decode(toDecodeAndDecrypt.getBytes(UTF_8))), + cipher(DECRYPT_MODE).doFinal(base64Decoder.decode(toDecodeAndDecrypt.getBytes(UTF_8))), UTF_8 ) } } object AesCrypto { - type Secret = String Refined MinSize[W.`16`.T] + + final class Secret private (val value: ByteVector) extends AnyVal { + override def toString: String = "" + def toBase64: String = value.toBase64 + } + + object Secret { + + def apply(value: ByteVector): Either[String, Secret] = { + val expectedLength = 16 + if (value.length == expectedLength) Right(new Secret(value)) + else Left(s"Expected $expectedLength bytes, but got ${value.length}") + } + + def unsafe(value: ByteVector): Secret = apply(value).fold(sys.error, identity) + + def fromBase64(b64: String): Either[String, Secret] = + ByteVector.fromBase64Descriptive(b64).flatMap(apply) + + def unsafeFromBase64(b64: String): Secret = + fromBase64(b64).fold(sys.error, identity) + + implicit def secretReader: ConfigReader[Secret] = + base64ByteVectorReader.emap(bv => + Secret(bv.takeWhile(_ != 10)).leftMap(err => + new FailureReason { override lazy val description: String = s"Cannot read AES secret: $err" } + ) + ) + } } diff --git a/graph-commons/src/main/scala/io/renku/http/client/AccessToken.scala b/graph-commons/src/main/scala/io/renku/http/client/AccessToken.scala index 9cec201ca6..c063f642bb 100644 --- a/graph-commons/src/main/scala/io/renku/http/client/AccessToken.scala +++ b/graph-commons/src/main/scala/io/renku/http/client/AccessToken.scala @@ -52,7 +52,7 @@ object AccessToken { private val base64Decoder = Base64.getDecoder private val base64Encoder = Base64.getEncoder - implicit val accessTokenEncoder: Encoder[AccessToken] = { + implicit def accessTokenEncoder[A <: AccessToken]: Encoder[A] = { case ProjectAccessToken(token) => Json.obj("projectAccessToken" -> Json.fromString(new String(base64Encoder.encode(token.getBytes(UTF_8)), UTF_8))) case UserOAuthAccessToken(token) => diff --git a/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala b/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala new file mode 100644 index 0000000000..14135d01be --- /dev/null +++ b/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala @@ -0,0 +1,74 @@ +/* + * 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.crypto + +import com.typesafe.config.ConfigFactory +import io.renku.crypto.AesCrypto.Secret +import io.renku.generators.CommonGraphGenerators.aesCryptoSecrets +import io.renku.generators.Generators.Implicits._ +import org.scalacheck.Gen +import org.scalatest.EitherValues +import org.scalatest.matchers.should +import org.scalatest.wordspec.AnyWordSpec +import pureconfig.ConfigSource +import scodec.bits.Bases.Alphabets +import scodec.bits.ByteVector + +class SecretSpec extends AnyWordSpec with should.Matchers with EitherValues { + + "secretReader" should { + + val keyName = "secret" + + "decode Base64 encoded secret" in { + + val secret = aesCryptoSecrets.generateOne + + val config = ConfigFactory.parseString(s"""$keyName = "${secret.toBase64}"""") + + ConfigSource.fromConfig(config).at(keyName).load[Secret].value.toBase64 shouldBe secret.toBase64 + } + + "decode Base64 encoded secret ending with LF char" in { + + val secret = aesCryptoSecrets.generateOne + + val config = ConfigFactory.parseString(s"""$keyName = "${(secret.value :+ 10.toByte).toBase64}"""") + + ConfigSource.fromConfig(config).at(keyName).load[Secret].value.toBase64 shouldBe secret.toBase64 + } + + "fail and not print the secret if decoding fails" in { + + val value = Gen + .listOfN(2, Gen.hexChar) + .map(_.mkString.toLowerCase) + .map(ByteVector.fromValidHex(_, Alphabets.HexLowercase)) + .generateOne + .toBase64 + + val config = ConfigFactory.parseString(s"""$keyName = "$value"""") + + val failure = ConfigSource.fromConfig(config).at(keyName).load[Secret].left.value.prettyPrint() + + failure should include("Cannot read AES secret") + failure should not include value + } + } +} diff --git a/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala b/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala index 357302be71..3e78698146 100644 --- a/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala +++ b/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala @@ -26,7 +26,7 @@ import io.renku.config.certificates.Certificate import io.renku.config.sentry.SentryConfig import io.renku.config.sentry.SentryConfig.{Dsn, Environment} import io.renku.control.{RateLimit, RateLimitUnit} -import io.renku.crypto.AesCrypto +import io.renku.crypto.AesCrypto.Secret import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.http.server.security.Authorizer.AuthContext @@ -49,20 +49,21 @@ import io.renku.triplesstore._ import org.http4s.Status import org.http4s.Status._ import org.scalacheck.{Arbitrary, Gen} +import scodec.bits.Bases.Alphabets +import scodec.bits.ByteVector -import java.nio.charset.StandardCharsets.UTF_8 -import java.util.Base64 import scala.language.implicitConversions import scala.util.Try object CommonGraphGenerators { - implicit val aesCryptoSecrets: Gen[AesCrypto.Secret] = - stringsOfLength(16) - .map(_.getBytes(UTF_8)) - .map(Base64.getEncoder.encode) - .map(new String(_, UTF_8)) - .map(Refined.unsafeApply) + implicit val aesCryptoSecrets: Gen[Secret] = + Gen + .listOfN(32, Gen.hexChar) + .map(_.mkString.toLowerCase) + .map(ByteVector.fromValidHex(_, Alphabets.HexLowercase)) + .retryUntil(_.takeWhile(_ != 10.toByte).length == 16) + .map(Secret.unsafe) implicit val personalAccessTokens: Gen[PersonalAccessToken] = for { length <- Gen.choose(5, 40) diff --git a/graph-commons/src/test/scala/io/renku/testtools/GitLabClientTools.scala b/graph-commons/src/test/scala/io/renku/testtools/GitLabClientTools.scala index a737c3c45b..de166f0c2b 100644 --- a/graph-commons/src/test/scala/io/renku/testtools/GitLabClientTools.scala +++ b/graph-commons/src/test/scala/io/renku/testtools/GitLabClientTools.scala @@ -30,6 +30,7 @@ import org.http4s.Method.{DELETE, GET, HEAD, POST} import org.http4s.{Method, Uri} import org.scalacheck.Gen import org.scalamock.matchers.ArgCapture.CaptureOne +import org.scalamock.matchers.MockParameter import org.scalamock.scalatest.MockFactory trait GitLabClientTools[F[_]] { @@ -39,6 +40,7 @@ trait GitLabClientTools[F[_]] { findingMethod: => Any, resultGenerator: Gen[ResultType], method: Method = GET, + maybeEndpointName: Option[String Refined NonEmpty] = None, expectedNumberOfCalls: Int = 1 )(implicit applicative: Applicative[F]): ResponseMappingF[F, ResultType] = { val responseMapping = CaptureOne[ResponseMappingF[F, ResultType]]() @@ -49,7 +51,7 @@ trait GitLabClientTools[F[_]] { .get(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[F, ResultType])( _: Option[AccessToken] )) - .expects(*, *, capture(responseMapping), *) + .expects(*, maybeEndpointName.map(new MockParameter(_)).getOrElse(*), capture(responseMapping), *) .returning(resultGenerator.generateOne.pure[F]) .repeat(expectedNumberOfCalls) case HEAD => @@ -57,7 +59,7 @@ trait GitLabClientTools[F[_]] { .head(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[F, ResultType])( _: Option[AccessToken] )) - .expects(*, *, capture(responseMapping), *) + .expects(*, maybeEndpointName.map(new MockParameter(_)).getOrElse(*), capture(responseMapping), *) .returning(resultGenerator.generateOne.pure[F]) .repeat(expectedNumberOfCalls) case POST => @@ -65,7 +67,7 @@ trait GitLabClientTools[F[_]] { .post(_: Uri, _: String Refined NonEmpty, _: Json)(_: ResponseMappingF[F, ResultType])( _: Option[AccessToken] )) - .expects(*, *, *, capture(responseMapping), *) + .expects(*, maybeEndpointName.map(new MockParameter(_)).getOrElse(*), *, capture(responseMapping), *) .returning(resultGenerator.generateOne.pure[F]) .repeat(expectedNumberOfCalls) case DELETE => @@ -73,7 +75,7 @@ trait GitLabClientTools[F[_]] { .delete(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[F, ResultType])( _: Option[AccessToken] )) - .expects(*, *, capture(responseMapping), *) + .expects(*, maybeEndpointName.map(new MockParameter(_)).getOrElse(*), capture(responseMapping), *) .returning(resultGenerator.generateOne.pure[F]) .repeat(expectedNumberOfCalls) } diff --git a/helm-chart/renku-graph/values.yaml b/helm-chart/renku-graph/values.yaml index 37332fb6e5..7602b1120d 100644 --- a/helm-chart/renku-graph/values.yaml +++ b/helm-chart/renku-graph/values.yaml @@ -48,6 +48,10 @@ jena: additionalEnvironmentVariables: - name: JVM_ARGS value: -Xmx2G -Xms2G + resources: + requests: + cpu: 1000m + memory: 3Gi additionalVolumeMounts: - name: shiro mountPath: /fuseki/shiro.ini @@ -72,6 +76,7 @@ webhookService: port: 80 resources: requests: + cpu: 500m memory: 256Mi jvmXmx: 128M gitlab: @@ -91,6 +96,7 @@ tokenRepository: port: 9003 resources: requests: + cpu: 500m memory: 256Mi jvmXmx: 128M gitlab: diff --git a/renku-model-tiny-types/src/main/scala/io/renku/graph/model/persons.scala b/renku-model-tiny-types/src/main/scala/io/renku/graph/model/persons.scala index 9856b8a60c..65eb3c909e 100644 --- a/renku-model-tiny-types/src/main/scala/io/renku/graph/model/persons.scala +++ b/renku-model-tiny-types/src/main/scala/io/renku/graph/model/persons.scala @@ -108,7 +108,7 @@ object persons { def apply(orcidId: OrcidId)(implicit renkuUrl: RenkuUrl, ev: OrcidId.type): OrcidIdBased = new OrcidIdBased((renkuUrl / "persons" / "orcid" / orcidId.id).show) - def apply(email: Email): EmailBased = new EmailBased(show"mailto:$email") + def apply(email: Email): EmailBased = new EmailBased(show"mailto:${email.value}") def apply(name: Name)(implicit renkuUrl: RenkuUrl): NameBased = new NameBased((renkuUrl / "persons" / name).show) @@ -188,7 +188,7 @@ object persons { ) } - final class Email private (val value: String) extends AnyVal with StringTinyType + final class Email private (val value: String) extends AnyVal with StringTinyType with Sensitive implicit object Email extends TinyTypeFactory[Email](new Email(_)) with NonBlank[Email] diff --git a/renku-model-tiny-types/src/test/scala/io/renku/graph/model/personsSpec.scala b/renku-model-tiny-types/src/test/scala/io/renku/graph/model/personsSpec.scala index 2a877918fe..ea08eddac8 100644 --- a/renku-model-tiny-types/src/test/scala/io/renku/graph/model/personsSpec.scala +++ b/renku-model-tiny-types/src/test/scala/io/renku/graph/model/personsSpec.scala @@ -23,6 +23,7 @@ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.model.persons.{Email, ResourceId} import io.renku.graph.model.views.RdfResource +import io.renku.tinytypes.Sensitive import io.renku.tinytypes.constraints.NonBlank import org.apache.jena.util.URIref import org.scalacheck.Gen @@ -38,6 +39,10 @@ class EmailSpec extends AnyWordSpec with ScalaCheckPropertyChecks with should.Ma "be a NonBlank" in { Email shouldBe a[NonBlank[_]] } + + "be Sensitive" in { + personEmails.generateOne shouldBe a[Sensitive] + } } "instantiation" should { @@ -117,7 +122,7 @@ class PersonResourceIdSpec "apply(Email)" should { "generate 'mailto:email' ResourceId" in { val email = personEmails.generateOne - ResourceId(email).show shouldBe show"mailto:$email" + ResourceId(email).show shouldBe show"mailto:${email.value}" } } diff --git a/token-repository/README.md b/token-repository/README.md index f02bfa90b9..c03298c0e4 100644 --- a/token-repository/README.md +++ b/token-repository/README.md @@ -59,7 +59,7 @@ Response for a case when the token is an OAuth Access Token { "oauthAccessToken": "" } ``` -#### PUT /projects/:id/tokens +#### POST /projects/:id/tokens Associates the given token and project id. It succeeds regardless of the association is newly created, it existed before or it got updated. diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/AccessTokenCrypto.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/AccessTokenCrypto.scala index 15676b4fdb..bcadce447f 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/AccessTokenCrypto.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/AccessTokenCrypto.scala @@ -24,7 +24,6 @@ import cats.syntax.all._ import com.typesafe.config.{Config, ConfigFactory} import eu.timepit.refined.W import eu.timepit.refined.api.{RefType, Refined} -import eu.timepit.refined.pureconfig._ import eu.timepit.refined.string.MatchesRegex import io.circe._ import io.circe.parser._ diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpoint.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpoint.scala index 2323b367ed..0065b18bc9 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpoint.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpoint.scala @@ -66,7 +66,7 @@ class CreateTokenEndpointImpl[F[_]: Concurrent: Logger]( case BadRequestError(exception) => BadRequest(ErrorMessage(exception)) case NonFatal(exception) => - val errorMessage = ErrorMessage(s"Associating token with projectId: $projectId failed") + val errorMessage = ErrorMessage(show"Associating token with projectId: $projectId failed") Logger[F].error(exception)(errorMessage.value) >> InternalServerError(errorMessage) } } diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/NewTokensCreator.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/NewTokensCreator.scala index b10bc1f823..bbd2f938db 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/NewTokensCreator.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/NewTokensCreator.scala @@ -25,11 +25,12 @@ import com.typesafe.config.{Config, ConfigFactory} import io.renku.graph.model.projects import io.renku.http.client.AccessToken.ProjectAccessToken import io.renku.http.client.{AccessToken, GitLabClient} +import io.renku.http.tinytypes.TinyTypeURIEncoder._ import java.time.{LocalDate, Period} private[tokenrepository] trait NewTokensCreator[F[_]] { - def createPersonalAccessToken(projectId: projects.GitLabId, accessToken: AccessToken): OptionT[F, TokenCreationInfo] + def createProjectAccessToken(projectId: projects.GitLabId, accessToken: AccessToken): OptionT[F, TokenCreationInfo] } private[tokenrepository] object NewTokensCreator { @@ -61,13 +62,12 @@ private class NewTokensCreatorImpl[F[_]: Async: GitLabClient]( import org.http4s.implicits._ import org.http4s.{EntityDecoder, Request, Response, Status} - override def createPersonalAccessToken(projectId: projects.GitLabId, - accessToken: AccessToken + override def createProjectAccessToken(projectId: projects.GitLabId, + accessToken: AccessToken ): OptionT[F, TokenCreationInfo] = OptionT { - GitLabClient[F].post(uri"projects" / projectId.value / "access_tokens", - "create-project-access-token", - createPayload() - )(mapResponse)(accessToken.some) + GitLabClient[F].post(uri"projects" / projectId / "access_tokens", "create-project-access-token", createPayload())( + mapResponse + )(accessToken.some) } private def createPayload() = json"""{ diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokenValidator.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokenValidator.scala index 379e15eff1..a41ddd30dc 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokenValidator.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokenValidator.scala @@ -19,29 +19,86 @@ package io.renku.tokenrepository.repository.creation import cats.MonadThrow +import cats.effect.Async import cats.syntax.all._ import eu.timepit.refined.auto._ +import io.circe.Decoder +import io.circe.Decoder.decodeOption +import io.renku.graph.model.{persons, projects} import io.renku.http.client.{AccessToken, GitLabClient} +import io.renku.http.tinytypes.TinyTypeURIEncoder._ +import org.http4s.Status.{Forbidden, NotFound, Ok, Unauthorized} +import org.http4s._ +import org.http4s.circe.jsonOf +import org.http4s.implicits._ private[tokenrepository] trait TokenValidator[F[_]] { - def checkValid(token: AccessToken): F[Boolean] + def checkValid(projectId: projects.GitLabId, token: AccessToken): F[Boolean] } private[tokenrepository] object TokenValidator { - def apply[F[_]: MonadThrow: GitLabClient]: F[TokenValidator[F]] = new TokenValidatorImpl[F].pure[F].widen + def apply[F[_]: Async: GitLabClient]: F[TokenValidator[F]] = + new TokenValidatorImpl[F](new UserIdFinderImpl[F], new MemberRightsCheckerImpl[F]).pure[F].widen } -private class TokenValidatorImpl[F[_]: MonadThrow: GitLabClient] extends TokenValidator[F] { +private class TokenValidatorImpl[F[_]: MonadThrow](userIdFinder: UserIdFinder[F], + memberRightsChecker: MemberRightsChecker[F] +) extends TokenValidator[F] { - import org.http4s.Status.{Forbidden, NotFound, Ok, Unauthorized} - import org.http4s._ - import org.http4s.implicits._ + override def checkValid(projectId: projects.GitLabId, token: AccessToken): F[Boolean] = + userIdFinder.findUserId(token) >>= { + case None => false.pure[F] + case Some(userId) => memberRightsChecker.checkRights(projectId, userId, token) + } +} + +private trait UserIdFinder[F[_]] { + def findUserId(token: AccessToken): F[Option[persons.GitLabId]] +} +private class UserIdFinderImpl[F[_]: Async: GitLabClient] extends UserIdFinder[F] { + + override def findUserId(token: AccessToken): F[Option[persons.GitLabId]] = + GitLabClient[F].get(uri"user", "user")(mapUserResponse)(token.some) - override def checkValid(token: AccessToken): F[Boolean] = - GitLabClient[F].head(uri"user", "user")(mapResponse)(token.some) + private lazy val mapUserResponse: PartialFunction[(Status, Request[F], Response[F]), F[Option[persons.GitLabId]]] = { + case (Ok, _, resp) => resp.as[Option[persons.GitLabId]] + case (Unauthorized | Forbidden | NotFound, _, _) => Option.empty[persons.GitLabId].pure[F] + } + + private implicit lazy val userIdDecoder: EntityDecoder[F, Option[persons.GitLabId]] = { - private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[Boolean]] = { - case (Ok, _, _) => true.pure[F] + implicit val decoder: Decoder[Option[persons.GitLabId]] = Decoder.instance { + import io.renku.tinytypes.json.TinyTypeDecoders.intDecoder + _.downField("id").as[Option[persons.GitLabId]](decodeOption(intDecoder(persons.GitLabId))) + } + + jsonOf[F, Option[persons.GitLabId]] + } +} + +private trait MemberRightsChecker[F[_]] { + def checkRights(projectId: projects.GitLabId, userId: persons.GitLabId, token: AccessToken): F[Boolean] +} +private class MemberRightsCheckerImpl[F[_]: Async: GitLabClient] extends MemberRightsChecker[F] { + + override def checkRights(projectId: projects.GitLabId, userId: persons.GitLabId, token: AccessToken): F[Boolean] = + GitLabClient[F].get(uri"projects" / projectId / "members" / "all" / userId, "single-project-member")( + mapMemberResponse + )(token.some) + + private lazy val mapMemberResponse: PartialFunction[(Status, Request[F], Response[F]), F[Boolean]] = { + case (Ok, _, resp) => resp.as[Boolean] case (Unauthorized | Forbidden | NotFound, _, _) => false.pure[F] } + + private implicit lazy val roleChecker: EntityDecoder[F, Boolean] = { + implicit val permissionsExists: Decoder[Boolean] = Decoder.instance { cur => + (cur.downField("access_level").as[Option[Int]] -> cur.downField("state").as[Option[String]]) + .mapN { + case (Some(role), Some("active")) => role >= 40 + case _ => false + } + } + jsonOf[F, Boolean] + } } diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokensCreator.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokensCreator.scala index c74c3f226f..73d0e817bf 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokensCreator.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/creation/TokensCreator.scala @@ -19,20 +19,20 @@ package io.renku.tokenrepository.repository package creation -import AccessTokenCrypto.EncryptedAccessToken -import ProjectsTokensDB.SessionResource import cats.MonadThrow import cats.data.OptionT import cats.effect.Async import cats.syntax.all._ -import deletion.TokenRemover import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import eu.timepit.refined.numeric.NonNegative -import fetching.PersistedTokensFinder import io.renku.graph.model.projects 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 +import io.renku.tokenrepository.repository.fetching.PersistedTokensFinder import io.renku.tokenrepository.repository.metrics.QueriesExecutionTimes import org.typelevel.log4cats.Logger @@ -68,47 +68,62 @@ private class TokensCreatorImpl[F[_]: MonadThrow: Logger]( override def create(projectId: projects.GitLabId, userToken: AccessToken): F[Unit] = findStoredToken(projectId) - .flatMapF(decrypt >=> validate >=> checkIfDue(projectId)) - .semiflatMap(replacePathIfChanged(projectId)) - .getOrElseF(createOrDelete(projectId, userToken)) - - private lazy val decrypt: EncryptedAccessToken => F[Option[ProjectAccessToken]] = encryptedToken => - accessTokenCrypto.decrypt(encryptedToken) map { - case token: ProjectAccessToken => token.some - case _ => Option.empty[ProjectAccessToken] - } + .flatMapF( + decrypt >=> removeWhenInvalid(projectId) >=> replacePathIfChangedOrRemove(projectId) >=> checkIfDue(projectId) + ) + .void + .getOrElseF(createNew(projectId, userToken)) - private lazy val validate: Option[ProjectAccessToken] => F[Option[ProjectAccessToken]] = { - case Some(token) => checkValid(token).map(Option.when(_)(token)) - case _ => Option.empty[ProjectAccessToken].pure[F] + private lazy val decrypt: EncryptedAccessToken => F[Option[AccessToken]] = encryptedToken => + accessTokenCrypto.decrypt(encryptedToken).map(_.some) + + private def removeWhenInvalid(projectId: projects.GitLabId): Option[AccessToken] => F[Option[AccessToken]] = { + case None => Option.empty[AccessToken].pure[F] + case Some(token) => + checkValid(projectId, token) >>= { + case true => token.some.pure[F] + case false => tokenRemover.delete(projectId).as(Option.empty) + } } - private def checkIfDue(projectId: projects.GitLabId): Option[ProjectAccessToken] => F[Option[ProjectAccessToken]] = { - case Some(token) => checkTokenDue(projectId).map(Option.unless(_)(token)) - case _ => Option.empty[ProjectAccessToken].pure[F] + private def replacePathIfChangedOrRemove( + projectId: projects.GitLabId + ): Option[AccessToken] => F[Option[AccessToken]] = { + case None => Option.empty[AccessToken].pure[F] + case Some(token) => + projectPathFinder + .findProjectPath(projectId, token) + .semiflatMap(actualPath => + findPersistedProjectPath(projectId).flatMap { + case `actualPath` => ().pure[F] + case _ => updatePath(Project(projectId, actualPath)) + } + ) + .cataF( + default = tokenRemover.delete(projectId).as(Option.empty), + _ => token.some.pure[F] + ) } - private def replacePathIfChanged(projectId: projects.GitLabId)(token: ProjectAccessToken): F[Unit] = - projectPathFinder - .findProjectPath(projectId, token) - .semiflatMap(actualPath => - findPersistedProjectPath(projectId).flatMap { - case `actualPath` => ().pure[F] - case _ => updatePath(Project(projectId, actualPath)) - } - ) - .getOrElseF(tokenRemover.delete(projectId)) + private def checkIfDue(projectId: projects.GitLabId): Option[AccessToken] => F[Option[AccessToken]] = { + case Some(token) => checkTokenDue(projectId).map(Option.unless(_)(token)) + case _ => Option.empty[AccessToken].pure[F] + } - private def createOrDelete(projectId: projects.GitLabId, userToken: AccessToken) = - (findProjectPath(projectId, userToken) >>= createNewToken(userToken)) - .semiflatMap(encrypt >=> persist >=> logSuccess >=> tryRevokingOldTokens(userToken)) - .getOrElseF(tokenRemover.delete(projectId)) + private def createNew(projectId: projects.GitLabId, userToken: AccessToken) = + tokenValidator.checkValid(projectId, userToken) >>= { + case false => ().pure[F] + case true => + (findProjectPath(projectId, userToken) >>= createNewToken(userToken)) + .semiflatMap(encrypt >=> persist >=> logSuccess >=> tryRevokingOldTokens(userToken)) + .getOrElse(()) + } private def findProjectPath(projectId: projects.GitLabId, userToken: AccessToken): OptionT[F, Project] = projectPathFinder.findProjectPath(projectId, userToken).map(Project(projectId, _)) private def createNewToken(userToken: AccessToken)(project: Project): OptionT[F, (Project, TokenCreationInfo)] = - createPersonalAccessToken(project.id, userToken).map(project -> _) + createProjectAccessToken(project.id, userToken).map(project -> _) private lazy val encrypt: ((Project, TokenCreationInfo)) => F[(Project, TokenCreationInfo, EncryptedAccessToken)] = { case (project, creationInfo) => diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/TokenRemover.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/TokenRemover.scala index 0099578078..9266bbabde 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/TokenRemover.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/deletion/TokenRemover.scala @@ -45,10 +45,10 @@ private class TokenRemoverImpl[F[_]: MonadCancelThrow: SessionResource: Logger: override def delete(projectId: GitLabId): F[Unit] = SessionResource[F].useK { - measureExecutionTime(query(projectId)) - } >> Logger[F].info(show"token removed for $projectId") + query(projectId) + } - private def query(projectId: GitLabId) = + private def query(projectId: GitLabId) = measureExecutionTime { SqlStatement .named("remove token") .command[GitLabId](sql"""DELETE FROM projects_tokens @@ -56,9 +56,11 @@ private class TokenRemoverImpl[F[_]: MonadCancelThrow: SessionResource: Logger: .arguments(projectId) .build .flatMapResult(failIfMultiUpdate(projectId)) + } private def failIfMultiUpdate(projectId: GitLabId): Completion => F[Unit] = { - case Completion.Delete(0 | 1) => ().pure[F] + case Completion.Delete(0) => ().pure[F] + case Completion.Delete(1) => Logger[F].info(show"token removed for $projectId") case Completion.Delete(n) => new RuntimeException(s"Deleting token for a projectId: $projectId removed $n records") .raiseError[F, Unit] diff --git a/token-repository/src/main/scala/io/renku/tokenrepository/repository/init/TokensMigrator.scala b/token-repository/src/main/scala/io/renku/tokenrepository/repository/init/TokensMigrator.scala index 3ee39b6832..2742019dbe 100644 --- a/token-repository/src/main/scala/io/renku/tokenrepository/repository/init/TokensMigrator.scala +++ b/token-repository/src/main/scala/io/renku/tokenrepository/repository/init/TokensMigrator.scala @@ -110,14 +110,14 @@ private class TokensMigrator[F[_]: Async: SessionResource: Logger: QueriesExecut .recoverWith { case _ => tokenRemover.delete(project.id) >> Option.empty[(Project, AccessToken)].pure[F] } private def deleteWhenInvalidWithRetry(project: Project, token: AccessToken): F[Option[(Project, AccessToken)]] = { - checkValid(token) >>= { + checkValid(project.id, token) >>= { case true => (project, token).some.pure[F] case false => tokenRemover.delete(project.id) >> Option.empty[(Project, AccessToken)].pure[F] } }.recoverWith(retry(deleteWhenInvalidWithRetry(project, token))(project)) private def createTokenWithRetry(project: Project, token: AccessToken): F[Option[(Project, TokenCreationInfo)]] = - createPersonalAccessToken(project.id, token) + createProjectAccessToken(project.id, token) .map(project -> _) .flatTapNone( Logger[F].warn(show"$logPrefix $project cannot generate new token; deleting") >> tokenRemover.delete(project.id) diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/AccessTokenCryptoSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/AccessTokenCryptoSpec.scala index 60753362fa..2d9944e8de 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/AccessTokenCryptoSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/AccessTokenCryptoSpec.scala @@ -20,24 +20,26 @@ package io.renku.tokenrepository.repository import cats.syntax.all._ import com.typesafe.config.ConfigFactory -import eu.timepit.refined.api.RefType +import io.renku.config.ConfigLoader.ConfigLoadingException import io.renku.crypto.AesCrypto.Secret import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.http.client.AccessToken +import io.renku.http.client.AccessToken.{PersonalAccessToken, UserOAuthAccessToken} +import io.renku.testtools.IOSpec import io.renku.tokenrepository.repository.AccessTokenCrypto.EncryptedAccessToken import org.scalatest.matchers.should import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.wordspec.AnyWordSpec +import scodec.bits.ByteVector import java.nio.charset.StandardCharsets.UTF_8 -import java.security.InvalidKeyException import java.util.Base64 import scala.jdk.CollectionConverters._ import scala.util.{Failure, Success, Try} -class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableDrivenPropertyChecks { +class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableDrivenPropertyChecks with IOSpec { "encrypt/decrypt" should { @@ -59,6 +61,23 @@ class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableD decryptException shouldBe an[Exception] decryptException.getMessage shouldBe "AccessToken decryption failed" } + + "decrypt existing values" in { + val usedSecret = Secret.unsafeFromBase64("YWJjZGVmZzEyMzQ1Njc4OQ==") + val tokenCrypto = new AccessTokenCryptoImpl[Try](usedSecret) + val token1 = PersonalAccessToken("lhcrr5dkerm69osrrujlubkkw0ysx8io0e") + val encToken1 = EncryptedAccessToken + .from("jmamuPCm4R0Rr/ZuVRrYffwBHWdJt8siwuVYJ7WVrLgDIR6RzcCJcpuuvCWcVnlPRsXi2wFHfM+OBOsbt2erZQ==") + .fold(throw _, identity) + + val token2 = UserOAuthAccessToken("q2f4t4ph7atcfh08eau1g35") + val encToken2 = EncryptedAccessToken + .from("QXK4EOJLgdqQdh8MLkS+vCtsJFU0wsnpD9QZuu5qyWROFCzFfUr81xv6f1mN1r3T") + .fold(throw _, identity) + + tokenCrypto.decrypt(encToken1) shouldBe Success(token1) + tokenCrypto.decrypt(encToken2) shouldBe Success(token2) + } } "apply" should { @@ -67,7 +86,7 @@ class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableD val config = ConfigFactory.parseMap( Map( "projects-tokens" -> Map( - "secret" -> aesCryptoSecrets.generateOne.value + "secret" -> aesCryptoSecrets.generateOne.toBase64 ).asJava ).asJava ) @@ -90,19 +109,15 @@ class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableD val Failure(exception) = AccessTokenCrypto[Try](config) - exception shouldBe a[InvalidKeyException] - exception.getMessage shouldBe "Invalid AES key length: 15 bytes" + exception shouldBe a[ConfigLoadingException] + exception.getMessage should include("Expected 16 bytes, but got 15") } } private trait TestCase { - private val secret = new String(Base64.getEncoder.encode("1234567890123456".getBytes(UTF_8)), UTF_8) - val hookTokenCrypto = new AccessTokenCryptoImpl[Try]( - RefType - .applyRef[Secret](secret) - .getOrElse(throw new IllegalArgumentException("Wrong secret")) - ) + private val secret = Secret.unsafe(ByteVector.view("1234567890123456".getBytes(UTF_8))) + val hookTokenCrypto = new AccessTokenCryptoImpl[Try](secret) } private lazy val tokenScenarios = Table( diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpointSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpointSpec.scala index 436c429a79..6256c114fc 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpointSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/CreateTokenEndpointSpec.scala @@ -27,12 +27,15 @@ import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.model.GraphModelGenerators._ +import io.renku.graph.model.projects import io.renku.graph.model.projects.GitLabId import io.renku.http.client.AccessToken import io.renku.http.server.EndpointTester._ +import io.renku.http.tinytypes.TinyTypeURIEncoder._ import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Error import io.renku.testtools.IOSpec +import org.http4s.Method.POST import org.http4s._ import org.http4s.headers.`Content-Type` import org.http4s.implicits._ @@ -46,13 +49,10 @@ class CreateTokenEndpointSpec extends AnyWordSpec with IOSpec with MockFactory w "respond with NO_CONTENT if the Personal Access Token association was successful" in new TestCase { - val accessToken: AccessToken = personalAccessTokens.generateOne - (tokensCreator - .create(_: GitLabId, _: AccessToken)) - .expects(projectId, accessToken) - .returning(IO.unit) + val accessToken = personalAccessTokens.generateOne + givenTokenCreation(projectId, accessToken, returning = ().pure[IO]) - val request = Request(Method.PUT, uri"projects" / projectId.toString / "tokens") + val request = Request(POST, uri"projects" / projectId / "tokens") .withEntity(accessToken.asJson) val response = endpoint.createToken(projectId, request).unsafeRunSync() @@ -65,14 +65,10 @@ class CreateTokenEndpointSpec extends AnyWordSpec with IOSpec with MockFactory w "respond with NO_CONTENT if the OAuth Access Token association was successful" in new TestCase { - val accessToken: AccessToken = userOAuthAccessTokens.generateOne + val accessToken = userOAuthAccessTokens.generateOne + givenTokenCreation(projectId, accessToken, returning = ().pure[IO]) - (tokensCreator - .create(_: GitLabId, _: AccessToken)) - .expects(projectId, accessToken) - .returning(IO.unit) - - val request = Request(Method.PUT, uri"projects" / projectId.toString / "tokens") + val request = Request(POST, uri"projects" / projectId / "tokens") .withEntity(accessToken.asJson) val response = endpoint.createToken(projectId, request).unsafeRunSync() @@ -85,7 +81,7 @@ class CreateTokenEndpointSpec extends AnyWordSpec with IOSpec with MockFactory w "respond with BAD_REQUEST if the request body is invalid" in new TestCase { - val request = Request[IO](Method.PUT, uri"projects" / projectId.toString / "tokens") + val request = Request[IO](POST, uri"projects" / projectId / "tokens") val response = endpoint.createToken(projectId, request).unsafeRunSync() @@ -100,15 +96,11 @@ class CreateTokenEndpointSpec extends AnyWordSpec with IOSpec with MockFactory w "respond with INTERNAL_SERVER_ERROR if Access Token association fails" in new TestCase { - val accessToken: AccessToken = personalAccessTokens.generateOne + val accessToken = personalAccessTokens.generateOne + val exception = exceptions.generateOne + givenTokenCreation(projectId, accessToken, returning = exception.raiseError[IO, Unit]) - val exception = exceptions.generateOne - (tokensCreator - .create(_: GitLabId, _: AccessToken)) - .expects(projectId, accessToken) - .returning(exception.raiseError[IO, Unit]) - - val request = Request(Method.PUT, uri"projects" / projectId.toString / "tokens") + val request = Request(POST, uri"projects" / projectId / "tokens") .withEntity(accessToken.asJson) val response = endpoint.createToken(projectId, request).unsafeRunSync() @@ -126,8 +118,21 @@ class CreateTokenEndpointSpec extends AnyWordSpec with IOSpec with MockFactory w val projectId = projectIds.generateOne - val tokensCreator = mock[TokensCreator[IO]] + private val tokensCreator = mock[TokensCreator[IO]] + private val tokenValidator = mock[TokenValidator[IO]] implicit val logger: TestLogger[IO] = TestLogger[IO]() val endpoint = new CreateTokenEndpointImpl[IO](tokensCreator) + + def givenTokenValidation(projectId: projects.GitLabId, accessToken: AccessToken, returning: IO[Boolean]) = + (tokenValidator + .checkValid(_: GitLabId, _: AccessToken)) + .expects(projectId, accessToken) + .returning(returning) + + def givenTokenCreation(projectId: projects.GitLabId, accessToken: AccessToken, returning: IO[Unit]) = + (tokensCreator + .create(_: GitLabId, _: AccessToken)) + .expects(projectId, accessToken) + .returning(returning) } } diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/NewTokensCreatorSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/NewTokensCreatorSpec.scala index 3df5f47880..6a361054af 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/NewTokensCreatorSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/NewTokensCreatorSpec.scala @@ -52,7 +52,7 @@ class NewTokensCreatorSpec with IOSpec with should.Matchers { - "createPersonalAccessToken" should { + "createProjectAccessToken" should { "do POST projects/:id/access_tokens with relevant payload" in new TestCase { @@ -70,7 +70,7 @@ class NewTokensCreatorSpec .expects(uri"projects" / projectId.value / "access_tokens", endpointName, payload, *, Option(accessToken)) .returning(creationInfo.some.pure[IO]) - tokensCreator.createPersonalAccessToken(projectId, accessToken).value.unsafeRunSync() shouldBe creationInfo.some + tokensCreator.createProjectAccessToken(projectId, accessToken).value.unsafeRunSync() shouldBe creationInfo.some } s"retrieve the created Project Access Token from the response with $Created status" in new TestCase { @@ -106,7 +106,7 @@ class NewTokensCreatorSpec val tokensCreator = new NewTokensCreatorImpl[IO](projectTokenTTL, renkuTokenName, currentDate) lazy val mapResponse = captureMapping(gitLabClient)( - findingMethod = tokensCreator.createPersonalAccessToken(projectId, accessToken).value.unsafeRunSync(), + findingMethod = tokensCreator.createProjectAccessToken(projectId, accessToken).value.unsafeRunSync(), resultGenerator = tokenCreationInfos.generateSome, method = POST ) diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokenValidatorSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokenValidatorSpec.scala index 9a3e1ed760..8a8f13f16f 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokenValidatorSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokenValidatorSpec.scala @@ -23,13 +23,17 @@ import cats.syntax.all._ import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import eu.timepit.refined.collection.NonEmpty +import io.circe.literal._ import io.renku.generators.CommonGraphGenerators.accessTokens import io.renku.generators.Generators.Implicits._ +import io.renku.graph.model.RenkuTinyTypeGenerators.{personGitLabIds, projectIds} +import io.renku.graph.model.{persons, projects} import io.renku.http.client.RestClient.ResponseMappingF import io.renku.http.client.{AccessToken, GitLabClient} +import io.renku.http.tinytypes.TinyTypeURIEncoder._ import io.renku.testtools.{GitLabClientTools, IOSpec} -import org.http4s.Method.HEAD import org.http4s.Status.{BadRequest, Forbidden, NotFound, Ok, Unauthorized} +import org.http4s.circe._ import org.http4s.implicits._ import org.http4s.{Request, Response, Uri} import org.scalacheck.Gen @@ -38,7 +42,70 @@ import org.scalatest.matchers.should import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.wordspec.AnyWordSpec -class TokenValidatorSpec +class TokenValidatorSpec extends AnyWordSpec with IOSpec with MockFactory with should.Matchers { + + "checkValid" should { + + "return true if the user the token belongs to " + + "is a member of the project with the given projectId " + + "and it has sufficient rights to it" in new TestCase { + + val accessToken = accessTokens.generateOne + val user = personGitLabIds.generateOne + givenFindingUser(accessToken, returning = user.some) + + val projectId = projectIds.generateOne + givenFindingRights(user, projectId, accessToken, returning = true) + + validator.checkValid(projectId, accessToken).unsafeRunSync() shouldBe true + } + + "return false if the user the token belongs to " + + "is a member of the project with the given projectId " + + "but it has insufficient rights to it" in new TestCase { + + val accessToken = accessTokens.generateOne + val user = personGitLabIds.generateOne + givenFindingUser(accessToken, returning = user.some) + + val projectId = projectIds.generateOne + givenFindingRights(user, project = projectId, accessToken, returning = false) + + validator.checkValid(projectId, accessToken).unsafeRunSync() shouldBe false + } + + "return false if the token is invalid" in new TestCase { + + val accessToken = accessTokens.generateOne + givenFindingUser(accessToken, returning = None) + + val projectId = projectIds.generateOne + + validator.checkValid(projectId, accessToken).unsafeRunSync() shouldBe false + } + } + + private trait TestCase { + private val userIdFinder = mock[UserIdFinder[IO]] + private val memberRightsChecker = mock[MemberRightsChecker[IO]] + val validator = new TokenValidatorImpl[IO](userIdFinder, memberRightsChecker) + + def givenFindingUser(accessToken: AccessToken, returning: Option[persons.GitLabId]) = + (userIdFinder.findUserId _) + .expects(accessToken) + .returning(returning.pure[IO]) + + def givenFindingRights(user: persons.GitLabId, + project: projects.GitLabId, + accessToken: AccessToken, + returning: Boolean + ) = (memberRightsChecker.checkRights _) + .expects(project, user, accessToken) + .returning(returning.pure[IO]) + } +} + +class UserIdFinderSpec extends AnyWordSpec with IOSpec with MockFactory @@ -48,50 +115,150 @@ class TokenValidatorSpec "checkValid" should { - "return true if HEAD /user call to the the GL returns 200" in new TestCase { + "return userId if returned" in new TestCase { val accessToken = accessTokens.generateOne + val maybeUserId = personGitLabIds.generateOption + givenFindingUser(accessToken, returning = maybeUserId) + + finder.findUserId(accessToken).unsafeRunSync() shouldBe maybeUserId + } + + forAll { + Table( + ("Case", "Response", "Expected Result"), + ("ok valid", Response[IO](Ok).withEntity(json"""{"id": ${persons.GitLabId(1)}}"""), Some(persons.GitLabId(1))), + ("ok invalid", Response[IO](Ok).withEntity(json"""{}"""), None), + ("unauthorized", Response[IO](Unauthorized), None), + ("forbidden", Response[IO](Forbidden), None), + ("notFound", Response[IO](NotFound), None) + ) + } { (caze, response, result) => + show"map user API call responding $caze to $result" in new TestCase { + responseMapping(response.status, Request[IO](), response).unsafeRunSync() shouldBe result + } + } + + "throw an Exception if remote responds with status different than OK, NOT_FOUND or UNAUTHORIZED" in new TestCase { + intercept[Exception] { + responseMapping(BadRequest, Request[IO](), Response[IO](BadRequest)).unsafeRunSync() + } + } + } - val endpointName: String Refined NonEmpty = "user" - val validationResult = results.generateOne + private trait TestCase { + implicit val gitLabClient: GitLabClient[IO] = mock[GitLabClient[IO]] + val finder = new UserIdFinderImpl[IO] + + private val userEndpointName: String Refined NonEmpty = "user" + + lazy val responseMapping = captureMapping(gitLabClient)( + findingMethod = finder.findUserId(accessTokens.generateOne).unsafeRunSync(), + resultGenerator = personGitLabIds.generateOption, + maybeEndpointName = userEndpointName.some + ) + + def givenFindingUser(accessToken: AccessToken, returning: Option[persons.GitLabId]) = (gitLabClient - .head(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[IO, Boolean])(_: Option[AccessToken])) - .expects(uri"user", endpointName, *, Option(accessToken)) - .returning(validationResult.pure[IO]) + .get(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[IO, Option[persons.GitLabId]])( + _: Option[AccessToken] + )) + .expects(uri"user", userEndpointName, *, accessToken.some) + .returning(returning.pure[IO]) + } +} - validator.checkValid(accessToken).unsafeRunSync() shouldBe validationResult +class MemberRightsCheckerSpec + extends AnyWordSpec + with IOSpec + with MockFactory + with GitLabClientTools[IO] + with TableDrivenPropertyChecks + with should.Matchers { + + "checkValid" should { + + "return boolean based on the response from the GET to GL's project member API" in new TestCase { + + val accessToken = accessTokens.generateOne + val userId = personGitLabIds.generateOne + val projectId = projectIds.generateOne + val result = results.generateOne + givenCheckingMemberRights(userId, projectId, accessToken, returning = result) + + rightsChecker.checkRights(projectId, userId, accessToken).unsafeRunSync() shouldBe result } forAll { Table( - "Status" -> "Expected Result", - Ok -> true, - Unauthorized -> false, - Forbidden -> false, - NotFound -> false + ("Case", "Response", "Expected Result"), + ("ok role 30 and active", + Response[IO](Ok).withEntity(json"""{"access_level": ${Role.Developer.value}, "state": "active"}"""), + false + ), + ("ok role 40 and active", + Response[IO](Ok).withEntity(json"""{"access_level": ${Role.Maintainer.value}, "state": "active"}"""), + true + ), + ("ok role 40 and non-active", + Response[IO](Ok).withEntity(json"""{"access_level": ${Role.Owner.value}, "state": "waiting"}"""), + false + ), + ("ok role 40 and no state", + Response[IO](Ok).withEntity(json"""{"access_level": ${Role.Owner.value}}"""), + false + ), + ("ok role 50 and active", + Response[IO](Ok).withEntity(json"""{"access_level": ${Role.Owner.value}, "state": "active"}"""), + true + ), + ("ok invalid", Response[IO](Ok).withEntity(json"""{}"""), false), + ("unauthorized", Response[IO](Unauthorized), false), + ("forbidden", Response[IO](Forbidden), false), + ("notFound", Response[IO](NotFound), false) ) - } { (status, result) => - s"map $status to true" in new TestCase { - mapResponse(status, Request[IO](), Response[IO](status)).unsafeRunSync() shouldBe result + } { (caze, response, result) => + show"map project member API call responding $caze to $result" in new TestCase { + responseMapping(response.status, Request[IO](), response).unsafeRunSync() shouldBe result } } "throw an Exception if remote responds with status different than OK, NOT_FOUND or UNAUTHORIZED" in new TestCase { intercept[Exception] { - mapResponse(BadRequest, Request[IO](), Response[IO](BadRequest)).unsafeRunSync() + responseMapping(BadRequest, Request[IO](), Response[IO](BadRequest)).unsafeRunSync() } } } + private sealed trait Role { val value: Int } + private object Role { + case object Developer extends Role { val value: Int = 30 } + case object Maintainer extends Role { val value: Int = 40 } + case object Owner extends Role { val value: Int = 50 } + } + private trait TestCase { implicit val gitLabClient: GitLabClient[IO] = mock[GitLabClient[IO]] - val validator = new TokenValidatorImpl[IO] + val rightsChecker = new MemberRightsCheckerImpl[IO] - lazy val mapResponse = captureMapping(gitLabClient)( - findingMethod = validator.checkValid(accessTokens.generateOne).unsafeRunSync(), + private val projectMemberEndpointName: String Refined NonEmpty = "single-project-member" + + lazy val responseMapping = captureMapping(gitLabClient)( + findingMethod = rightsChecker + .checkRights(projectIds.generateOne, personGitLabIds.generateOne, accessTokens.generateOne) + .unsafeRunSync(), resultGenerator = results.generateOne, - method = HEAD + maybeEndpointName = projectMemberEndpointName.some ) + + def givenCheckingMemberRights(user: persons.GitLabId, + project: projects.GitLabId, + accessToken: AccessToken, + returning: Boolean + ) = (gitLabClient + .get(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[IO, Boolean])(_: Option[AccessToken])) + .expects(uri"projects" / project / "members" / "all" / user, projectMemberEndpointName, *, accessToken.some) + .returning(returning.pure[IO]) } private lazy val results = Gen.oneOf(true, false) diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokensCreatorSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokensCreatorSpec.scala index 4c990d6f6c..b4112d3dbb 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokensCreatorSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/creation/TokensCreatorSpec.scala @@ -19,16 +19,11 @@ package io.renku.tokenrepository.repository package creation -import AccessTokenCrypto.EncryptedAccessToken -import Generators._ -import RepositoryGenerators._ import cats.data.OptionT import cats.syntax.all._ -import deletion.TokenRemover import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import eu.timepit.refined.numeric.NonNegative -import fetching.PersistedTokensFinder import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ @@ -39,159 +34,135 @@ import io.renku.http.client.AccessToken.ProjectAccessToken import io.renku.http.client.{AccessToken, UserAccessToken} import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Warn +import io.renku.tokenrepository.repository.AccessTokenCrypto.EncryptedAccessToken +import io.renku.tokenrepository.repository.RepositoryGenerators._ +import io.renku.tokenrepository.repository.creation.Generators._ +import io.renku.tokenrepository.repository.deletion.TokenRemover +import io.renku.tokenrepository.repository.fetching.PersistedTokensFinder import org.scalamock.scalatest.MockFactory +import org.scalatest.TryValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -import scala.util.{Failure, Try} +import scala.util.Try -class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers { +class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers with TryValues { "create" should { - "do nothing if Project Access Token from the DB is valid, is not due for refresh " + - "and project path has not changed" in new TestCase { + "do nothing if the stored Access Token is valid, is not due for refresh " + + "and the project path has not changed" in new TestCase { val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) - val projectAccessToken = projectAccessTokens.generateOne - givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) + val storedAccessToken = accessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = storedAccessToken.pure[Try]) - givenTokenValidation(of = projectAccessToken, returning = true.pure[Try]) + givenTokenValidation(of = storedAccessToken, returning = true.pure[Try]) - givenTokenDueCheck(projectId, returning = false.pure[Try]) + givenPathHasNotChanged(projectId, storedAccessToken) - givenPathHasNotChanged(projectId, projectAccessToken) + givenTokenDueCheck(projectId, returning = false.pure[Try]) tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "replace the stored token if it's not a Project Access Token" in new TestCase { + "replace the stored token when it's invalid" in new TestCase { val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some(encryptedToken)) - givenTokenDecryption(of = encryptedToken, returning = userAccessTokens.generateOne.pure[Try]) + val projectAccessToken = projectAccessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) + + givenTokenValidation(of = projectAccessToken, returning = false.pure[Try]) + givenTokenRemoval(projectId, returning = ().pure[Try]) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) - givenSuccessfulTokenCreation(projectPath) givenSuccessfulOldTokenRevoking(projectId, userAccessToken) tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "replace the stored token if it's invalid" in new TestCase { + "leave the stored Access Token untouched but update the path if " + + "the token is valid, it's not due for refresh " + + "but the path has changed" in new TestCase { - val encryptedToken = encryptedAccessTokens.generateOne - givenStoredTokenFinder(projectId, returning = OptionT.some(encryptedToken)) + val encryptedToken = encryptedAccessTokens.generateOne + givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) - val projectAccessToken = projectAccessTokens.generateOne - givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) + val projectAccessToken = projectAccessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) - givenTokenValidation(of = projectAccessToken, returning = false.pure[Try]) + givenTokenValidation(of = projectAccessToken, returning = true.pure[Try]) - val projectPath = projectPaths.generateOne - givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) + val newProjectPath = projectPaths.generateOne + givenPathFinder(projectId, projectAccessToken, OptionT.some[Try](newProjectPath)) + givenStoredPathFinder(projectId, returning = projectPaths.generateOne.pure[Try]) - givenSuccessfulTokenCreation(projectPath) - givenSuccessfulOldTokenRevoking(projectId, userAccessToken) + givenPathUpdate(Project(projectId, newProjectPath), returning = ().pure[Try]) - tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] - } + givenTokenDueCheck(projectId, returning = false.pure[Try]) + + tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] + } "replace the stored token if it's due for refresh" in new TestCase { val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some(encryptedToken)) - val projectAccessToken = projectAccessTokens.generateOne - givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) + val storedAccessToken = projectAccessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = storedAccessToken.pure[Try]) - givenTokenValidation(of = projectAccessToken, returning = true.pure[Try]) + givenTokenValidation(of = storedAccessToken, returning = true.pure[Try]) + + givenPathHasNotChanged(projectId, storedAccessToken) givenTokenDueCheck(projectId, returning = true.pure[Try]) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) - givenSuccessfulTokenCreation(projectPath) givenSuccessfulOldTokenRevoking(projectId, userAccessToken) tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "leave the stored Project Access Token untouched but update the path if " + - "the token is valid, it's not due for refresh " + - "but the path has not changed" in new TestCase { - - val encryptedToken = encryptedAccessTokens.generateOne - givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) - - val projectAccessToken = projectAccessTokens.generateOne - givenTokenDecryption(of = encryptedToken, returning = projectAccessToken.pure[Try]) - - givenTokenValidation(of = projectAccessToken, returning = true.pure[Try]) - - givenTokenDueCheck(projectId, returning = false.pure[Try]) - - val newProjectPath = projectPaths.generateOne - givenPathFinder(projectId, projectAccessToken, OptionT.some[Try](newProjectPath)) - givenStoredPathFinder(projectId, returning = projectPaths.generateOne.pure[Try]) - - givenPathUpdate(Project(projectId, newProjectPath), returning = ().pure[Try]) - - tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] - } - "store a new Project Access Token if there's none stored" in new TestCase { givenStoredTokenFinder(projectId, returning = OptionT.none) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) - givenSuccessfulTokenCreation(projectPath) givenSuccessfulOldTokenRevoking(projectId, userAccessToken) tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "remove the stored token (non Project Access Token) if project with the given id does not exist" in new TestCase { + "remove the stored token if project with the given id does not exist" in new TestCase { val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) - givenTokenDecryption(of = encryptedToken, returning = userAccessTokens.generateOne.pure[Try]) - - givenPathFinder(projectId, userAccessToken, returning = OptionT.none) - + val storedToken = userAccessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = storedToken.pure[Try]) + givenTokenValidation(storedToken, returning = false.pure[Try]) givenTokenRemoval(projectId, returning = ().pure[Try]) + givenTokenValidation(userAccessToken, returning = false.pure[Try]) + tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "delete token if stored token is valid and not due for refresh " + - "but current project path cannot be found in GL" in new TestCase { - - val encryptedToken = encryptedAccessTokens.generateOne - givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) - - val storedAccessToken = projectAccessTokens.generateOne - givenTokenDecryption(of = encryptedToken, returning = storedAccessToken.pure[Try]) - givenTokenValidation(of = storedAccessToken, returning = true.pure[Try]) - givenTokenDueCheck(projectId, returning = false.pure[Try]) - - givenPathFinder(projectId, storedAccessToken, returning = OptionT.none) - - givenTokenRemoval(projectId, returning = ().pure[Try]) - - tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] - } - - "delete token if new token creation failed with NotFound or Forbidden" in new TestCase { + "do nothing if new token creation failed with NotFound or Forbidden" in new TestCase { val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some[Try](encryptedToken)) @@ -199,22 +170,27 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher val storedAccessToken = projectAccessTokens.generateOne givenTokenDecryption(of = encryptedToken, returning = storedAccessToken.pure[Try]) - givenTokenValidation(of = storedAccessToken, returning = false.pure[Try]) + givenTokenValidation(of = storedAccessToken, returning = true.pure[Try]) + + givenPathHasNotChanged(projectId, storedAccessToken) + + givenTokenDueCheck(projectId, returning = true.pure[Try]) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) val newProjectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(newProjectPath)) givenProjectTokenCreator(projectId, userAccessToken, returning = OptionT.none) - givenTokenRemoval(projectId, returning = ().pure[Try]) - tokensCreator.create(projectId, userAccessToken) shouldBe ().pure[Try] } - "retry if the token after storing couldn't be decrypted" in new TestCase { + "retry if the token couldn't be decrypted after storing (token sanity check failed)" in new TestCase { givenStoredTokenFinder(projectId, returning = OptionT.none) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) + val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) @@ -246,6 +222,8 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher givenStoredTokenFinder(projectId, returning = OptionT.none) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) + val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) @@ -282,6 +260,8 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher givenStoredTokenFinder(projectId, returning = OptionT.none) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) + val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) @@ -299,9 +279,11 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher givenStoredTokenFinder(projectId, returning = OptionT.none).repeated(3) - val Failure(exception) = tokensCreator.create(projectId, userAccessToken) - - exception.getMessage shouldBe show"Token associator - just saved token cannot be found for project: $projectId" + tokensCreator + .create(projectId, userAccessToken) + .failure + .exception + .getMessage shouldBe show"Token associator - just saved token cannot be found for project: $projectId" } "log a warning and succeed when token revoking fails" in new TestCase { @@ -309,11 +291,15 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher val encryptedToken = encryptedAccessTokens.generateOne givenStoredTokenFinder(projectId, returning = OptionT.some(encryptedToken)) - givenTokenDecryption(of = encryptedToken, returning = userAccessTokens.generateOne.pure[Try]) + val storedToken = accessTokens.generateOne + givenTokenDecryption(of = encryptedToken, returning = storedToken.pure[Try]) + + givenTokenValidation(storedToken, returning = false.pure[Try]) + givenTokenRemoval(projectId, returning = ().pure[Try]) + givenTokenValidation(userAccessToken, returning = true.pure[Try]) val projectPath = projectPaths.generateOne givenPathFinder(projectId, userAccessToken, returning = OptionT.some(projectPath)) - givenSuccessfulTokenCreation(projectPath) val exception = exceptions.generateOne @@ -377,9 +363,9 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher .expects(projectAccessToken) .returning(returning) - def givenTokenValidation(of: ProjectAccessToken, returning: Try[Boolean]) = + def givenTokenValidation(of: AccessToken, returning: Try[Boolean]) = (tokenValidator.checkValid _) - .expects(of) + .expects(projectId, of) .returning(returning) def givenTokenDueCheck(projectId: projects.GitLabId, returning: Try[Boolean]) = @@ -409,7 +395,7 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher def givenProjectTokenCreator(projectId: projects.GitLabId, userAccessToken: UserAccessToken, returning: OptionT[Try, TokenCreationInfo] - ) = (newTokensCreator.createPersonalAccessToken _) + ) = (newTokensCreator.createProjectAccessToken _) .expects(projectId, userAccessToken) .returning(returning) @@ -440,6 +426,7 @@ class TokensCreatorSpec extends AnyWordSpec with MockFactory with should.Matcher } def givenSuccessfulTokenCreation(projectPath: projects.Path) = { + val tokenCreationInfo = tokenCreationInfos.generateOne givenProjectTokenCreator(projectId, userAccessToken, returning = OptionT.some(tokenCreationInfo)) diff --git a/token-repository/src/test/scala/io/renku/tokenrepository/repository/init/TokensMigratorSpec.scala b/token-repository/src/test/scala/io/renku/tokenrepository/repository/init/TokensMigratorSpec.scala index 1b11d595eb..4aaa8071d8 100644 --- a/token-repository/src/test/scala/io/renku/tokenrepository/repository/init/TokensMigratorSpec.scala +++ b/token-repository/src/test/scala/io/renku/tokenrepository/repository/init/TokensMigratorSpec.scala @@ -110,7 +110,7 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val oldToken = accessTokens.generateOne givenDecryption(oldTokenEncrypted, returning = oldToken.pure[IO]) - givenTokenValidation(oldToken, returning = false.pure[IO]) + givenTokenValidation(oldTokenProject.id, oldToken, returning = false.pure[IO]) migration.run.unsafeRunSync() shouldBe () @@ -127,7 +127,7 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val oldToken = accessTokens.generateOne givenDecryption(oldTokenEncrypted, returning = oldToken.pure[IO]) - givenTokenValidation(oldToken, returning = true.pure[IO]) + givenTokenValidation(oldTokenProject.id, oldToken, returning = true.pure[IO]) givenProjectTokenCreator(oldTokenProject.id, oldToken, returning = OptionT.none) @@ -157,8 +157,8 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh givenDecryption(oldTokenEncrypted, returning = oldToken.pure[IO]) val exception = exceptions.generateOne - givenTokenValidation(oldToken, returning = exception.raiseError[IO, Boolean]) - givenTokenValidation(oldToken, returning = true.pure[IO]) + givenTokenValidation(oldTokenProject.id, oldToken, returning = exception.raiseError[IO, Boolean]) + givenTokenValidation(oldTokenProject.id, oldToken, returning = true.pure[IO]) val projectToken = projectAccessTokens.generateOne val creationInfo = @@ -188,7 +188,7 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val oldToken = accessTokens.generateOne givenDecryption(oldTokenEncrypted, returning = oldToken.pure[IO]) - givenTokenValidation(oldToken, returning = true.pure[IO]) + givenTokenValidation(oldTokenProject.id, oldToken, returning = true.pure[IO]) val exception = exceptions.generateOne givenProjectTokenCreator(oldTokenProject.id, @@ -226,11 +226,11 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val oldTokenEncrypted = encryptedAccessTokens.generateOne implicit val logger: TestLogger[IO] = TestLogger[IO]() - val tokenCrypto = mock[AccessTokenCrypto[IO]] - val tokenValidator = mock[TokenValidator[IO]] - val tokenRemover = TokenRemover[IO] - val tokensCreator = mock[NewTokensCreator[IO]] - val tokensPersister = TokensPersister[IO] + private val tokenCrypto = mock[AccessTokenCrypto[IO]] + private val tokenValidator = mock[TokenValidator[IO]] + private val tokenRemover = TokenRemover[IO] + private val tokensCreator = mock[NewTokensCreator[IO]] + private val tokensPersister = TokensPersister[IO] val migration = new TokensMigrator[IO](tokenCrypto, tokenValidator, tokenRemover, @@ -256,8 +256,7 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val command: Command[projects.GitLabId ~ projects.Path ~ EncryptedAccessToken] = sql""" INSERT INTO projects_tokens (project_id, project_path, token) - VALUES ($projectIdEncoder, $projectPathEncoder, $encryptedAccessTokenEncoder) - """.command + VALUES ($projectIdEncoder, $projectPathEncoder, $encryptedAccessTokenEncoder)""".command session .prepare(command) .flatMap(_.execute(project.id ~ project.path ~ encryptedToken)) @@ -272,7 +271,7 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh val oldToken = accessTokens.generateOne givenDecryption(oldTokenEnc, returning = oldToken.pure[IO]) - givenTokenValidation(oldToken, returning = true.pure[IO]) + givenTokenValidation(project.id, oldToken, returning = true.pure[IO]) val projectToken = projectAccessTokens.generateOne val creationInfo = TokenCreationInfo( @@ -299,15 +298,15 @@ class TokensMigratorSpec extends AnyWordSpec with IOSpec with DbInitSpec with sh .expects(token) .returning(returning) - def givenTokenValidation(token: AccessToken, returning: IO[Boolean]) = + def givenTokenValidation(projectId: projects.GitLabId, token: AccessToken, returning: IO[Boolean]) = (tokenValidator.checkValid _) - .expects(token) + .expects(projectId, token) .returning(returning) def givenProjectTokenCreator(projectId: projects.GitLabId, userAccessToken: AccessToken, returning: OptionT[IO, TokenCreationInfo] - ) = (tokensCreator.createPersonalAccessToken _) + ) = (tokensCreator.createProjectAccessToken _) .expects(projectId, userAccessToken) .returning(returning) } diff --git a/webhook-service/README.md b/webhook-service/README.md index 0fc15f61b2..3f9e55860f 100644 --- a/webhook-service/README.md +++ b/webhook-service/README.md @@ -53,11 +53,12 @@ The headers are not required. **Response** -| Status | Description | -|-----------------------------|--------------------------------------------------------| -| OK (200) | When there is a hook for the project | -| NOT_FOUND (404) | When project (or a valid access token) cannot be found | -| INTERNAL SERVER ERROR (500) | When there are problems with finding the status | +| Status | Description | +|-----------------------------|----------------------------------------------------------------------| +| OK (200) | When there is a hook for the project | +| UNAUTHORIZED (401) | When the given `private-token` or `authorization: bearer` is invalid | +| NOT_FOUND (404) | When project (or a valid access token) cannot be found | +| INTERNAL SERVER ERROR (500) | When there are problems with finding the status | Response examples: - project not activated @@ -81,15 +82,16 @@ Response examples: "percentage": 40.00 }, "details": { - "status": "in-progress|success|failure", - "message": "generating triples" + "status": "in-progress|success|failure", + "message": "generating triples", + "stacktrace": "some stack trace" // optional } } ``` #### POST /projects/:id/webhooks -creates a webhook for a project with the given `project id`. +Creates a webhook for a project with the given `project id`. **request format** @@ -104,6 +106,7 @@ The endpoint requires an authorization token passed in the request header as: | OK (200) | when hook already exists for the project | | CREATED (201) | when a new hook was created | | UNAUTHORIZED (401) | when there is neither `private-token` nor `authorization: bearer` in the header or it's invalid | +| NOT_FOUND (404) | when project does not exists or user has not access to it | | INTERNAL_SERVER_ERROR (500) | when there are problems with webhook creation | #### DELETE /projects/:id/webhooks @@ -121,7 +124,7 @@ The endpoint requires an authorization token passed in the request header as: | status | description | |-----------------------------|-------------------------------------------------------------------------------------------------| | OK (200) | when hook is successfully deleted | -| NOT_FOUND (404) | when the project does not exists | +| NOT_FOUND (404) | when the project or its hook does not exists | | UNAUTHORIZED (401) | when there is neither `private-token` nor `authorization: bearer` in the header or it's invalid | | INTERNAL_SERVER_ERROR (500) | when there are problems with webhook creation | @@ -140,12 +143,12 @@ The endpoint requires an authorization token passed in the request header as: **Response** -| Status | Description | -|----------------------------|-------------------------------------------------------------------------------------------------| -| OK (200) | When the hook exists for the project | -| NOT_FOUND (404) | When there's no hook for the project | -| UNAUTHORIZED (401) | When there is neither `PRIVATE-TOKEN` nor `AUTHORIZATION: BEARER` in the header or it's invalid | -| INTERNAL SERVER ERROR (500)| When there are problems with validating the hook presence | +| Status | Description | +|----------------------------|------------------------------------------------------------------| +| OK (200) | When the hook exists for the project | +| NOT_FOUND (404) | When there's no hook for the project or the hook cannot be found | +| UNAUTHORIZED (401) | When using does not have access to the project | +| INTERNAL SERVER ERROR (500)| When there are problems with validating the hook presence | #### GET /version diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/MicroserviceRoutes.scala b/webhook-service/src/main/scala/io/renku/webhookservice/MicroserviceRoutes.scala index c6d9684a1c..befe1466bb 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/MicroserviceRoutes.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/MicroserviceRoutes.scala @@ -18,7 +18,7 @@ package io.renku.webhookservice -import cats.MonadThrow +import cats.{MonadThrow, NonEmptyParallel} import cats.effect.{Async, Resource} import cats.syntax.all._ import io.renku.graph.http.server.binders.ProjectId @@ -84,7 +84,8 @@ private class MicroserviceRoutes[F[_]: MonadThrow]( } private object MicroserviceRoutes { - def apply[F[_]: Async: GitLabClient: Logger: MetricsRegistry: ExecutionTimeRecorder]: F[MicroserviceRoutes[F]] = for { + def apply[F[_]: Async: NonEmptyParallel: GitLabClient: Logger: MetricsRegistry: ExecutionTimeRecorder] + : F[MicroserviceRoutes[F]] = for { projectHookUrl <- ProjectHookUrl.fromConfig[F]() hookTokenCrypto <- HookTokenCrypto[F]() webhookEventsEndpoint <- webhookevents.Endpoint(hookTokenCrypto) diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/crypto/HookTokenCrypto.scala b/webhook-service/src/main/scala/io/renku/webhookservice/crypto/HookTokenCrypto.scala index 94f32349ba..3ff8db208a 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/crypto/HookTokenCrypto.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/crypto/HookTokenCrypto.scala @@ -23,7 +23,6 @@ import cats.syntax.all._ import com.typesafe.config.{Config, ConfigFactory} import eu.timepit.refined.W import eu.timepit.refined.api.{RefType, Refined} -import eu.timepit.refined.pureconfig._ import eu.timepit.refined.string.MatchesRegex import io.circe.parser._ import io.circe.{Decoder, HCursor, Json} diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/Endpoint.scala b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/Endpoint.scala index 33a012d2a9..3af160dd34 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/Endpoint.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/Endpoint.scala @@ -19,10 +19,9 @@ package io.renku.webhookservice package eventstatus -import cats.MonadThrow -import cats.data.EitherT import cats.effect._ import cats.syntax.all._ +import cats.{MonadThrow, NonEmptyParallel} import io.circe.syntax._ import io.renku.graph.eventlog import io.renku.graph.eventlog.api.events.CommitSyncRequest @@ -36,7 +35,7 @@ import io.renku.logging.ExecutionTimeRecorder import io.renku.metrics.MetricsRegistry import io.renku.webhookservice.hookvalidation import io.renku.webhookservice.hookvalidation.HookValidator -import io.renku.webhookservice.hookvalidation.HookValidator.{HookValidationResult, NoAccessTokenException} +import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult import io.renku.webhookservice.model.ProjectHookUrl import org.http4s.Response import org.http4s.circe._ @@ -47,7 +46,7 @@ trait Endpoint[F[_]] { def fetchProcessingStatus(projectId: GitLabId, authUser: Option[AuthUser]): F[Response[F]] } -private class EndpointImpl[F[_]: MonadThrow: Logger: ExecutionTimeRecorder]( +private class EndpointImpl[F[_]: MonadThrow: NonEmptyParallel: Logger: ExecutionTimeRecorder]( hookValidator: HookValidator[F], statusInfoFinder: StatusInfoFinder[F], projectInfoFinder: ProjectInfoFinder[F], @@ -56,44 +55,27 @@ private class EndpointImpl[F[_]: MonadThrow: Logger: ExecutionTimeRecorder]( with Endpoint[F] { import HookValidationResult._ + import hookValidator.validateHook import projectInfoFinder.findProjectInfo import statusInfoFinder.findStatusInfo private val executionTimeRecorder = ExecutionTimeRecorder[F] import executionTimeRecorder._ def fetchProcessingStatus(projectId: GitLabId, authUser: Option[AuthUser]): F[Response[F]] = measureExecutionTime { - validateHook(projectId, authUser) - .semiflatMap { - case HookMissing => - Ok(StatusInfo.NotActivated.asJson) - case HookExists => - findStatusInfo(projectId) - .flatTap(sendCommitSyncIfNone(projectId, authUser)) - .map(_.getOrElse(StatusInfo.webhookReady.widen)) - .flatMap(si => Ok(si.asJson)) + (validateHook(projectId, authUser.map(_.accessToken)) -> findStatusInfo(projectId)) + .parFlatMapN { + case (Some(HookMissing), _) => Ok(StatusInfo.NotActivated.asJson) + case (Some(HookExists), Some(si)) => Ok(si.asJson) + case (Some(HookExists), None) => sendCommitSync(projectId, authUser) >> Ok(StatusInfo.webhookReady.widen.asJson) + case (None, Some(si)) => Ok(si.asJson) + case (None, None) => NotFound(InfoMessage("Info about project cannot be found")) } - .merge .recoverWith(internalServerError(projectId)) } map logExecutionTime(withMessage = show"Finding status info for project '$projectId' finished") - private def validateHook(projectId: GitLabId, authUser: Option[AuthUser]) = - EitherT { - hookValidator - .validateHook(projectId, authUser.map(_.accessToken)) - .map(_.asRight[Response[F]]) - .recoverWith(noAccessTokenToNotFound) - } - - private lazy val noAccessTokenToNotFound: PartialFunction[Throwable, F[Either[Response[F], HookValidationResult]]] = { - case _: NoAccessTokenException => NotFound(InfoMessage("Info about project cannot be found")).map(_.asLeft) - } - - private def sendCommitSyncIfNone(projectId: GitLabId, authUser: Option[AuthUser]): Option[StatusInfo] => F[Unit] = { - case Some(_) => ().pure[F] - case None => - findProjectInfo(projectId)(authUser.map(_.accessToken)).map(CommitSyncRequest(_)) >>= - elClient.send - } + private def sendCommitSync(projectId: GitLabId, authUser: Option[AuthUser]): F[Unit] = + findProjectInfo(projectId)(authUser.map(_.accessToken)).map(CommitSyncRequest(_)) >>= + elClient.send private def internalServerError(projectId: projects.GitLabId): PartialFunction[Throwable, F[Response[F]]] = { case exception => @@ -103,7 +85,7 @@ private class EndpointImpl[F[_]: MonadThrow: Logger: ExecutionTimeRecorder]( } object Endpoint { - def apply[F[_]: Async: GitLabClient: ExecutionTimeRecorder: Logger: MetricsRegistry]( + def apply[F[_]: Async: NonEmptyParallel: GitLabClient: ExecutionTimeRecorder: Logger: MetricsRegistry]( projectHookUrl: ProjectHookUrl ): F[Endpoint[F]] = for { hookValidator <- hookvalidation.HookValidator(projectHookUrl) diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfo.scala b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfo.scala index b99e90873a..1a292cf698 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfo.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfo.scala @@ -19,12 +19,12 @@ package io.renku.webhookservice.eventstatus import cats.syntax.all._ -import io.circe.{Encoder, Json} import io.circe.literal._ -import io.renku.graph.model.events.{EventStatus, EventStatusProgress} -import EventStatus._ import io.circe.syntax.EncoderOps +import io.circe.{Encoder, Json} +import io.renku.graph.model.events.EventStatus._ import io.renku.graph.model.events.EventStatusProgress.Stage +import io.renku.graph.model.events.{EventInfo, EventStatus, EventStatusProgress} private sealed trait StatusInfo extends Product { def activated: Boolean @@ -42,11 +42,11 @@ private object StatusInfo { def fold[A](whenActivated: StatusInfo.ActivatedProject => A, notActivated: => A): A = whenActivated(this) } - def activated(eventStatus: EventStatus): StatusInfo.ActivatedProject = - ActivatedProject(Progress.from(eventStatus), Details.fromStatus(eventStatus)) + def activated(event: EventInfo): StatusInfo.ActivatedProject = + ActivatedProject(Progress.from(event.status), Details.from(event)) def webhookReady: StatusInfo.ActivatedProject = - ActivatedProject(Progress.Zero, Details("in-progress", "Webhook has been installed.")) + ActivatedProject(Progress.Zero, Details("in-progress", "Webhook has been installed.", maybeDetails = None)) final case object NotActivated extends StatusInfo { override val activated: Boolean = false @@ -99,11 +99,13 @@ private object Progress { } } -private final case class Details(status: String, message: String) +private final case class Details(status: String, message: String, maybeDetails: Option[String]) private object Details { - def fromStatus(eventStatus: EventStatus): Details = { - val status: String = eventStatus match { + + def from(event: EventInfo): Details = { + + val status: String = event.status match { case New | GeneratingTriples | GenerationRecoverableFailure | TriplesGenerated | TransformingTriples | TransformationRecoverableFailure | AwaitingDeletion | Deleting => "in-progress" @@ -111,10 +113,15 @@ private object Details { case GenerationNonRecoverableFailure | TransformationNonRecoverableFailure => "failure" } - val message: String = eventStatus.show.toLowerCase.replace('_', ' ') - Details(status, message) + val message: String = event.status.show.toLowerCase.replace('_', ' ') + + Details(status, message, event.maybeMessage.map(_.value)) } implicit val jsonEncoder: Encoder[Details] = - Encoder.instance(d => Json.obj("status" -> d.status.asJson, "message" -> d.message.asJson)) + Encoder.instance { d => + Json + .obj("status" -> d.status.asJson, "message" -> d.message.asJson, "stacktrace" -> d.maybeDetails.asJson) + .deepDropNullValues + } } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfoFinder.scala b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfoFinder.scala index aefae65020..dc0e9019e8 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfoFinder.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/eventstatus/StatusInfoFinder.scala @@ -53,6 +53,6 @@ private class StatusInfoFinderImpl[F[_]: MonadThrow](eventLogClient: EventLogCli private def toStatusInfo(events: List[EventInfo]): Option[StatusInfo] = events match { case Nil => None - case event :: _ => StatusInfo.activated(event.status).some + case event :: _ => StatusInfo.activated(event).some } } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreationEndpoint.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreationEndpoint.scala index 694b3a2477..7532f5591b 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreationEndpoint.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreationEndpoint.scala @@ -49,15 +49,13 @@ class HookCreationEndpointImpl[F[_]: MonadThrow: Logger]( with HookCreationEndpoint[F] { def createHook(projectId: GitLabId, authUser: AuthUser): F[Response[F]] = { - for { - creationResult <- hookCreator.createHook(projectId, authUser.accessToken) - response <- toHttpResponse(creationResult) - } yield response + hookCreator.createHook(projectId, authUser) >>= toHttpResponse } recoverWith httpResponse - private lazy val toHttpResponse: CreationResult => F[Response[F]] = { - case HookCreated => Created(InfoMessage("Hook created")) - case HookExisted => Ok(InfoMessage("Hook already existed")) + private lazy val toHttpResponse: Option[CreationResult] => F[Response[F]] = { + case Some(HookCreated) => Created(InfoMessage("Hook created")) + case Some(HookExisted) => Ok(InfoMessage("Hook already existed")) + case None => NotFound(InfoMessage("Project not found")) } private lazy val httpResponse: PartialFunction[Throwable, F[Response[F]]] = { diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreator.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreator.scala index 5e7edbe68f..ad94e1ea5c 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreator.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/HookCreator.scala @@ -26,6 +26,7 @@ import io.renku.graph.eventlog.api.events.CommitSyncRequest import io.renku.graph.model.projects import io.renku.graph.model.projects.GitLabId import io.renku.http.client.{AccessToken, GitLabClient} +import io.renku.http.server.security.model.AuthUser import io.renku.metrics.MetricsRegistry import io.renku.webhookservice.crypto.HookTokenCrypto import io.renku.webhookservice.hookcreation.HookCreator.CreationResult @@ -38,48 +39,53 @@ import io.renku.webhookservice.{ProjectInfoFinder, hookvalidation} import org.typelevel.log4cats.Logger private trait HookCreator[F[_]] { - def createHook(projectId: GitLabId, accessToken: AccessToken): F[CreationResult] + def createHook(projectId: GitLabId, authUser: AuthUser): F[Option[CreationResult]] } private class HookCreatorImpl[F[_]: Spawn: Logger]( - projectHookUrl: ProjectHookUrl, - projectHookValidator: HookValidator[F], - projectInfoFinder: ProjectInfoFinder[F], - hookTokenCrypto: HookTokenCrypto[F], - projectHookCreator: ProjectHookCreator[F], - accessTokenAssociator: AccessTokenAssociator[F], - elClient: eventlog.api.events.Client[F] + projectHookUrl: ProjectHookUrl, + projectHookValidator: HookValidator[F], + tokenAssociator: AccessTokenAssociator[F], + projectInfoFinder: ProjectInfoFinder[F], + hookTokenCrypto: HookTokenCrypto[F], + projectHookCreator: ProjectHookCreator[F], + elClient: eventlog.api.events.Client[F] ) extends HookCreator[F] { import HookCreator.CreationResult._ - import accessTokenAssociator._ import hookTokenCrypto._ import projectHookCreator.create - import projectHookValidator._ + import projectHookValidator.validateHook import projectInfoFinder.findProjectInfo - override def createHook(projectId: GitLabId, accessToken: AccessToken): F[CreationResult] = { + override def createHook(projectId: GitLabId, authUser: AuthUser): F[Option[CreationResult]] = { - def createIfMissing(hvr: HookValidationResult): F[CreationResult] = - hvr match { - case HookValidationResult.HookMissing => + val accessToken = authUser.accessToken + + val createIfMissing: HookValidationResult => F[CreationResult] = { + case HookValidationResult.HookMissing => + tokenAssociator.associate(projectId, accessToken) >> + encrypt(HookToken(projectId)) + .flatMap(t => create(ProjectHook(projectId, projectHookUrl, t), accessToken)) + .as(HookCreated.widen) + case HookValidationResult.HookExists => + HookExisted.widen.pure[F] + } + + validateHook(projectId, accessToken.some) + .flatMap { + case Some(validationResult) => for { - _ <- associate(projectId, accessToken) - serializedHookToken <- encrypt(HookToken(projectId)) - _ <- create(ProjectHook(projectId, projectHookUrl, serializedHookToken), accessToken) - } yield HookCreated - case HookValidationResult.HookExists => - associate(projectId, accessToken).map(_ => HookExisted.widen) + creationResult <- createIfMissing(validationResult) + _ <- Logger[F].info(show"Hook $creationResult for projectId $projectId") + _ <- Spawn[F].start(sendCommitSyncReq(projectId, accessToken)) + } yield creationResult.some + case None => + Logger[F] + .info(show"Hook on projectId $projectId cannot be created by user ${authUser.id}") + .as(Option.empty[CreationResult]) } - - { - for { - validationResult <- validateHook(projectId, accessToken.some) - creationResult <- createIfMissing(validationResult) - _ <- Logger[F].info(show"Hook $creationResult for projectId $projectId") - _ <- Spawn[F].start(sendCommitSyncReq(projectId, accessToken)) - } yield creationResult - }.onError(loggingError(projectId)) + .onError(loggingError(projectId)) } private def sendCommitSyncReq(projectId: projects.GitLabId, accessToken: AccessToken) = @@ -116,17 +122,17 @@ private object HookCreator { hookTokenCrypto: HookTokenCrypto[F] ): F[HookCreator[F]] = for { hookValidator <- hookvalidation.HookValidator(projectHookUrl) + tokenAssociator <- AccessTokenAssociator[F] projectInfoFinder <- ProjectInfoFinder[F] hookCreator <- ProjectHookCreator[F] - tokenAssociator <- AccessTokenAssociator[F] elClient <- eventlog.api.events.Client[F] } yield new HookCreatorImpl[F]( projectHookUrl, hookValidator, + tokenAssociator, projectInfoFinder, hookTokenCrypto, hookCreator, - tokenAssociator, elClient ) } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/ProjectHookCreator.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/ProjectHookCreator.scala index 7b648945ee..b240d29498 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/ProjectHookCreator.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookcreation/ProjectHookCreator.scala @@ -36,12 +36,13 @@ private class ProjectHookCreatorImpl[F[_]: Async: GitLabClient: Logger] extends import io.circe.Json import io.renku.http.client.RestClientError.UnauthorizedException + import io.renku.http.tinytypes.TinyTypeURIEncoder._ import org.http4s.Status.{Created, Unauthorized, UnprocessableEntity} import org.http4s.implicits._ import org.http4s.{Request, Response, Status} def create(projectHook: ProjectHook, accessToken: AccessToken): F[Unit] = { - val uri = uri"projects" / projectHook.projectId.show / "hooks" + val uri = uri"projects" / projectHook.projectId / "hooks" GitLabClient[F].post(uri, "create-hook", payload(projectHook))(mapResponse(projectHook))(Some(accessToken)) } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletor.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/GLHookRemover.scala similarity index 55% rename from webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletor.scala rename to webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/GLHookRemover.scala index ddaf0dcef8..bbb069931c 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletor.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/GLHookRemover.scala @@ -22,51 +22,39 @@ import cats.effect.Async import cats.syntax.all._ import eu.timepit.refined.auto._ import io.renku.graph.model.projects -import io.renku.graph.model.projects.GitLabId import io.renku.http.client.{AccessToken, GitLabClient} -import io.renku.webhookservice.crypto.HookTokenCrypto.SerializedHookToken -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult import io.renku.webhookservice.hookfetcher.ProjectHookFetcher.HookIdAndUrl -import io.renku.webhookservice.model.ProjectHookUrl import org.http4s.Status -import org.http4s.Status.{NoContent, NotFound, Ok} +import org.http4s.Status.{Forbidden, NoContent, NotFound, Ok} import org.http4s.implicits.http4sLiteralsSyntax import org.typelevel.log4cats.Logger -private trait ProjectHookDeletor[F[_]] { - def delete( - projectId: projects.GitLabId, - projectHookId: HookIdAndUrl, - accessToken: AccessToken - ): F[DeletionResult] +private trait GLHookRemover[F[_]] { + def delete(projectId: projects.GitLabId, projectHookId: HookIdAndUrl, accessToken: AccessToken): F[DeletionResult] } -private class ProjectHookDeletorImpl[F[_]: Async: GitLabClient: Logger] extends ProjectHookDeletor[F] { +private class GLHookRemoverImpl[F[_]: Async: GitLabClient: Logger] extends GLHookRemover[F] { import cats.effect._ import io.renku.http.client.RestClientError.UnauthorizedException + import io.renku.http.tinytypes.TinyTypeURIEncoder._ import org.http4s.Status.Unauthorized import org.http4s.{Request, Response} - private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[DeletionResult]] = { - case (Ok | NoContent, _, _) => DeletionResult.HookDeleted.pure[F].widen[DeletionResult] - case (NotFound, _, _) => DeletionResult.HookNotFound.pure[F].widen[DeletionResult] - case (Unauthorized, _, _) => MonadCancelThrow[F].raiseError(UnauthorizedException) - } - def delete(projectId: projects.GitLabId, projectHookId: HookIdAndUrl, accessToken: AccessToken): F[DeletionResult] = - GitLabClient[F].delete(uri"projects" / projectId.show / "hooks" / projectHookId.id.show, "delete-hook")( + GitLabClient[F].delete(uri"projects" / projectId / "hooks" / projectHookId.id, "delete-hook")( mapResponse )(accessToken.some) -} -private object ProjectHookDeletor { - def apply[F[_]: Async: GitLabClient: Logger]: F[ProjectHookDeletor[F]] = - new ProjectHookDeletorImpl[F].pure[F].widen[ProjectHookDeletor[F]] + private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[DeletionResult]] = { + case (Ok | NoContent, _, _) => DeletionResult.HookDeleted.pure[F].widen[DeletionResult] + case (NotFound, _, _) => DeletionResult.HookNotFound.pure[F].widen[DeletionResult] + case (Unauthorized | Forbidden, _, _) => MonadCancelThrow[F].raiseError(UnauthorizedException) + } +} - final case class ProjectHook( - projectId: GitLabId, - projectHookUrl: ProjectHookUrl, - serializedHookToken: SerializedHookToken - ) +private object GLHookRemover { + def apply[F[_]: Async: GitLabClient: Logger]: F[GLHookRemover[F]] = + new GLHookRemoverImpl[F].pure[F].widen[GLHookRemover[F]] } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpoint.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpoint.scala index 7cfbdf4864..f8428e38b4 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpoint.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpoint.scala @@ -27,8 +27,8 @@ import io.renku.http.client.GitLabClient import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.server.security.model.AuthUser import io.renku.http.{ErrorMessage, InfoMessage} -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult.{HookDeleted, HookNotFound} +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult.HookDeleted import io.renku.webhookservice.model.{HookIdentifier, ProjectHookUrl} import org.http4s.dsl.Http4sDsl import org.http4s.{Response, Status} @@ -42,13 +42,17 @@ trait HookDeletionEndpoint[F[_]] { class HookDeletionEndpointImpl[F[_]: MonadThrow: Logger]( projectHookUrl: ProjectHookUrl, - hookDeletor: HookDeletor[F] + hookDeletor: HookRemover[F] ) extends Http4sDsl[F] with HookDeletionEndpoint[F] { - private lazy val toHttpResponse: DeletionResult => F[Response[F]] = { - case HookNotFound => NotFound(InfoMessage("Hook already deleted")) - case HookDeleted => Ok(InfoMessage("Hook deleted")) + def deleteHook(projectId: GitLabId, authUser: AuthUser): F[Response[F]] = { + hookDeletor.deleteHook(HookIdentifier(projectId, projectHookUrl), authUser.accessToken).flatMap(toHttpResponse(_)) + } recoverWith httpResponse + + private lazy val toHttpResponse: Option[DeletionResult] => F[Response[F]] = { + case Some(HookDeleted) => Ok(InfoMessage("Hook deleted")) + case _ => NotFound(InfoMessage("Hook not found")) } private lazy val httpResponse: PartialFunction[Throwable, F[Response[F]]] = { case ex @ UnauthorizedException => @@ -58,17 +62,10 @@ class HookDeletionEndpointImpl[F[_]: MonadThrow: Logger]( case NonFatal(exception) => Logger[F].error(exception)(exception.getMessage) >> InternalServerError(ErrorMessage(exception)) } - - def deleteHook(projectId: GitLabId, authUser: AuthUser): F[Response[F]] = { - for { - deletionResult <- hookDeletor.deleteHook(HookIdentifier(projectId, projectHookUrl), authUser.accessToken) - response <- toHttpResponse(deletionResult) - } yield response - } recoverWith httpResponse } object HookDeletionEndpoint { def apply[F[_]: Async: GitLabClient: Logger](projectHookUrl: ProjectHookUrl): F[HookDeletionEndpoint[F]] = for { - hookDeletor <- HookDeletor[F] + hookDeletor <- HookRemover[F] } yield new HookDeletionEndpointImpl[F](projectHookUrl, hookDeletor) } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletor.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookRemover.scala similarity index 53% rename from webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletor.scala rename to webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookRemover.scala index 02be238b1c..3e69265d00 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookDeletor.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookdeletion/HookRemover.scala @@ -22,51 +22,54 @@ import cats.effect._ import cats.syntax.all._ import io.renku.graph.model.projects.GitLabId import io.renku.http.client.{AccessToken, GitLabClient} -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult import io.renku.webhookservice.hookfetcher.ProjectHookFetcher import io.renku.webhookservice.hookfetcher.ProjectHookFetcher.HookIdAndUrl import io.renku.webhookservice.model.HookIdentifier import org.typelevel.log4cats.Logger -import scala.util.control.NonFatal - -private trait HookDeletor[F[_]] { - def deleteHook(hookIdentifier: HookIdentifier, accessToken: AccessToken): F[DeletionResult] +private trait HookRemover[F[_]] { + def deleteHook(hookIdentifier: HookIdentifier, accessToken: AccessToken): F[Option[DeletionResult]] } -private class HookDeletorImpl[F[_]: Spawn: Logger](projectHookFetcher: ProjectHookFetcher[F], - projectHookDeletor: ProjectHookDeletor[F] -) extends HookDeletor[F] { - import HookDeletor.DeletionResult - import projectHookDeletor._ +private class HookRemoverImpl[F[_]: Async: Logger](projectHookFetcher: ProjectHookFetcher[F], + projectHookRemover: GLHookRemover[F] +) extends HookRemover[F] { + import HookRemover.DeletionResult + import projectHookRemover._ + import projectHookFetcher._ - override def deleteHook(projectHookIdentifier: HookIdentifier, accessToken: AccessToken): F[DeletionResult] = (for { - gitlabProjectHooks <- projectHookFetcher.fetchProjectHooks(projectHookIdentifier.projectId, accessToken) - result <- findProjectHook(projectHookIdentifier, gitlabProjectHooks) match { - case Some(hookToDelete) => delete(projectHookIdentifier.projectId, hookToDelete, accessToken) - case None => DeletionResult.HookNotFound.pure[F].widen[DeletionResult] - } - } yield result) recoverWith loggingError(projectHookIdentifier.projectId) + override def deleteHook(projectHookIdentifier: HookIdentifier, + accessToken: AccessToken + ): F[Option[DeletionResult]] = { + fetchProjectHooks(projectHookIdentifier.projectId, accessToken) >>= { + case None => Option.empty[DeletionResult].pure[F] + case Some(hooks) => + findProjectHook(projectHookIdentifier, hooks) match { + case Some(hookToDelete) => delete(projectHookIdentifier.projectId, hookToDelete, accessToken).map(_.some) + case None => DeletionResult.HookNotFound.widen.some.pure[F] + } + } + } onError logError(projectHookIdentifier.projectId) private def findProjectHook(projectHook: HookIdentifier, gitlabHooks: List[HookIdAndUrl]): Option[HookIdAndUrl] = gitlabHooks.find(_.url == projectHook.projectHookUrl) - private def loggingError(projectId: GitLabId): PartialFunction[Throwable, F[DeletionResult]] = { - case NonFatal(exception) => - Logger[F].error(exception)(s"Hook deletion failed for project with id $projectId") >> - exception.raiseError[F, DeletionResult] + private def logError(projectId: GitLabId): PartialFunction[Throwable, F[Unit]] = { exception => + Logger[F].error(exception)(s"Hook deletion failed for project with id $projectId") } - } -private object HookDeletor { +private object HookRemover { - def apply[F[_]: Async: GitLabClient: Logger]: F[HookDeletor[F]] = for { - hookDeletor <- ProjectHookDeletor[F] + def apply[F[_]: Async: GitLabClient: Logger]: F[HookRemover[F]] = for { + hookRemover <- GLHookRemover[F] projectHookFetcher <- ProjectHookFetcher[F] - } yield new HookDeletorImpl[F](projectHookFetcher, hookDeletor) + } yield new HookRemoverImpl[F](projectHookFetcher, hookRemover) - sealed trait DeletionResult extends Product with Serializable + sealed trait DeletionResult extends Product { + lazy val widen: DeletionResult = this + } object DeletionResult { final case object HookDeleted extends DeletionResult diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcher.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcher.scala index 106cd88823..256ac5ac51 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcher.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcher.scala @@ -18,7 +18,6 @@ package io.renku.webhookservice.hookfetcher -import cats.MonadThrow import cats.effect.Async import cats.syntax.all._ import eu.timepit.refined.auto._ @@ -31,41 +30,40 @@ import org.http4s.implicits.http4sLiteralsSyntax import org.typelevel.log4cats.Logger private[webhookservice] trait ProjectHookFetcher[F[_]] { - def fetchProjectHooks( - projectId: projects.GitLabId, - accessToken: AccessToken - ): F[List[HookIdAndUrl]] + def fetchProjectHooks(projectId: projects.GitLabId, accessToken: AccessToken): F[Option[List[HookIdAndUrl]]] } private[webhookservice] object ProjectHookFetcher { - def apply[F[_]: Async: GitLabClient: Logger]: F[ProjectHookFetcher[F]] = new ProjectHookFetcherImpl[F].pure[F].widen + def apply[F[_]: Async: GitLabClient: Logger]: F[ProjectHookFetcher[F]] = + new ProjectHookFetcherImpl[F].pure[F].widen - final case class HookIdAndUrl(id: String, url: ProjectHookUrl) + final case class HookIdAndUrl(id: Int, url: ProjectHookUrl) } private[webhookservice] class ProjectHookFetcherImpl[F[_]: Async: GitLabClient: Logger] extends ProjectHookFetcher[F] { import io.circe._ - import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.tinytypes.TinyTypeURIEncoder._ - import org.http4s.Status.{NotFound, Ok, Unauthorized} + import org.http4s.Status.{Forbidden, NotFound, Ok, Unauthorized} import org.http4s._ import org.http4s.circe._ - private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[List[HookIdAndUrl]]] = { - case (Ok, _, response) => response.as[List[HookIdAndUrl]] - case (NotFound, _, _) => List.empty[HookIdAndUrl].pure[F] - case (Unauthorized, _, _) => MonadThrow[F].raiseError(UnauthorizedException) - } - - override def fetchProjectHooks(projectId: projects.GitLabId, accessToken: AccessToken): F[List[HookIdAndUrl]] = + override def fetchProjectHooks(projectId: projects.GitLabId, + accessToken: AccessToken + ): F[Option[List[HookIdAndUrl]]] = GitLabClient[F].get(uri"projects" / projectId / "hooks", "project-hooks")(mapResponse)(accessToken.some) + private lazy val mapResponse: PartialFunction[(Status, Request[F], Response[F]), F[Option[List[HookIdAndUrl]]]] = { + case (Ok, _, response) => response.as[List[HookIdAndUrl]].map(_.some) + case (NotFound, _, _) => List.empty[HookIdAndUrl].some.pure[F] + case (Unauthorized | Forbidden, _, _) => Option.empty[List[HookIdAndUrl]].pure[F] + } + private implicit lazy val hooksIdsAndUrlsDecoder: EntityDecoder[F, List[HookIdAndUrl]] = { - implicit val hookIdAndUrlDecoder: Decoder[List[HookIdAndUrl]] = decodeList { cursor => + implicit val decoder: Decoder[List[HookIdAndUrl]] = decodeList { cursor => for { url <- cursor.downField("url").as[String].map(ProjectHookUrl.fromGitlab) - id <- cursor.downField("id").as[String] orElse cursor.downField("id").as[Long].map(_.toString) + id <- cursor.downField("id").as[Int] } yield HookIdAndUrl(id, url) } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpoint.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpoint.scala index 3579e120eb..79b6ad44e6 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpoint.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpoint.scala @@ -24,48 +24,37 @@ import cats.syntax.all._ import io.renku.graph.model.projects.GitLabId import io.renku.http.ErrorMessage._ import io.renku.http.client.GitLabClient -import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.server.security.model.AuthUser import io.renku.http.{ErrorMessage, InfoMessage} import io.renku.webhookservice.hookvalidation import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult.{HookExists, HookMissing} import io.renku.webhookservice.model.ProjectHookUrl +import org.http4s.Response import org.http4s.dsl.Http4sDsl -import org.http4s.{Response, Status} import org.typelevel.log4cats.Logger -import scala.util.control.NonFatal - trait HookValidationEndpoint[F[_]] { def validateHook(projectId: GitLabId, authUser: AuthUser): F[Response[F]] } -class HookValidationEndpointImpl[F[_]: MonadThrow: Logger]( - hookValidator: HookValidator[F] -) extends Http4sDsl[F] +class HookValidationEndpointImpl[F[_]: MonadThrow: Logger](hookValidator: HookValidator[F]) + extends Http4sDsl[F] with HookValidationEndpoint[F] { def validateHook(projectId: GitLabId, authUser: AuthUser): F[Response[F]] = { - for { - creationResult <- hookValidator.validateHook(projectId, Some(authUser.accessToken)) - response <- toHttpResponse(creationResult) - } yield response - } recoverWith withHttpResult + hookValidator.validateHook(projectId, Some(authUser.accessToken)).flatMap(toHttpResponse(_)) + } handleErrorWith withHttpResult - private lazy val toHttpResponse: HookValidationResult => F[Response[F]] = { - case HookExists => Ok(InfoMessage("Hook valid")) - case HookMissing => NotFound(InfoMessage("Hook not found")) + private lazy val toHttpResponse: Option[HookValidationResult] => F[Response[F]] = { + case Some(HookExists) => Ok(InfoMessage("Hook valid")) + case Some(HookMissing) => NotFound(InfoMessage("Hook not found")) + case None => Response[F](Unauthorized).withEntity[ErrorMessage](ErrorMessage("Unauthorized")).pure[F] } - private lazy val withHttpResult: PartialFunction[Throwable, F[Response[F]]] = { - case ex @ UnauthorizedException => - Response[F](Status.Unauthorized) - .withEntity[ErrorMessage](ErrorMessage(ex)) - .pure[F] - case NonFatal(exception) => - Logger[F].error(exception)(exception.getMessage) >> - InternalServerError(ErrorMessage(exception)) + private lazy val withHttpResult: Throwable => F[Response[F]] = { exception => + Logger[F].error(exception)(exception.getMessage) >> + InternalServerError(ErrorMessage(exception)) } } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidator.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidator.scala index 54cb7501c7..589f02e9d1 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidator.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/HookValidator.scala @@ -18,9 +18,10 @@ package io.renku.webhookservice.hookvalidation -import cats.{Applicative, MonadThrow} +import cats.data.OptionT import cats.effect.Async import cats.syntax.all._ +import cats.{Applicative, MonadThrow} import io.renku.graph.model.projects.GitLabId import io.renku.graph.tokenrepository.{AccessTokenFinder, AccessTokenFinderImpl, TokenRepositoryUrl} import io.renku.http.client.{AccessToken, GitLabClient} @@ -29,13 +30,32 @@ import io.renku.webhookservice.model.{HookIdentifier, ProjectHookUrl} import io.renku.webhookservice.tokenrepository._ import org.typelevel.log4cats.Logger -import scala.util.control.NonFatal - trait HookValidator[F[_]] { - def validateHook(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[HookValidationResult] + def validateHook(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Option[HookValidationResult]] +} + +object HookValidator { + + sealed trait HookValidationResult + object HookValidationResult { + final case object HookExists extends HookValidationResult + final case object HookMissing extends HookValidationResult + } + + def apply[F[_]: Async: GitLabClient: Logger](projectHookUrl: ProjectHookUrl): F[HookValidator[F]] = for { + tokenRepositoryUrl <- TokenRepositoryUrl[F]() + projectHookVerifier <- ProjectHookVerifier[F] + accessTokenAssociator <- AccessTokenAssociator[F] + accessTokenRemover <- AccessTokenRemover[F] + } yield new HookValidatorImpl[F](projectHookUrl, + projectHookVerifier, + new AccessTokenFinderImpl(tokenRepositoryUrl), + accessTokenAssociator, + accessTokenRemover + ) } -class HookValidatorImpl[F[_]: MonadThrow: Logger]( +private class HookValidatorImpl[F[_]: MonadThrow: Logger]( projectHookUrl: ProjectHookUrl, projectHookVerifier: ProjectHookVerifier[F], accessTokenFinder: AccessTokenFinder[F], @@ -45,92 +65,43 @@ class HookValidatorImpl[F[_]: MonadThrow: Logger]( private val applicative = Applicative[F] - import HookValidator._ import HookValidator.HookValidationResult._ - import Token._ + import HookValidator._ import accessTokenAssociator._ import accessTokenFinder._ import accessTokenRemover._ import applicative._ import projectHookVerifier._ - def validateHook(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[HookValidationResult] = - findToken(projectId, maybeAccessToken) - .flatMap { - case GivenToken(token) => - for { - hookPresent <- checkHookPresence(HookIdentifier(projectId, projectHookUrl), token) - _ <- whenA(hookPresent)(associate(projectId, token)) - _ <- whenA(!hookPresent)(removeAccessToken(projectId)) - validationResult <- toValidationResult(hookPresent).pure[F] - } yield validationResult - case StoredToken(token) => - for { - hookPresent <- checkHookPresence(HookIdentifier(projectId, projectHookUrl), token) - _ <- whenA(!hookPresent)(removeAccessToken(projectId)) - validationResult <- toValidationResult(hookPresent).pure[F] - } yield validationResult - } - .onError(logError(projectId)) - - private def findToken(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Token] = - maybeAccessToken - .map(GivenToken(_).widen.pure[F]) - .getOrElse(findStoredToken(projectId)) + def validateHook(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Option[HookValidationResult]] = { + persistGivenToken(projectId, maybeAccessToken) >> validateHookExistence(projectId) + }.onError(logError(projectId)) - private def findStoredToken(projectId: GitLabId): F[Token] = + private def persistGivenToken(projectId: GitLabId, maybeAccessToken: Option[AccessToken]): F[Unit] = + maybeAccessToken + .map(associate(projectId, _)) + .getOrElse(().pure[F]) + + private def validateHookExistence(projectId: GitLabId): F[Option[HookValidationResult]] = + fetchToken(projectId) + .flatMapF(at => checkHookPresence(HookIdentifier(projectId, projectHookUrl), at)) + .cataF( + default = Option.empty[HookValidationResult].pure[F], + hookPresent => whenA(!hookPresent)(removeAccessToken(projectId)).as(validationResultFrom(hookPresent).some) + ) + + private def fetchToken(projectId: GitLabId): OptionT[F, AccessToken] = OptionT { findAccessToken(projectId) .adaptError(storedAccessTokenError(projectId)) - .flatMap(getOrError(projectId)) + } private def storedAccessTokenError(projectId: GitLabId): PartialFunction[Throwable, Throwable] = exception => new Exception(s"Finding stored access token for $projectId failed", exception) - private def getOrError(projectId: GitLabId): Option[AccessToken] => F[Token] = { - case Some(token) => - StoredToken(token).widen.pure[F] - case None => - NoAccessTokenException(s"No stored access token found for projectId $projectId").raiseError[F, Token] - } - - private def toValidationResult(projectHookPresent: Boolean): HookValidationResult = + private def validationResultFrom(projectHookPresent: Boolean): HookValidationResult = if (projectHookPresent) HookExists else HookMissing - private def logError(projectId: GitLabId): PartialFunction[Throwable, F[Unit]] = { - case NoAccessTokenException(message) => - Logger[F].info(s"Hook validation failed: $message") - case NonFatal(exception) => - Logger[F].error(exception)(s"Hook validation failed for projectId $projectId") - } - - private sealed abstract class Token(val value: AccessToken) { - lazy val widen: Token = this - } - private object Token { - case class GivenToken(override val value: AccessToken) extends Token(value) - case class StoredToken(override val value: AccessToken) extends Token(value) - } -} - -object HookValidator { - - sealed trait HookValidationResult - object HookValidationResult { - final case object HookExists extends HookValidationResult - final case object HookMissing extends HookValidationResult + private def logError(projectId: GitLabId): PartialFunction[Throwable, F[Unit]] = { case exception => + Logger[F].error(exception)(s"Hook validation failed for projectId $projectId") } - - final case class NoAccessTokenException(message: String) extends RuntimeException(message) - - def apply[F[_]: Async: GitLabClient: Logger](projectHookUrl: ProjectHookUrl): F[HookValidator[F]] = for { - tokenRepositoryUrl <- TokenRepositoryUrl[F]() - projectHookVerifier <- ProjectHookVerifier[F] - accessTokenAssociator <- AccessTokenAssociator[F] - accessTokenRemover <- AccessTokenRemover[F] - } yield new HookValidatorImpl[F](projectHookUrl, - projectHookVerifier, - new AccessTokenFinderImpl(tokenRepositoryUrl), - accessTokenAssociator, - accessTokenRemover - ) } diff --git a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifier.scala b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifier.scala index d9cb48c251..83dd59bdf4 100644 --- a/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifier.scala +++ b/webhook-service/src/main/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifier.scala @@ -27,10 +27,7 @@ import io.renku.webhookservice.model.{HookIdentifier, ProjectHookUrl} import org.typelevel.log4cats.Logger private trait ProjectHookVerifier[F[_]] { - def checkHookPresence( - projectHookId: HookIdentifier, - accessToken: AccessToken - ): F[Boolean] + def checkHookPresence(projectHookId: HookIdentifier, accessToken: AccessToken): F[Option[Boolean]] } private object ProjectHookVerifier { @@ -43,10 +40,10 @@ private class ProjectHookVerifierImpl[F[_]: Async: Logger]( projectHookFetcher: ProjectHookFetcher[F] ) extends ProjectHookVerifier[F] { - override def checkHookPresence(projectHookId: HookIdentifier, accessToken: AccessToken): F[Boolean] = + override def checkHookPresence(projectHookId: HookIdentifier, accessToken: AccessToken): F[Option[Boolean]] = projectHookFetcher .fetchProjectHooks(projectHookId.projectId, accessToken) - .map(checkProjectHookExists(projectHookId.projectHookUrl)) + .map(_.map(checkProjectHookExists(projectHookId.projectHookUrl))) private def checkProjectHookExists(urlToFind: ProjectHookUrl): List[HookIdAndUrl] => Boolean = hooksIdsAndUrls => hooksIdsAndUrls.map(_.url.value) contains urlToFind.value diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/WebhookServiceGenerators.scala b/webhook-service/src/test/scala/io/renku/webhookservice/WebhookServiceGenerators.scala index 5ac7d65f8a..1ef88dbd20 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/WebhookServiceGenerators.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/WebhookServiceGenerators.scala @@ -30,15 +30,14 @@ object WebhookServiceGenerators { implicit val serializedHookTokens: Gen[SerializedHookToken] = nonEmptyStrings() map Refined.unsafeApply - implicit val hookTokens: Gen[HookToken] = for { - projectId <- projectIds - } yield HookToken(projectId) + implicit val hookTokens: Gen[HookToken] = + projectIds.map(HookToken) implicit val selfUrls: Gen[SelfUrl] = validatedUrls map (url => SelfUrl.apply(url.value)) implicit val projectHookUrls: Gen[ProjectHookUrl] = selfUrls map ProjectHookUrl.from lazy val hookIdAndUrls: Gen[HookIdAndUrl] = for { - id <- nonEmptyStrings() + id <- positiveInts().map(_.value) hookUrl <- projectHookUrls } yield HookIdAndUrl(id, hookUrl) diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/crypto/HookTokenCryptoSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/crypto/HookTokenCryptoSpec.scala index 123f815d4a..20c6bf9f57 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/crypto/HookTokenCryptoSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/crypto/HookTokenCryptoSpec.scala @@ -19,7 +19,7 @@ package io.renku.webhookservice.crypto import com.typesafe.config.ConfigFactory -import eu.timepit.refined.api.RefType +import io.renku.config.ConfigLoader.ConfigLoadingException import io.renku.crypto.AesCrypto.Secret import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ @@ -29,9 +29,9 @@ import io.renku.webhookservice.crypto.HookTokenCrypto.SerializedHookToken import io.renku.webhookservice.model.HookToken import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec +import scodec.bits.ByteVector import java.nio.charset.StandardCharsets.UTF_8 -import java.security.InvalidKeyException import java.util.Base64 import scala.jdk.CollectionConverters._ import scala.util.{Failure, Success, Try} @@ -58,6 +58,18 @@ class HookTokenCryptoSpec extends AnyWordSpec with should.Matchers { decryptException shouldBe an[Exception] decryptException.getMessage shouldBe "HookToken decryption failed" } + + "decrypt existing values" in { + val usedSecret = Secret.unsafeFromBase64("YWJjZGVmZzEyMzQ1Njc4OQ==") + val hookTokenCrypto = new HookTokenCryptoImpl[Try](usedSecret) + + val plain = + hookTokenCrypto.decrypt( + SerializedHookToken.from("4TV8A81+1+U3dfa8sVO9TUuMvGHLaerySYpxEIkVT/U=").fold(throw _, identity) + ) + + plain shouldBe Success(HookToken(123456)) + } } "apply" should { @@ -67,7 +79,7 @@ class HookTokenCryptoSpec extends AnyWordSpec with should.Matchers { Map( "services" -> Map( "gitlab" -> Map( - "hook-token-secret" -> aesCryptoSecrets.generateOne.value + "hook-token-secret" -> aesCryptoSecrets.generateOne.toBase64 ).asJava ).asJava ).asJava @@ -93,18 +105,14 @@ class HookTokenCryptoSpec extends AnyWordSpec with should.Matchers { val Failure(exception) = HookTokenCrypto[Try](config) - exception shouldBe a[InvalidKeyException] - exception.getMessage shouldBe "Invalid AES key length: 15 bytes" + exception shouldBe a[ConfigLoadingException] + exception.getMessage should include("Expected 16 bytes, but got 15") } } private trait TestCase { - private val secret = new String(Base64.getEncoder.encode("1234567890123456".getBytes("utf-8")), "utf-8") - val hookTokenCrypto = new HookTokenCryptoImpl[Try]( - RefType - .applyRef[Secret](secret) - .getOrElse(throw new IllegalArgumentException("Wrong secret")) - ) + private val secret = Secret.unsafe(ByteVector.fromValidHex(List.fill(4)("caffee00").mkString)) + val hookTokenCrypto = new HookTokenCryptoImpl[Try](secret) } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/EndpointSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/EndpointSpec.scala index 425a80ce93..a037d9c5a3 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/EndpointSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/EndpointSpec.scala @@ -45,11 +45,9 @@ import io.renku.testtools.IOSpec import io.renku.webhookservice.eventstatus.Generators._ import io.renku.webhookservice.hookvalidation.HookValidator import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult.{HookExists, HookMissing} -import io.renku.webhookservice.hookvalidation.HookValidator.NoAccessTokenException import org.http4s.MediaType.application import org.http4s.Status._ import org.http4s.headers.`Content-Type` -import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -59,39 +57,36 @@ class EndpointSpec extends AnyWordSpec with MockFactory with should.Matchers wit "fetchProcessingStatus" should { - "return OK with the status info if webhook for the project exists" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() + "return OK with project status info if webhook for the project exists" in new TestCase { - givenHookValidation(projectId, authUser, returning = HookExists.pure[IO]) + val authUser = authUsers.generateOption - val statusInfo = statusInfos.generateOne - givenStatusInfoFinding(projectId, returning = statusInfo.some.pure[IO]) + givenHookValidation(projectId, authUser, returning = HookExists.some.pure[IO]) - val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() + val statusInfo = statusInfos.generateOne + givenStatusInfoFinding(projectId, returning = statusInfo.some.pure[IO]) - response.status shouldBe Ok - response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe statusInfo.asJson + val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() - logger.loggedOnly( - Warn(s"Finding status info for project '$projectId' finished${executionTimeRecorder.executionTimeInfo}") - ) - } - } + response.status shouldBe Ok + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.as[Json].unsafeRunSync() shouldBe statusInfo.asJson - "send COMMIT_SYNC_REQUEST message and return OK with the status info " + - "if webhook for the project exists but no status info can be found" in new TestCase { + logger.loggedOnly( + Warn(s"Finding status info for project '$projectId' finished${executionTimeRecorder.executionTimeInfo}") + ) + } - val authUser = authUserGen.generateOne + "send COMMIT_SYNC_REQUEST message and return OK with activated = true and status 'in-progress' " + + "if webhook for the project exists but no status info can be found in EL" in new TestCase { - givenHookValidation(projectId, authUser, returning = HookExists.pure[IO]) + val authUser = authUsers.generateOption + givenHookValidation(projectId, authUser, returning = HookExists.some.pure[IO]) givenStatusInfoFinding(projectId, returning = None.pure[IO]) val project = consumerProjects.generateOne.copy(id = projectId) givenProjectInfoFinding(projectId, authUser, returning = project.pure[IO]) - givenCommitSyncRequestSend(project, returning = ().pure[IO]) val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() @@ -106,101 +101,90 @@ class EndpointSpec extends AnyWordSpec with MockFactory with should.Matchers wit } "return OK with activated = false if the webhook does not exist" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() - givenHookValidation(projectId, authUser, returning = HookMissing.pure[IO]) + val authUser = authUsers.generateOption - val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() + givenHookValidation(projectId, authUser, returning = HookMissing.some.pure[IO]) + givenStatusInfoFinding(projectId, returning = statusInfos.generateOption.pure[IO]) - response.status shouldBe Ok - response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe StatusInfo.NotActivated.asJson - } + val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() + + response.status shouldBe Ok + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.as[Json].unsafeRunSync() shouldBe StatusInfo.NotActivated.asJson } - "return NOT_FOUND if no Access Token found for the project" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() + "return OK with project status info with activated = true " + + "if determining project hook existence returns None " + + "but status info can be found in EL" in new TestCase { - val exception = NoAccessTokenException("error") - givenHookValidation(projectId, - authUser, - returning = exception.raiseError[IO, HookValidator.HookValidationResult] - ) + val authUser = authUsers.generateOption + + givenHookValidation(projectId, authUser, returning = None.pure[IO]) + + val statusInfo = statusInfos.generateOne + givenStatusInfoFinding(projectId, returning = statusInfo.some.pure[IO]) val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() - response.status shouldBe NotFound + response.status shouldBe Ok response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe InfoMessage("Info about project cannot be found").asJson + response.as[Json].unsafeRunSync() shouldBe statusInfo.asJson } - } - "return INTERNAL_SERVER_ERROR when checking if the webhook exists fails" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() + "return NOT_FOUND if determining project hook existence returns None " + + "and no status info can be found in EL" in new TestCase { - val exception = exceptions.generateOne - givenHookValidation(projectId, - authUser, - returning = exception.raiseError[IO, HookValidator.HookValidationResult] - ) + val authUser = authUsers.generateOption + + givenHookValidation(projectId, authUser, returning = None.pure[IO]) + givenStatusInfoFinding(projectId, returning = None.pure[IO]) val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() - response.status shouldBe InternalServerError + response.status shouldBe NotFound response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe ErrorMessage(statusInfoFindingErrorMessage).asJson - - logger.logged(Error(statusInfoFindingErrorMessage, exception)) + response.as[Json].unsafeRunSync() shouldBe InfoMessage("Info about project cannot be found").asJson } - } - "return INTERNAL_SERVER_ERROR when finding status info returns a failure" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() + "return INTERNAL_SERVER_ERROR when checking if project webhook exists fails" in new TestCase { - givenHookValidation(projectId, authUser, returning = HookExists.pure[IO]) + val authUser = authUsers.generateOption - val exception = exceptions.generateOne - givenStatusInfoFinding(projectId, returning = IO.raiseError(exception)) + val exception = exceptions.generateOne + givenHookValidation(projectId, authUser, returning = exception.raiseError[IO, Nothing]) + givenStatusInfoFinding(projectId, returning = statusInfos.generateOption.pure[IO]) - val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() + val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() - response.status shouldBe InternalServerError - response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe ErrorMessage(statusInfoFindingErrorMessage).asJson + response.status shouldBe InternalServerError + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.as[Json].unsafeRunSync() shouldBe ErrorMessage(statusInfoFindingErrorMessage).asJson - logger.logged(Error(statusInfoFindingErrorMessage, exception)) - } + logger.logged(Error(statusInfoFindingErrorMessage, exception)) } - "return INTERNAL_SERVER_ERROR when finding status info fails" in new TestCase { - forAll(authUserGen) { authUser => - logger.reset() + "return INTERNAL_SERVER_ERROR when finding status info returns a failure" in new TestCase { - givenHookValidation(projectId, authUser, returning = HookExists.pure[IO]) + val authUser = authUsers.generateOption - val exception = exceptions.generateOne - givenStatusInfoFinding(projectId, returning = IO.raiseError(exception)) + givenHookValidation(projectId, authUser, returning = hookValidationResults.generateOption.pure[IO]) - val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() + val exception = exceptions.generateOne + givenStatusInfoFinding(projectId, returning = exception.raiseError[IO, Nothing]) - response.status shouldBe InternalServerError - response.contentType shouldBe Some(`Content-Type`(application.json)) - response.as[Json].unsafeRunSync() shouldBe ErrorMessage(statusInfoFindingErrorMessage).asJson + val response = endpoint.fetchProcessingStatus(projectId, authUser).unsafeRunSync() - logger.logged(Error(statusInfoFindingErrorMessage, exception)) - } + response.status shouldBe InternalServerError + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.as[Json].unsafeRunSync() shouldBe ErrorMessage(statusInfoFindingErrorMessage).asJson + + logger.logged(Error(statusInfoFindingErrorMessage, exception)) } } private trait TestCase { - val authUserGen: Gen[Option[AuthUser]] = - Gen.option(authUsers) - val projectId = projectIds.generateOne private val hookValidator = mock[HookValidator[IO]] @@ -215,12 +199,11 @@ class EndpointSpec extends AnyWordSpec with MockFactory with should.Matchers wit def givenHookValidation(projectId: projects.GitLabId, authUser: Option[AuthUser], - returning: IO[HookValidator.HookValidationResult] - ) = - (hookValidator - .validateHook(_: GitLabId, _: Option[AccessToken])) - .expects(projectId, authUser.map(_.accessToken)) - .returning(returning) + returning: IO[Option[HookValidator.HookValidationResult]] + ) = (hookValidator + .validateHook(_: GitLabId, _: Option[AccessToken])) + .expects(projectId, authUser.map(_.accessToken)) + .returning(returning) def givenStatusInfoFinding(projectId: projects.GitLabId, returning: IO[Option[StatusInfo]]) = (statusInfoFinder diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/Generators.scala b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/Generators.scala index 9a94475899..a5803ddbfb 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/Generators.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/Generators.scala @@ -18,7 +18,8 @@ package io.renku.webhookservice.eventstatus -import io.renku.graph.model.EventsGenerators.eventStatuses +import io.renku.graph.model.EventContentGenerators.eventInfos +import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult import org.scalacheck.Gen private object Generators { @@ -26,8 +27,11 @@ private object Generators { implicit val statusInfos: Gen[StatusInfo] = Gen .either( - eventStatuses.map(StatusInfo.activated), + eventInfos().map(StatusInfo.activated), StatusInfo.NotActivated ) .map(_.merge) + + implicit val hookValidationResults: Gen[HookValidationResult] = + Gen.oneOf(HookValidationResult.HookExists, HookValidationResult.HookMissing) } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoFinderSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoFinderSpec.scala index d6fd4489c5..581349102c 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoFinderSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoFinderSpec.scala @@ -42,7 +42,7 @@ class StatusInfoFinderSpec extends AnyWordSpec with should.Matchers with MockFac val eventInfo = eventInfos(projectIdGen = fixed(projectId)).generateOne givenGetEvents(projectId, returning = EventLogClient.Result.Success(List(eventInfo)).pure[Try]) - fetcher.findStatusInfo(projectId) shouldBe StatusInfo.activated(eventInfo.status).some.pure[Try] + fetcher.findStatusInfo(projectId) shouldBe StatusInfo.activated(eventInfo).some.pure[Try] } "return activated project StatusInfo when no events found for the project" in new TestCase { diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoSpec.scala index dbca5c2feb..a22f2ab07d 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/eventstatus/StatusInfoSpec.scala @@ -22,9 +22,10 @@ import cats.syntax.all._ import io.circe.literal._ import io.circe.syntax._ import io.renku.generators.Generators.Implicits._ +import io.renku.graph.model.EventContentGenerators.{eventInfos, eventMessages} import io.renku.graph.model.EventsGenerators.eventStatuses -import io.renku.graph.model.events.{EventStatus, EventStatusProgress} -import EventStatus._ +import io.renku.graph.model.events.EventStatus._ +import io.renku.graph.model.events.{EventMessage, EventStatus, EventStatusProgress} import org.scalatest.matchers.should import org.scalatest.prop.TableDrivenPropertyChecks import org.scalatest.wordspec.AnyWordSpec @@ -35,8 +36,9 @@ class StatusInfoSpec extends AnyWordSpec with should.Matchers with ScalaCheckPro "encode" should { "produce Json from a StatusInfo of an activated project with a NonZero progress" in { - forAll { eventStatus: EventStatus => - val info = StatusInfo.activated(eventStatus) + + forAll(eventInfos()) { eventInfo => + val info = StatusInfo.activated(eventInfo) info.asJson shouldBe json"""{ "activated": true, @@ -46,10 +48,11 @@ class StatusInfoSpec extends AnyWordSpec with should.Matchers with ScalaCheckPro "percentage": ${info.progress.percentage} }, "details": { - "status": ${info.details.status}, - "message": ${info.details.message} + "status": ${info.details.status}, + "message": ${info.details.message}, + "stacktrace": ${info.details.maybeDetails} } - }""" + }""".deepDropNullValues } } @@ -95,26 +98,36 @@ class DetailsSpec extends AnyWordSpec with should.Matchers with TableDrivenPrope forAll( Table( - ("eventStatus", "status", "message"), - (New, "in-progress", "new"), - (Skipped, "success", "skipped"), - (GeneratingTriples, "in-progress", "generating triples"), - (GenerationRecoverableFailure, "in-progress", "generation recoverable failure"), - (GenerationNonRecoverableFailure, "failure", "generation non recoverable failure"), - (TriplesGenerated, "in-progress", "triples generated"), - (TransformingTriples, "in-progress", "transforming triples"), - (TransformationRecoverableFailure, "in-progress", "transformation recoverable failure"), - (TransformationNonRecoverableFailure, "failure", "transformation non recoverable failure"), - (TriplesStore, "success", "triples store"), - (AwaitingDeletion, "in-progress", "awaiting deletion"), - (Deleting, "in-progress", "deleting") + ("eventStatus", "status", "message", "maybeMessage"), + (New, "in-progress", "new", Option.empty[EventMessage]), + (Skipped, "success", "skipped", eventMessages.generateSome), + (GeneratingTriples, "in-progress", "generating triples", Option.empty[EventMessage]), + (GenerationRecoverableFailure, "in-progress", "generation recoverable failure", eventMessages.generateSome), + (GenerationNonRecoverableFailure, "failure", "generation non recoverable failure", eventMessages.generateSome), + (TriplesGenerated, "in-progress", "triples generated", Option.empty[EventMessage]), + (TransformingTriples, "in-progress", "transforming triples", Option.empty[EventMessage]), + (TransformationRecoverableFailure, + "in-progress", + "transformation recoverable failure", + eventMessages.generateSome + ), + (TransformationNonRecoverableFailure, + "failure", + "transformation non recoverable failure", + eventMessages.generateSome + ), + (TriplesStore, "success", "triples store", Option.empty[EventMessage]), + (AwaitingDeletion, "in-progress", "awaiting deletion", Option.empty[EventMessage]), + (Deleting, "in-progress", "deleting", Option.empty[EventMessage]) ) - ) { (eventStatus, status, message) => + ) { (eventStatus, status, message, maybeEventMessage) => show"provide '$status' as status and '$message' as message for the '$eventStatus' status" in { - val details = Details.fromStatus(eventStatus) + val details = + Details.from(eventInfos().generateOne.copy(status = eventStatus, maybeMessage = maybeEventMessage)) - details.status shouldBe status - details.message shouldBe message + details.status shouldBe status + details.message shouldBe message + details.maybeDetails shouldBe maybeEventMessage.map(_.value) } } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreationEndpointSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreationEndpointSpec.scala index d455dc9b0a..d99b349391 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreationEndpointSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreationEndpointSpec.scala @@ -19,6 +19,7 @@ package io.renku.webhookservice.hookcreation import cats.effect.IO +import cats.syntax.all._ import io.circe.Json import io.circe.literal._ import io.circe.syntax._ @@ -28,9 +29,9 @@ import io.renku.graph.model.GraphModelGenerators.projectIds import io.renku.graph.model.projects.GitLabId import io.renku.http.ErrorMessage import io.renku.http.ErrorMessage._ -import io.renku.http.client.AccessToken import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.server.EndpointTester._ +import io.renku.http.server.security.model.AuthUser import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Error import io.renku.testtools.IOSpec @@ -46,12 +47,12 @@ class HookCreationEndpointSpec extends AnyWordSpec with MockFactory with should. "createHook" should { - "return CREATED when webhook is successfully created for project with the given id in in GitLab" in new TestCase { + "return CREATED when webhook is successfully created for project with the given id" in new TestCase { (hookCreator - .createHook(_: GitLabId, _: AccessToken)) - .expects(projectId, authUser.accessToken) - .returning(IO.pure(HookCreated)) + .createHook(_: GitLabId, _: AuthUser)) + .expects(projectId, authUser) + .returning(HookCreated.some.pure[IO]) val response = createHook(projectId, authUser).unsafeRunSync() @@ -63,9 +64,9 @@ class HookCreationEndpointSpec extends AnyWordSpec with MockFactory with should. "return OK when hook was already created" in new TestCase { (hookCreator - .createHook(_: GitLabId, _: AccessToken)) - .expects(projectId, authUser.accessToken) - .returning(IO.pure(HookExisted)) + .createHook(_: GitLabId, _: AuthUser)) + .expects(projectId, authUser) + .returning(HookExisted.some.pure[IO]) val response = createHook(projectId, authUser).unsafeRunSync() @@ -74,13 +75,27 @@ class HookCreationEndpointSpec extends AnyWordSpec with MockFactory with should. response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Hook already existed"}""" } + "return NOT_FOUND when hook creation returns None" in new TestCase { + + (hookCreator + .createHook(_: GitLabId, _: AuthUser)) + .expects(projectId, authUser) + .returning(None.pure[IO]) + + val response = createHook(projectId, authUser).unsafeRunSync() + + response.status shouldBe NotFound + response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) + response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Project not found"}""" + } + "return INTERNAL_SERVER_ERROR when there was an error during hook creation and log the error" in new TestCase { val errorMessage = ErrorMessage("some error") val exception = new Exception(errorMessage.toString()) (hookCreator - .createHook(_: GitLabId, _: AccessToken)) - .expects(projectId, authUser.accessToken) + .createHook(_: GitLabId, _: AuthUser)) + .expects(projectId, authUser) .returning(IO.raiseError(exception)) val response = createHook(projectId, authUser).unsafeRunSync() @@ -89,16 +104,14 @@ class HookCreationEndpointSpec extends AnyWordSpec with MockFactory with should. response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) response.as[Json].unsafeRunSync() shouldBe errorMessage.asJson - logger.loggedOnly( - Error(exception.getMessage, exception) - ) + logger.loggedOnly(Error(exception.getMessage, exception)) } "return UNAUTHORIZED when there was an UnauthorizedException thrown during hook creation" in new TestCase { (hookCreator - .createHook(_: GitLabId, _: AccessToken)) - .expects(projectId, authUser.accessToken) + .createHook(_: GitLabId, _: AuthUser)) + .expects(projectId, authUser) .returning(IO.raiseError(UnauthorizedException)) val response = createHook(projectId, authUser).unsafeRunSync() diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreatorSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreatorSpec.scala index e8d0b35ca7..d21e935c49 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreatorSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookcreation/HookCreatorSpec.scala @@ -45,17 +45,24 @@ import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult import io.renku.webhookservice.model.HookToken import io.renku.webhookservice.tokenrepository.AccessTokenAssociator import org.scalamock.scalatest.MockFactory +import org.scalatest.OptionValues import org.scalatest.concurrent.Eventually import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers with IOSpec with Eventually { +class HookCreatorSpec + extends AnyWordSpec + with MockFactory + with should.Matchers + with OptionValues + with IOSpec + with Eventually { "createHook" should { "return HookCreated if hook does not exists and it was successfully created" in new TestCase { - givenHookValidation(returning = HookMissing.pure[IO]) + givenHookValidation(returning = HookMissing.some.pure[IO]) givenTokenAssociation(returning = IO.unit) @@ -67,7 +74,7 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers givenCommitSyncRequestSending(returning = IO.unit) - hookCreation.createHook(projectId, accessToken).unsafeRunSync() shouldBe HookCreated + hookCreation.createHook(projectId, authUser).unsafeRunSync().value shouldBe HookCreated validateCommitSyncRequestSent @@ -76,28 +83,35 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers "return HookExisted if hook was already created for that project" in new TestCase { - givenHookValidation(returning = HookExists.pure[IO]) - - givenTokenAssociation(returning = IO.unit) + givenHookValidation(returning = HookExists.some.pure[IO]) givenProjectInfoFinding(returning = projectInfo.pure[IO]) givenCommitSyncRequestSending(returning = ().pure[IO]) - hookCreation.createHook(projectId, accessToken).unsafeRunSync() shouldBe HookExisted + hookCreation.createHook(projectId, authUser).unsafeRunSync().value shouldBe HookExisted validateCommitSyncRequestSent logger.loggedOnly(Info(show"Hook existed for projectId $projectId")) } + "return None if hook validation returns None" in new TestCase { + + givenHookValidation(returning = None.pure[IO]) + + hookCreation.createHook(projectId, authUser).unsafeRunSync() shouldBe None + + logger.loggedOnly(Info(show"Hook on projectId $projectId cannot be created by user ${authUser.id}")) + } + "log an error if hook validation fails" in new TestCase { val exception = exceptions.generateOne - givenHookValidation(returning = exception.raiseError[IO, HookValidationResult]) + givenHookValidation(returning = exception.raiseError[IO, Nothing]) intercept[Exception] { - hookCreation.createHook(projectId, accessToken).unsafeRunSync() + hookCreation.createHook(projectId, authUser).unsafeRunSync() } shouldBe exception logger.loggedOnly(Error(s"Hook creation failed for project with id $projectId", exception)) @@ -105,13 +119,13 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers "log an error if associating projectId with accessToken fails" in new TestCase { - givenHookValidation(returning = HookMissing.pure[IO]) + givenHookValidation(returning = HookMissing.some.pure[IO]) val exception = exceptions.generateOne givenTokenAssociation(returning = exception.raiseError[IO, Unit]) intercept[Exception] { - hookCreation.createHook(projectId, accessToken).unsafeRunSync() + hookCreation.createHook(projectId, authUser).unsafeRunSync() } shouldBe exception logger.loggedOnly(Error(s"Hook creation failed for project with id $projectId", exception)) @@ -119,15 +133,15 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers "log an error if hook token encryption fails" in new TestCase { - givenHookValidation(returning = HookMissing.pure[IO]) + givenHookValidation(returning = HookMissing.some.pure[IO]) - givenTokenAssociation(returning = IO.unit) + givenTokenAssociation(returning = ().pure[IO]) val exception = exceptions.generateOne givenTokenEncryption(returning = exception.raiseError[IO, HookTokenCrypto.SerializedHookToken]) intercept[Exception] { - hookCreation.createHook(projectId, accessToken).unsafeRunSync() + hookCreation.createHook(projectId, authUser).unsafeRunSync() } shouldBe exception logger.loggedOnly(Error(s"Hook creation failed for project with id $projectId", exception)) @@ -135,9 +149,9 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers "log an error if hook creation fails" in new TestCase { - givenHookValidation(returning = HookMissing.pure[IO]) + givenHookValidation(returning = HookMissing.some.pure[IO]) - givenTokenAssociation(returning = IO.unit) + givenTokenAssociation(returning = ().pure[IO]) givenTokenEncryption(returning = serializedHookToken.pure[IO]) @@ -145,7 +159,7 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers givenHookCreation(returning = exception.raiseError[IO, Unit]) intercept[Exception] { - hookCreation.createHook(projectId, accessToken).unsafeRunSync() + hookCreation.createHook(projectId, authUser).unsafeRunSync() } shouldBe exception logger.loggedOnly(Error(s"Hook creation failed for project with id $projectId", exception)) @@ -153,14 +167,12 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers "log an error and do not send a COMMIT_SYNC_REQUEST if project info fetching fails" in new TestCase { - givenHookValidation(returning = HookExists.pure[IO]) - - givenTokenAssociation(returning = IO.unit) + givenHookValidation(returning = HookExists.some.pure[IO]) val exception = exceptions.generateOne givenProjectInfoFinding(returning = exception.raiseError[IO, Project]) - hookCreation.createHook(projectId, accessToken).unsafeRunSync() shouldBe HookExisted + hookCreation.createHook(projectId, authUser).unsafeRunSync().value shouldBe HookExisted eventually { logger.loggedOnly( @@ -175,17 +187,18 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers } private trait TestCase { - val projectInfo = consumerProjects.generateOne - val projectId = projectInfo.id - val serializedHookToken = serializedHookTokens.generateOne - val accessToken = accessTokens.generateOne - val projectHookUrl = projectHookUrls.generateOne + val projectInfo = consumerProjects.generateOne + val projectId = projectInfo.id + val serializedHookToken = serializedHookTokens.generateOne + val authUser = authUsers.generateOne + private val accessToken = authUser.accessToken + private val projectHookUrl = projectHookUrls.generateOne implicit val logger: TestLogger[IO] = TestLogger[IO]() private val projectHookValidator = mock[HookValidator[IO]] + private val tokenAssociator = mock[AccessTokenAssociator[IO]] private val projectHookCreator = mock[ProjectHookCreator[IO]] private val hookTokenCrypto = mock[HookTokenCrypto[IO]] - private val accessTokenAssociator = mock[AccessTokenAssociator[IO]] private val projectInfoFinderResponse = Queue.bounded[IO, IO[Project]](1).unsafeRunSync() private val projectInfoFinder = new ProjectInfoFinder[IO] { override def findProjectInfo(projectId: GitLabId)(implicit maybeAccessToken: Option[AccessToken]): IO[Project] = @@ -199,21 +212,21 @@ class HookCreatorSpec extends AnyWordSpec with MockFactory with should.Matchers val hookCreation = new HookCreatorImpl[IO]( projectHookUrl, projectHookValidator, + tokenAssociator, projectInfoFinder, hookTokenCrypto, projectHookCreator, - accessTokenAssociator, elClient ) - def givenHookValidation(returning: IO[HookValidationResult]) = + def givenHookValidation(returning: IO[Option[HookValidationResult]]) = (projectHookValidator .validateHook(_: GitLabId, _: Option[AccessToken])) .expects(projectId, Some(accessToken)) .returning(returning) def givenTokenAssociation(returning: IO[Unit]) = - (accessTokenAssociator + (tokenAssociator .associate(_: GitLabId, _: AccessToken)) .expects(projectId, accessToken) .returning(returning) diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletorSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/GLHookRemoverSpec.scala similarity index 76% rename from webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletorSpec.scala rename to webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/GLHookRemoverSpec.scala index d5f89c194c..91b88c9ef4 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/ProjectHookDeletorSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/GLHookRemoverSpec.scala @@ -28,12 +28,12 @@ import io.renku.graph.model.GraphModelGenerators.projectIds import io.renku.http.client.RestClient.ResponseMappingF import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.client.{AccessToken, GitLabClient} +import io.renku.http.tinytypes.TinyTypeURIEncoder._ import io.renku.interpreters.TestLogger import io.renku.stubbing.ExternalServiceStubbing import io.renku.testtools.{GitLabClientTools, IOSpec} -import io.renku.webhookservice.WebhookServiceGenerators.{hookIdAndUrls, projectHookUrls, serializedHookTokens} -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult -import io.renku.webhookservice.hookdeletion.ProjectHookDeletor.ProjectHook +import io.renku.webhookservice.WebhookServiceGenerators.hookIdAndUrls +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult import org.http4s.Method.DELETE import org.http4s.implicits.http4sLiteralsSyntax import org.http4s.{Request, Response, Status, Uri} @@ -42,7 +42,7 @@ import org.scalamock.scalatest.MockFactory import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -class ProjectHookDeletorSpec +class GLHookRemoverSpec extends AnyWordSpec with MockFactory with ExternalServiceStubbing @@ -53,16 +53,16 @@ class ProjectHookDeletorSpec "delete" should { "return the DeletionResult from GitLabClient" in new TestCase { - val accessToken = accessTokens.generateOne - val result = deletionResults.generateOne + val accessToken = accessTokens.generateOne + val result = deletionResults.generateOne (gitLabClient .delete(_: Uri, _: NonEmptyString)(_: ResponseMappingF[IO, DeletionResult])(_: Option[AccessToken])) .expects(uri, endpointName, *, accessToken.some) .returning(result.pure[IO]) - hookDeletor + remover .delete(projectId, hookIdAndUrl, accessToken) .unsafeRunSync() shouldBe result } @@ -78,13 +78,15 @@ class ProjectHookDeletorSpec mapResponse(Status.NotFound, Request(), Response()).unsafeRunSync() shouldBe DeletionResult.HookNotFound } - "return an UnauthorizedException if remote client responds with UNAUTHORIZED" in new TestCase { - intercept[UnauthorizedException] { - mapResponse(Status.Unauthorized, Request(), Response()).unsafeRunSync() + Status.Unauthorized :: Status.Forbidden :: Nil foreach { status => + s"return an UnauthorizedException if remote client responds with $status" in new TestCase { + intercept[UnauthorizedException] { + mapResponse(status, Request(), Response()).unsafeRunSync() + } } } - "return an Exception if remote client responds with status neither OK, NOT_FOUND or UNAUTHORIZED" in new TestCase { + "return an Exception if remote client responds with status neither OK, NOT_FOUND, UNAUTHORIZED or FORBIDDEN" in new TestCase { intercept[Exception] { mapResponse(Status.ServiceUnavailable, Request(), Response()).unsafeRunSync() } @@ -94,26 +96,20 @@ class ProjectHookDeletorSpec private trait TestCase { val hookIdAndUrl = hookIdAndUrls.generateOne val projectId = projectIds.generateOne - val uri = uri"projects" / projectId.show / "hooks" / hookIdAndUrl.id.show + val uri = uri"projects" / projectId / "hooks" / hookIdAndUrl.id val endpointName: NonEmptyString = "delete-hook" implicit val logger: TestLogger[IO] = TestLogger[IO]() implicit val gitLabClient: GitLabClient[IO] = mock[GitLabClient[IO]] - val hookDeletor = new ProjectHookDeletorImpl[IO] + val remover = new GLHookRemoverImpl[IO] lazy val mapResponse = captureMapping(gitLabClient)( - hookDeletor.delete(projectIds.generateOne, hookIdAndUrls.generateOne, accessTokens.generateOne).unsafeRunSync(), + remover.delete(projectIds.generateOne, hookIdAndUrls.generateOne, accessTokens.generateOne).unsafeRunSync(), Gen.const(DeletionResult.HookDeleted), method = DELETE ) val deletionResults: Gen[DeletionResult] = Gen.oneOf(DeletionResult.HookDeleted, DeletionResult.HookNotFound) } - - private implicit lazy val projectHooks: Gen[ProjectHook] = for { - projectId <- projectIds - hookUrl <- projectHookUrls - serializedHookToken <- serializedHookTokens - } yield ProjectHook(projectId, hookUrl, serializedHookToken) } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpointImplSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpointImplSpec.scala index f5f7ff94a7..c53264edf9 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpointImplSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletionEndpointImplSpec.scala @@ -34,7 +34,7 @@ import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Error import io.renku.testtools.IOSpec import io.renku.webhookservice.WebhookServiceGenerators.projectHookIds -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult.{HookDeleted, HookNotFound} +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult.{HookDeleted, HookNotFound} import io.renku.webhookservice.model.HookIdentifier import org.http4s.MediaType import org.http4s.Status.{InternalServerError, NotFound, Ok, Unauthorized} @@ -44,14 +44,15 @@ import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec class HookDeletionEndpointImplSpec extends AnyWordSpec with MockFactory with should.Matchers with IOSpec { + "deleteHook" should { "return OK when webhook is successfully deleted for project with the given id in GitLab" in new TestCase { - (hookDeletor + (hookRemover .deleteHook(_: HookIdentifier, _: AccessToken)) .expects(projectHookId, authUser.accessToken) - .returning(HookDeleted.pure[IO]) + .returning(HookDeleted.some.pure[IO]) val response = deleteHook(projectHookId.projectId, authUser).unsafeRunSync() @@ -61,25 +62,27 @@ class HookDeletionEndpointImplSpec extends AnyWordSpec with MockFactory with sho } - "return NOT_FOUND when hook was already deleted" in new TestCase { + HookNotFound.some :: None :: Nil foreach { deletionResult => + s"return NOT_FOUND when hook deletion returned $deletionResult" in new TestCase { - (hookDeletor - .deleteHook(_: HookIdentifier, _: AccessToken)) - .expects(projectHookId, authUser.accessToken) - .returning(HookNotFound.pure[IO]) + (hookRemover + .deleteHook(_: HookIdentifier, _: AccessToken)) + .expects(projectHookId, authUser.accessToken) + .returning(HookNotFound.some.pure[IO]) - val response = deleteHook(projectHookId.projectId, authUser).unsafeRunSync() + val response = deleteHook(projectHookId.projectId, authUser).unsafeRunSync() - response.status shouldBe NotFound - response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) - response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Hook already deleted"}""" + response.status shouldBe NotFound + response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) + response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Hook not found"}""" + } } "return INTERNAL_SERVER_ERROR when there was an error during hook deletion and log the error" in new TestCase { val errorMessage = ErrorMessage("some error") val exception = new Exception(errorMessage.toString()) - (hookDeletor + (hookRemover .deleteHook(_: HookIdentifier, _: AccessToken)) .expects(projectHookId, authUser.accessToken) .returning(IO.raiseError(exception)) @@ -95,7 +98,7 @@ class HookDeletionEndpointImplSpec extends AnyWordSpec with MockFactory with sho "return UNAUTHORIZED when there was an UnauthorizedException thrown during hook creation" in new TestCase { - (hookDeletor + (hookRemover .deleteHook(_: HookIdentifier, _: AccessToken)) .expects(projectHookId, authUser.accessToken) .returning(IO.raiseError(UnauthorizedException)) @@ -114,8 +117,8 @@ class HookDeletionEndpointImplSpec extends AnyWordSpec with MockFactory with sho val authUser = authUsers.generateOne implicit val logger: TestLogger[IO] = TestLogger[IO]() - val hookDeletor = mock[HookDeletor[IO]] + val hookRemover = mock[HookRemover[IO]] - val deleteHook = new HookDeletionEndpointImpl[IO](projectHookId.projectHookUrl, hookDeletor).deleteHook _ + val deleteHook = new HookDeletionEndpointImpl[IO](projectHookId.projectHookUrl, hookRemover).deleteHook _ } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletorSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookRemoverSpec.scala similarity index 76% rename from webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletorSpec.scala rename to webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookRemoverSpec.scala index 8548f61030..27ca4d450b 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookDeletorSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookdeletion/HookRemoverSpec.scala @@ -29,64 +29,78 @@ import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Error import io.renku.testtools.IOSpec import io.renku.webhookservice.WebhookServiceGenerators.{hookIdAndUrls, projectHookIds} -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult -import io.renku.webhookservice.hookdeletion.HookDeletor.DeletionResult.{HookDeleted, HookNotFound} +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult +import io.renku.webhookservice.hookdeletion.HookRemover.DeletionResult.{HookDeleted, HookNotFound} import io.renku.webhookservice.hookfetcher.ProjectHookFetcher import io.renku.webhookservice.hookfetcher.ProjectHookFetcher.HookIdAndUrl import org.scalamock.scalatest.MockFactory import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -class HookDeletorSpec extends AnyWordSpec with MockFactory with should.Matchers with IOSpec { +class HookRemoverSpec extends AnyWordSpec with MockFactory with should.Matchers with IOSpec { "deleteHook" should { "return HookeDeleted if hook exists and it was successfully deleted" in new TestCase { + val hookToDelete = hookIdAndUrls.generateOne.copy(url = projectHookId.projectHookUrl) val idsAndUrls = hookIdAndUrls.generateNonEmptyList().toList :+ hookToDelete (projectHookFetcher.fetchProjectHooks _) .expects(projectHookId.projectId, accessToken) - .returns(idsAndUrls.pure[IO]) + .returns(idsAndUrls.some.pure[IO]) - (projectHookDeletor + (glHookRemover .delete(_: GitLabId, _: HookIdAndUrl, _: AccessToken)) .expects(projectHookId.projectId, hookToDelete, accessToken) .returning(HookDeleted.pure[IO]) - hookDeletor.deleteHook(projectHookId, accessToken).unsafeRunSync() shouldBe HookDeleted + remover.deleteHook(projectHookId, accessToken).unsafeRunSync() shouldBe HookDeleted.some logger.expectNoLogs() } "return HookNotFound if hook was already removed for that project" in new TestCase { + val idsAndUrls = hookIdAndUrls.generateNonEmptyList().toList (projectHookFetcher.fetchProjectHooks _) .expects(projectHookId.projectId, accessToken) - .returns(idsAndUrls.pure[IO]) + .returns(idsAndUrls.some.pure[IO]) - hookDeletor.deleteHook(projectHookId, accessToken).unsafeRunSync() shouldBe HookNotFound + remover.deleteHook(projectHookId, accessToken).unsafeRunSync() shouldBe HookNotFound.some + + logger.expectNoLogs() + } + + "return None if hooks finding returns None" in new TestCase { + + (projectHookFetcher.fetchProjectHooks _) + .expects(projectHookId.projectId, accessToken) + .returns(None.pure[IO]) + + remover.deleteHook(projectHookId, accessToken).unsafeRunSync() shouldBe None logger.expectNoLogs() } "log an error if hook deletion fails" in new TestCase { + val hookToDelete = hookIdAndUrls.generateOne.copy(url = projectHookId.projectHookUrl) val idsAndUrls = hookIdAndUrls.generateNonEmptyList().toList :+ hookToDelete (projectHookFetcher.fetchProjectHooks _) .expects(projectHookId.projectId, accessToken) - .returns(idsAndUrls.pure[IO]) + .returns(idsAndUrls.some.pure[IO]) val exception = exceptions.generateOne - (projectHookDeletor + (glHookRemover .delete(_: GitLabId, _: HookIdAndUrl, _: AccessToken)) .expects(projectHookId.projectId, hookToDelete, accessToken) .returning(exception.raiseError[IO, DeletionResult]) intercept[Exception] { - hookDeletor.deleteHook(projectHookId, accessToken).unsafeRunSync() + remover.deleteHook(projectHookId, accessToken).unsafeRunSync() } shouldBe exception logger.loggedOnly(Error(s"Hook deletion failed for project with id ${projectHookId.projectId}", exception)) @@ -99,8 +113,8 @@ class HookDeletorSpec extends AnyWordSpec with MockFactory with should.Matchers implicit val logger: TestLogger[IO] = TestLogger[IO]() val projectHookFetcher = mock[ProjectHookFetcher[IO]] - val projectHookDeletor = mock[ProjectHookDeletor[IO]] + val glHookRemover = mock[GLHookRemover[IO]] - val hookDeletor = new HookDeletorImpl[IO](projectHookFetcher, projectHookDeletor) + val remover = new HookRemoverImpl[IO](projectHookFetcher, glHookRemover) } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcherSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcherSpec.scala index 5456288f21..b8d1a50bd8 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcherSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookfetcher/ProjectHookFetcherSpec.scala @@ -19,21 +19,19 @@ package io.renku.webhookservice.hookfetcher import cats.effect.IO -import cats.implicits.toShow +import cats.syntax.all._ import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import eu.timepit.refined.collection.NonEmpty import io.circe.literal._ -import io.circe.syntax.EncoderOps -import io.circe.{Encoder, Json} import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.model.GraphModelGenerators.projectIds import io.renku.http.client.RestClient.ResponseMappingF -import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.client.{AccessToken, GitLabClient} import io.renku.http.server.EndpointTester.jsonEntityEncoder +import io.renku.http.tinytypes.TinyTypeURIEncoder._ import io.renku.interpreters.TestLogger import io.renku.stubbing.ExternalServiceStubbing import io.renku.testtools.{GitLabClientTools, IOSpec} @@ -43,6 +41,7 @@ import org.http4s.implicits.http4sLiteralsSyntax import org.http4s.{Request, Response, Status, Uri} import org.scalacheck.Gen import org.scalamock.scalatest.MockFactory +import org.scalatest.OptionValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -52,52 +51,53 @@ class ProjectHookFetcherSpec with ExternalServiceStubbing with GitLabClientTools[IO] with should.Matchers + with OptionValues with IOSpec { "fetchProjectHooks" should { - "return the list of hooks of the project" in new TestCase { + "return list of project hooks" in new TestCase { val idAndUrls = hookIdAndUrls.toGeneratorOfNonEmptyList(2).generateOne.toList (gitLabClient - .get(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[IO, List[HookIdAndUrl]])(_: Option[AccessToken])) - .expects(uri, endpointName, *, Some(accessToken)) - .returning(IO.pure(idAndUrls)) - - fetcher.fetchProjectHooks(projectId, accessToken).unsafeRunSync() shouldBe idAndUrls + .get(_: Uri, _: String Refined NonEmpty)(_: ResponseMappingF[IO, Option[List[HookIdAndUrl]]])( + _: Option[AccessToken] + )) + .expects(uri, endpointName, *, accessToken.some) + .returning(idAndUrls.some.pure[IO]) + + fetcher + .fetchProjectHooks(projectId, accessToken) + .unsafeRunSync() + .value should contain theSameElementsAs idAndUrls } - // mapResponse - "return the list of hooks if the response is Ok" in new TestCase { - val id = nonNegativeInts().generateOne.value + + val id = positiveInts().generateOne.value val url = projectHookUrls.generateOne mapResponse((Status.Ok, Request(), Response().withEntity(json"""[{"id":$id, "url":${url.value}}]"""))) - .unsafeRunSync() shouldBe List(HookIdAndUrl(id.toString, url)) + .unsafeRunSync() shouldBe List(HookIdAndUrl(id, url)).some } "return an empty list of hooks if the project does not exists" in new TestCase { - - mapResponse(Status.NotFound, Request(), Response()).unsafeRunSync() shouldBe List.empty[HookIdAndUrl] + mapResponse(Status.NotFound, Request(), Response()).unsafeRunSync() shouldBe List.empty[HookIdAndUrl].some } - "return an UnauthorizedException if remote client responds with UNAUTHORIZED" in new TestCase { - - intercept[UnauthorizedException] { - mapResponse(Status.Unauthorized, Request(), Response()).unsafeRunSync() + Status.Unauthorized :: Status.Forbidden :: Nil foreach { status => + show"return None if remote client responds with $status" in new TestCase { + mapResponse(status, Request(), Response()).unsafeRunSync() shouldBe None } } - "return an Exception if remote client responds with status neither OK , NOT_FOUND nor UNAUTHORIZED" in new TestCase { - + "return an Exception if remote client responds with status any of OK , NOT_FOUND, UNAUTHORIZED or FORBIDDEN" in new TestCase { intercept[Exception] { mapResponse(Status.ServiceUnavailable, Request(), Response()).unsafeRunSync() } } "return a RuntimeException if remote client responds with unexpected body" in new TestCase { - intercept[RuntimeException] { mapResponse((Status.Ok, Request(), Response().withEntity("""{}"""))).unsafeRunSync() }.getMessage should include("Could not decode JSON") @@ -106,7 +106,7 @@ class ProjectHookFetcherSpec private trait TestCase { val projectId = projectIds.generateOne - val uri = uri"projects" / projectId.show / "hooks" + val uri = uri"projects" / projectId / "hooks" val endpointName: String Refined NonEmpty = "project-hooks" val accessToken = accessTokens.generateOne @@ -115,14 +115,7 @@ class ProjectHookFetcherSpec val fetcher = new ProjectHookFetcherImpl[IO] val mapResponse = captureMapping(gitLabClient)(fetcher.fetchProjectHooks(projectId, accessToken).unsafeRunSync(), - Gen.const(List.empty[HookIdAndUrl]) - ) - } - - private implicit val idsAndUrlsEncoder: Encoder[HookIdAndUrl] = Encoder.instance { idAndUrl => - Json.obj( - "id" -> idAndUrl.id.asJson, - "url" -> idAndUrl.url.value.asJson + Gen.const(Option.empty[List[HookIdAndUrl]]) ) } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpointSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpointSpec.scala index 4e62c0f54c..d76984bc81 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpointSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidationEndpointSpec.scala @@ -26,11 +26,10 @@ import io.circe.syntax._ import io.renku.generators.CommonGraphGenerators.authUsers import io.renku.generators.Generators.Implicits._ import io.renku.graph.model.GraphModelGenerators.projectIds -import io.renku.graph.model.projects.GitLabId +import io.renku.graph.model.projects import io.renku.http.ErrorMessage import io.renku.http.ErrorMessage._ import io.renku.http.client.AccessToken -import io.renku.http.client.RestClientError.UnauthorizedException import io.renku.http.server.EndpointTester._ import io.renku.interpreters.TestLogger import io.renku.interpreters.TestLogger.Level.Error @@ -50,63 +49,61 @@ class HookValidationEndpointSpec extends AnyWordSpec with MockFactory with shoul "return OK when the hook exists for the project with the given id" in new TestCase { (hookValidator - .validateHook(_: GitLabId, _: Option[AccessToken])) + .validateHook(_: projects.GitLabId, _: Option[AccessToken])) .expects(projectId, Some(authUser.accessToken)) - .returning(HookExists.pure[IO]) + .returning(HookExists.some.pure[IO]) - val response = validateHook(projectId, authUser).unsafeRunSync() + val response = endpoint.validateHook(projectId, authUser).unsafeRunSync() response.status shouldBe Ok response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Hook valid"}""" } - "return NOT_FOUND the hook does not exist" in new TestCase { + "return NOT_FOUND when the hook does not exist" in new TestCase { (hookValidator - .validateHook(_: GitLabId, _: Option[AccessToken])) + .validateHook(_: projects.GitLabId, _: Option[AccessToken])) .expects(projectId, Some(authUser.accessToken)) - .returning(HookMissing.pure[IO]) + .returning(HookMissing.some.pure[IO]) - val response = validateHook(projectId, authUser).unsafeRunSync() + val response = endpoint.validateHook(projectId, authUser).unsafeRunSync() response.status shouldBe NotFound response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Hook not found"}""" } - "return INTERNAL_SERVER_ERROR when there was an error during hook validation and log the error" in new TestCase { + "return UNAUTHORIZED when validation cannot determine hook existence" in new TestCase { - val errorMessage = ErrorMessage("some error") - private val exception = new Exception(errorMessage.toString()) (hookValidator - .validateHook(_: GitLabId, _: Option[AccessToken])) + .validateHook(_: projects.GitLabId, _: Option[AccessToken])) .expects(projectId, Some(authUser.accessToken)) - .returning(IO.raiseError(exception)) + .returning(None.pure[IO]) - val response = validateHook(projectId, authUser).unsafeRunSync() + val response = endpoint.validateHook(projectId, authUser).unsafeRunSync() - response.status shouldBe InternalServerError + response.status shouldBe Unauthorized response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) - response.as[Json].unsafeRunSync() shouldBe errorMessage.asJson - - logger.loggedOnly( - Error(exception.getMessage, exception) - ) + response.as[Json].unsafeRunSync() shouldBe json"""{"message": "Unauthorized"}""" } - "return UNAUTHORIZED when there was an UnauthorizedException thrown during hook validation" in new TestCase { + "return INTERNAL_SERVER_ERROR when there was an error during hook validation and log the error" in new TestCase { + val errorMessage = ErrorMessage("some error") + private val exception = new Exception(errorMessage.toString()) (hookValidator - .validateHook(_: GitLabId, _: Option[AccessToken])) + .validateHook(_: projects.GitLabId, _: Option[AccessToken])) .expects(projectId, Some(authUser.accessToken)) - .returning(IO.raiseError(UnauthorizedException)) + .returning(exception.raiseError[IO, Nothing]) - val response = validateHook(projectId, authUser).unsafeRunSync() + val response = endpoint.validateHook(projectId, authUser).unsafeRunSync() - response.status shouldBe Unauthorized + response.status shouldBe InternalServerError response.contentType shouldBe Some(`Content-Type`(MediaType.application.json)) - response.as[Json].unsafeRunSync() shouldBe ErrorMessage(UnauthorizedException).asJson + response.as[Json].unsafeRunSync() shouldBe errorMessage.asJson + + logger.loggedOnly(Error(exception.getMessage, exception)) } } @@ -117,6 +114,6 @@ class HookValidationEndpointSpec extends AnyWordSpec with MockFactory with shoul implicit val logger = TestLogger[IO]() val hookValidator = mock[HookValidator[IO]] - val validateHook = new HookValidationEndpointImpl[IO](hookValidator).validateHook _ + val endpoint = new HookValidationEndpointImpl[IO](hookValidator) } } diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidatorSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidatorSpec.scala index 7a9a76b349..c002286dd5 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidatorSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/HookValidatorSpec.scala @@ -22,21 +22,20 @@ import cats.syntax.all._ import io.renku.generators.CommonGraphGenerators._ import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators.exceptions -import io.renku.graph.model.projects.GitLabId import io.renku.graph.model.GraphModelGenerators.projectIds +import io.renku.graph.model.projects.GitLabId import io.renku.graph.tokenrepository.AccessTokenFinder import io.renku.http.client.AccessToken import io.renku.interpreters.TestLogger -import io.renku.interpreters.TestLogger.Level.{Error, Info} +import io.renku.interpreters.TestLogger.Level.Error import io.renku.webhookservice.WebhookServiceGenerators._ import io.renku.webhookservice.hookvalidation.HookValidator.HookValidationResult.{HookExists, HookMissing} -import io.renku.webhookservice.hookvalidation.HookValidator.NoAccessTokenException import io.renku.webhookservice.model.HookIdentifier import io.renku.webhookservice.tokenrepository.{AccessTokenAssociator, AccessTokenRemover} import org.scalamock.scalatest.MockFactory +import org.scalatest.TryValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec -import org.scalatest.TryValues import scala.util.Try @@ -45,7 +44,16 @@ class HookValidatorSpec extends AnyWordSpec with MockFactory with should.Matcher "validateHook - finding access token" should { - "fail if finding stored access token fails when no access token given" in new TestCase { + "return None if finding stored access token returns None when no access token given" in new TestCase { + + givenAccessTokenFinding(returning = Option.empty[AccessToken].pure[Try]) + + validator.validateHook(projectId, maybeAccessToken = None) shouldBe None.pure[Try] + + logger.expectNoLogs() + } + + "fail if finding stored access token fails and no access token given" in new TestCase { val exception = exceptions.generateOne givenAccessTokenFinding(returning = exception.raiseError[Try, Option[AccessToken]]) @@ -57,91 +65,122 @@ class HookValidatorSpec extends AnyWordSpec with MockFactory with should.Matcher logger.loggedOnly(Error(s"Hook validation failed for projectId $projectId", result.failure.exception)) } + } - "fail if finding stored access token return None when no access token given" in new TestCase { + "validateHook - given access token" should { - givenAccessTokenFinding(returning = Option.empty[AccessToken].pure[Try]) + "re-associate access token and succeed with HookExists if there's a valid hook" in new TestCase { - val noAccessTokenException = NoAccessTokenException(s"No stored access token found for projectId $projectId") - validator.validateHook(projectId, maybeAccessToken = None) shouldBe noAccessTokenException - .raiseError[Try, Nothing] + givenTokenAssociation(givenAccessToken, returning = ().pure[Try]) - logger.loggedOnly(Info(s"Hook validation failed: ${noAccessTokenException.getMessage}")) - } - } + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) - "validateHook - given access token" should { + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = true.some.pure[Try] + ) + + validator.validateHook(projectId, givenAccessToken.some) shouldBe HookExists.some.pure[Try] - "succeed with HookExists and re-associate access token if there's a hook" in new TestCase { + logger.expectNoLogs() + } - givenHookVerification(HookIdentifier(projectId, projectHookUrl), givenAccessToken, returning = true.pure[Try]) + "re-associate access token, delete the access token and succeed with HookMissing if there's no hook" in new TestCase { givenTokenAssociation(givenAccessToken, returning = ().pure[Try]) - validator.validateHook(projectId, givenAccessToken.some) shouldBe HookExists.pure[Try] + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) + + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = false.some.pure[Try] + ) + + givenTokenRemoving(returning = ().pure[Try]) + + validator.validateHook(projectId, givenAccessToken.some) shouldBe HookMissing.some.pure[Try] logger.expectNoLogs() } - "succeed with HookMissing and delete the access token if there's no hook" in new TestCase { + "return None if hook verification cannot determine hook existence" in new TestCase { - givenHookVerification(HookIdentifier(projectId, projectHookUrl), givenAccessToken, returning = false.pure[Try]) + givenTokenAssociation(givenAccessToken, returning = ().pure[Try]) - givenTokenRemoving(returning = ().pure[Try]) + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) + + givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = None.pure[Try]) - validator.validateHook(projectId, givenAccessToken.some) shouldBe HookMissing.pure[Try] + validator.validateHook(projectId, givenAccessToken.some) shouldBe None.pure[Try] logger.expectNoLogs() } "fail if hook verification step fails" in new TestCase { + givenTokenAssociation(givenAccessToken, returning = ().pure[Try]) + + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) + val exception = exceptions.generateOne val error = exception.raiseError[Try, Nothing] - givenHookVerification(HookIdentifier(projectId, projectHookUrl), givenAccessToken, returning = error) + givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = error) - validator.validateHook(projectId, Some(givenAccessToken)) shouldBe error + validator.validateHook(projectId, givenAccessToken.some) shouldBe error logger.loggedOnly(Error(s"Hook validation failed for projectId $projectId", exception)) } "fail if access token re-association fails" in new TestCase { - givenHookVerification(HookIdentifier(projectId, projectHookUrl), givenAccessToken, returning = true.pure[Try]) - val exception = exceptions.generateOne val error = exception.raiseError[Try, Nothing] givenTokenAssociation(givenAccessToken, returning = error) - validator.validateHook(projectId, Some(givenAccessToken)) shouldBe error + validator.validateHook(projectId, givenAccessToken.some) shouldBe error logger.loggedOnly(Error(s"Hook validation failed for projectId $projectId", exception)) } "fail if access token removal fails" in new TestCase { - givenHookVerification(HookIdentifier(projectId, projectHookUrl), givenAccessToken, returning = false.pure[Try]) + givenTokenAssociation(givenAccessToken, returning = ().pure[Try]) + + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) + + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = false.some.pure[Try] + ) val exception = exceptions.generateOne val error = exception.raiseError[Try, Nothing] givenTokenRemoving(returning = error) - validator.validateHook(projectId, Some(givenAccessToken)) shouldBe error + validator.validateHook(projectId, givenAccessToken.some) shouldBe error logger.loggedOnly(Error(s"Hook validation failed for projectId $projectId", exception)) } } - "validateHook - given valid stored access token" should { + "validateHook - given stored access token but no given token" should { "succeed with HookExists if there's a hook" in new TestCase { val storedAccessToken = accessTokens.generateOne givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) - givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = true.pure[Try]) + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = true.some.pure[Try] + ) - validator.validateHook(projectId, None) shouldBe HookExists.pure[Try] + validator.validateHook(projectId, maybeAccessToken = None) shouldBe HookExists.some.pure[Try] logger.expectNoLogs() } @@ -151,11 +190,26 @@ class HookValidatorSpec extends AnyWordSpec with MockFactory with should.Matcher val storedAccessToken = accessTokens.generateOne givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) - givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = false.pure[Try]) + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = false.some.pure[Try] + ) givenTokenRemoving(returning = ().pure[Try]) - validator.validateHook(projectId, maybeAccessToken = None) shouldBe HookMissing.pure[Try] + validator.validateHook(projectId, maybeAccessToken = None) shouldBe HookMissing.some.pure[Try] + + logger.expectNoLogs() + } + + "return None if hook verification cannot determine hook existence" in new TestCase { + + val storedAccessToken = accessTokens.generateOne + givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) + + givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = None.pure[Try]) + + validator.validateHook(projectId, maybeAccessToken = None) shouldBe None.pure[Try] logger.expectNoLogs() } @@ -179,7 +233,10 @@ class HookValidatorSpec extends AnyWordSpec with MockFactory with should.Matcher val storedAccessToken = accessTokens.generateOne givenAccessTokenFinding(returning = storedAccessToken.some.pure[Try]) - givenHookVerification(HookIdentifier(projectId, projectHookUrl), storedAccessToken, returning = false.pure[Try]) + givenHookVerification(HookIdentifier(projectId, projectHookUrl), + storedAccessToken, + returning = false.some.pure[Try] + ) val exception = exceptions.generateOne val error = exception.raiseError[Try, Nothing] @@ -215,7 +272,7 @@ class HookValidatorSpec extends AnyWordSpec with MockFactory with should.Matcher .expects(projectId, projectIdToPath) .returning(returning) - def givenHookVerification(identifier: HookIdentifier, accessToken: AccessToken, returning: Try[Boolean]) = + def givenHookVerification(identifier: HookIdentifier, accessToken: AccessToken, returning: Try[Option[Boolean]]) = (projectHookVerifier .checkHookPresence(_: HookIdentifier, _: AccessToken)) .expects(identifier, accessToken) diff --git a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifierSpec.scala b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifierSpec.scala index 2f8d99c836..64e227c8da 100644 --- a/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifierSpec.scala +++ b/webhook-service/src/test/scala/io/renku/webhookservice/hookvalidation/ProjectHookVerifierSpec.scala @@ -42,36 +42,47 @@ class ProjectHookVerifierSpec "checkHookPresence" should { - "return true if there's a hook with url pointing to expected project hook url - personal access token case" in new TestCase { - val idsAndUrls = hookIdAndUrls.generateNonEmptyList().toList :+ HookIdAndUrl(nonEmptyStrings().generateOne, - projectHookId.projectHookUrl - ) + "return true if there's a hook with url pointing to expected project hook url" in new TestCase { + + val idsAndUrls = + hookIdAndUrls.generateList() :+ HookIdAndUrl(positiveInts().generateOne.value, projectHookId.projectHookUrl) val accessToken = accessTokens.generateOne - (projectHookFetcher.fetchProjectHooks _).expects(projectId, accessToken).returns(idsAndUrls.pure[IO]) + (projectHookFetcher.fetchProjectHooks _).expects(projectId, accessToken).returns(idsAndUrls.some.pure[IO]) - verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() shouldBe true + verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() shouldBe true.some } "return false if there's no hook with url pointing to expected project hook url" in new TestCase { + val idsAndUrls = hookIdAndUrls.generateNonEmptyList().toList val accessToken = accessTokens.generateOne - (projectHookFetcher.fetchProjectHooks _).expects(projectId, accessToken).returns(idsAndUrls.pure[IO]) + (projectHookFetcher.fetchProjectHooks _).expects(projectId, accessToken).returns(idsAndUrls.some.pure[IO]) - verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() shouldBe false + verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() shouldBe false.some } - "return an UnauthorizedException if remote client responds with UNAUTHORIZED" in new TestCase { + "return None if finding hooks returns None" in new TestCase { + + val accessToken = accessTokens.generateOne + (projectHookFetcher.fetchProjectHooks _) + .expects(projectId, accessToken) + .returns(None.pure[IO]) + + verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() shouldBe None + } + + "fail if finding hooks fails" in new TestCase { + val exception = exceptions.generateOne val accessToken = accessTokens.generateOne (projectHookFetcher.fetchProjectHooks _) .expects(projectId, accessToken) - .returns(exception.raiseError[IO, List[HookIdAndUrl]]) + .returns(exception.raiseError[IO, Nothing]) intercept[Exception] { verifier.checkHookPresence(projectHookId, accessToken).unsafeRunSync() }.getMessage shouldBe exception.getMessage } - } private trait TestCase {