From 52286aa761517b65cb560ed1310def22bd8d636f Mon Sep 17 00:00:00 2001 From: RenkuBot <53332360+RenkuBot@users.noreply.github.com> Date: Tue, 7 Nov 2023 08:46:08 +0100 Subject: [PATCH 01/13] chore: Update fs2-core from 3.9.2 to 3.9.3 (#1779) Co-authored-by: RenkuBot --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 34738c6402..a123ddc0be 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -14,7 +14,7 @@ object Dependencies { val circeGenericExtras = "0.14.3" val circeOptics = "0.15.0" val diffx = "0.9.0" - val fs2 = "3.9.2" + val fs2 = "3.9.3" val http4s = "0.23.23" val http4sEmber = "0.23.23" val http4sPrometheus = "0.24.6" From 51b4ef8ce16cdf0d7254d8264f8ebd2ca342cb2b Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Tue, 7 Nov 2023 15:06:04 +0100 Subject: [PATCH 02/13] feat: Cross-Entity Search to allow filtering by owner (#1780) --- .../io/renku/entities/search/Criteria.scala | 5 +- .../renku/entities/search/DatasetsQuery.scala | 58 +++--- .../entities/search/EntitiesFinder.scala | 2 +- .../renku/entities/search/EntityQuery.scala | 4 +- .../renku/entities/search/PersonsQuery.scala | 67 ++++--- .../renku/entities/search/ProjectsQuery.scala | 28 ++- .../entities/search/WorkflowsQuery.scala | 43 +++-- .../io/renku/entities/search/package.scala | 6 +- .../entities/search/EntitiesFinderSpec.scala | 172 +++++++++++++++++- .../io/renku/entities/search/Generators.scala | 10 +- .../searchgraphs/SearchInfoDatasets.scala | 3 +- knowledge-graph/README.md | 13 +- .../knowledgegraph/MicroserviceRoutes.scala | 45 +++-- .../knowledgegraph/QueryParamDecoders.scala | 6 +- .../entities/EndpointDocs.scala | 10 +- .../MicroserviceRoutesSpec.scala | 40 +++- .../projectauth/util/SparqlSnippets.scala | 13 +- .../DatasetEntitiesGenerators.scala | 9 +- .../ProjectEntitiesGenerators.scala | 8 +- 19 files changed, 415 insertions(+), 127 deletions(-) diff --git a/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala b/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala index 549cb5bcb3..d79224b46f 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala @@ -42,6 +42,7 @@ object Criteria { final case class Filters(maybeQuery: Option[Filters.Query] = None, entityTypes: Set[Filters.EntityType] = Set.empty, creators: Set[persons.Name] = Set.empty, + maybeOwned: Option[Filters.Owned] = None, visibilities: Set[projects.Visibility] = Set.empty, namespaces: Set[projects.Namespace] = Set.empty, maybeSince: Option[Filters.Since] = None, @@ -53,7 +54,9 @@ object Criteria { final class Query private (val value: String) extends AnyVal with StringTinyType object Query extends TinyTypeFactory[Query](new Query(_)) with NonBlank[Query] - sealed trait EntityType extends StringTinyType with Product with Serializable + final case class Owned(userId: persons.GitLabId) + + sealed trait EntityType extends StringTinyType with Product object EntityType extends TinyTypeFactory[EntityType](EntityTypeApply) { final case object Project extends EntityType { override val value: String = "project" } diff --git a/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala index 7ce2ff4ae2..50aec95dbd 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala @@ -25,7 +25,6 @@ import io.renku.entities.search.Criteria.Filters import io.renku.entities.search.model.{Entity, MatchingScore} import io.renku.entities.searchgraphs.concatSeparator import io.renku.graph.model._ -import io.renku.graph.model.projects.Visibility import io.renku.http.server.security.model.AuthUser import io.renku.projectauth.util.SparqlSnippets import io.renku.triplesstore.client.sparql.{Fragment, LuceneQuery, VarName} @@ -34,22 +33,23 @@ import io.renku.triplesstore.client.syntax._ object DatasetsQuery extends EntityQuery[Entity.Dataset] { override val entityType: Filters.EntityType = Filters.EntityType.Dataset - val matchingScoreVar = VarName("matchingScore") - val nameVar = VarName("name") - val idsSlugsVisibilitiesVar = VarName("idsSlugsVisibilities") - val sameAsVar = VarName("sameAs") - val maybeDateCreatedVar = VarName("maybeDateCreated") - val maybeDatePublishedVar = VarName("maybeDatePublished") - val maybeDateModified = VarName("maybeDateModified") - val dateVar = VarName("date") - val creatorsNamesVar = VarName("creatorsNames") - val maybeDescriptionVar = VarName("maybeDescription") - val keywordsVar = VarName("keywords") - val imagesVar = VarName("images") + private val matchingScoreVar = VarName("matchingScore") + private val nameVar = VarName("name") + private val idsSlugsVisibilitiesVar = VarName("idsSlugsVisibilities") + private val sameAsVar = VarName("sameAs") + private val maybeDateCreatedVar = VarName("maybeDateCreated") + private val maybeDatePublishedVar = VarName("maybeDatePublished") + private val maybeDateModified = VarName("maybeDateModified") + private val dateVar = VarName("date") + private val creatorsNamesVar = VarName("creatorsNames") + private val maybeDescriptionVar = VarName("maybeDescription") + private val keywordsVar = VarName("keywords") + private val imagesVar = VarName("images") - private val authSnippets = SparqlSnippets(VarName("projId")) + private val projIdVar = VarName("projId") + private val authSnippets = SparqlSnippets(projIdVar) - override val selectVariables = Set( + override val selectVariables: Set[String] = Set( entityTypeVar, matchingScoreVar, nameVar, @@ -65,7 +65,7 @@ object DatasetsQuery extends EntityQuery[Entity.Dataset] { imagesVar ).map(_.name) - override def query(criteria: Criteria): Option[String] = + override def query(criteria: Criteria): Option[Fragment] = criteria.filters.whenRequesting(entityType) { fr"""{ |SELECT DISTINCT $entityTypeVar @@ -111,7 +111,7 @@ object DatasetsQuery extends EntityQuery[Entity.Dataset] { | ${namespacesPart(criteria.filters.namespaces)} | | # access restriction - | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters.visibilities)} + | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters)} | } | } | }# end sub select @@ -130,14 +130,26 @@ object DatasetsQuery extends EntityQuery[Entity.Dataset] { | } |} |} - |""".stripMargin.sparql + |""".stripMargin } - private def accessRightsAndVisibility(maybeUser: Option[AuthUser], visibilities: Set[Visibility]): Fragment = - sparql""" - |?projId renku:projectVisibility ?visibility . - |${authSnippets.visibleProjects(maybeUser.map(_.id), visibilities)} - """ + private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = + filters.maybeOwned match { + case Some(Criteria.Filters.Owned(ownerId)) => + fr"""|$projIdVar renku:projectVisibility ?visibility . + |${visibilitiesPart(filters.visibilities)} + |${authSnippets.ownedProjects(ownerId)} + |""".stripMargin + case None => + fr"""|$projIdVar renku:projectVisibility ?visibility . + |${authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities)} + |""".stripMargin + } + + private lazy val visibilitiesPart: Set[projects.Visibility] => Fragment = { + case vis if vis.nonEmpty => fr"""VALUES (?visibility) {${vis.map(_.value)}}.""" + case _ => Fragment.empty + } private def resolveProject: Fragment = fr"""| $sameAsVar a renku:DiscoverableDataset; diff --git a/entities-search/src/main/scala/io/renku/entities/search/EntitiesFinder.scala b/entities-search/src/main/scala/io/renku/entities/search/EntitiesFinder.scala index 0504254b16..bf99e6dea4 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/EntitiesFinder.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/EntitiesFinder.scala @@ -65,7 +65,7 @@ private class EntitiesFinderImpl[F[_]: Async: NonEmptyParallel: Logger: SparqlQu Prefixes of (prov -> "prov", renku -> "renku", schema -> "schema", text -> "text", xsd -> "xsd"), s"""|SELECT ${entityQueries.map(_.selectVariables).combineAll.mkString(" ")} |WHERE { - | ${entityQueries.flatMap(_.query(criteria)).mkString(" UNION ")} + | ${entityQueries.flatMap(_.query(criteria)).map(_.sparql).mkString(" UNION ")} |} |${`ORDER BY`(criteria.sorting)} |""".stripMargin diff --git a/entities-search/src/main/scala/io/renku/entities/search/EntityQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/EntityQuery.scala index f5aad74629..4db8269f7c 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/EntityQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/EntityQuery.scala @@ -21,12 +21,12 @@ package io.renku.entities.search import io.circe.Decoder import io.renku.entities.search.Criteria.Filters.EntityType import io.renku.triplesstore.ResultsDecoder -import io.renku.triplesstore.client.sparql.VarName +import io.renku.triplesstore.client.sparql.{Fragment, VarName} private[entities] trait EntityQuery[+E <: model.Entity] extends ResultsDecoder with EntityQueryVars { val entityType: EntityType val selectVariables: Set[String] - def query(criteria: Criteria): Option[String] + def query(criteria: Criteria): Option[Fragment] def decoder[EE >: E]: Decoder[EE] def getDecoder[EE >: E](entityType: EntityType): Option[Decoder[EE]] = diff --git a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala index c86b21e02c..45a5af1828 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala @@ -21,43 +21,58 @@ package io.renku.entities.search import io.circe.Decoder import io.renku.entities.search.Criteria.Filters.EntityType import io.renku.entities.search.model.{Entity, MatchingScore} +import io.renku.graph.model.entities.Person.gitLabSameAsAdditionalType import io.renku.graph.model.{GraphClass, persons} -import io.renku.triplesstore.client.sparql.VarName +import io.renku.triplesstore.client.sparql.{Fragment, VarName} +import io.renku.triplesstore.client.syntax._ private case object PersonsQuery extends EntityQuery[model.Entity.Person] { override val entityType: EntityType = EntityType.Person - override val selectVariables = Set("?entityType", "?matchingScore", "?name") + override val selectVariables: Set[String] = Set("?entityType", "?matchingScore", "?name") - override def query(criteria: Criteria) = + override def query(criteria: Criteria): Option[Fragment] = (criteria.filters whenRequesting (entityType, criteria.filters.withNoOrPublicVisibility, criteria.filters.namespaces.isEmpty, criteria.filters.maybeSince.isEmpty, criteria.filters.maybeUntil.isEmpty)) { import criteria._ - // format: off - s"""|{ - | SELECT DISTINCT ?entityType ?matchingScore ?name - | WHERE { - | { - | SELECT (SAMPLE(?id) AS ?personId) ?name (MAX(?score) AS ?matchingScore) - | WHERE { - | ${filters.onQuery( - s"""(?id ?score) text:query (schema:name '${filters.query.query}').""", - matchingScoreVariableName = "?score")} - | GRAPH <${GraphClass.Persons.id}> { - | ?id a schema:Person; - | schema:name ?name - | } - | ${filters.maybeOnCreatorName(VarName("name")).sparql} - | } - | GROUP BY ?name - | } - | BIND ('person' AS ?entityType) - | } - |} - |""".stripMargin - // format: on + sparql"""|{ + | SELECT DISTINCT ?entityType ?matchingScore ?name + | WHERE { + | { + | SELECT (SAMPLE(?id) AS ?personId) ?name (MAX(?score) AS ?matchingScore) + | WHERE { + | ${textPart(criteria.filters)} + | + | GRAPH ${GraphClass.Persons.id} { + | ?id a schema:Person; + | schema:name ?name. + | ${filterOnOwned(criteria.filters.maybeOwned)} + | } + | ${filters.maybeOnCreatorName(VarName("name"))} + | } + | GROUP BY ?name + | } + | BIND ('person' AS ?entityType) + | } + |} + |""".stripMargin } + private def textPart(filters: Criteria.Filters) = + filters.onQuery( + snippet = fr"""(?id ?score) text:query (schema:name ${filters.query.query.asTripleObject}).""", + matchingScoreVariableName = VarName("score") + ) + + private lazy val filterOnOwned: Option[Criteria.Filters.Owned] => Fragment = { + case Some(Criteria.Filters.Owned(userId)) => + fr"""|?id schema:sameAs ?sameAsId. + |?sameAsId schema:additionalType ${gitLabSameAsAdditionalType.asTripleObject}; + | schema:identifier ${userId.asObject}. + |""".stripMargin + case None => Fragment.empty + } + override def decoder[EE >: Entity.Person]: Decoder[EE] = { implicit cursor => import io.renku.tinytypes.json.TinyTypeDecoders._ diff --git a/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala index 6a666e264d..220b4cffdc 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala @@ -63,7 +63,7 @@ private case object ProjectsQuery extends EntityQuery[model.Entity.Project] { imagesVar ).map(_.name) - override def query(criteria: Criteria): Option[String] = (criteria.filters whenRequesting entityType) { + override def query(criteria: Criteria): Option[Fragment] = (criteria.filters whenRequesting entityType) { import criteria._ sparql"""|{ | SELECT DISTINCT $entityTypeVar $matchingScoreVar $nameVar $slugVar $visibilityVar @@ -87,7 +87,7 @@ private case object ProjectsQuery extends EntityQuery[model.Entity.Project] { | | ${filters.maybeOnDateCreated(maybeDateCreatedVar)} | - | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters.visibilities)} + | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters)} | | GRAPH $projectIdVar { | ${namespacesPart(criteria.filters.namespaces)} @@ -107,7 +107,7 @@ private case object ProjectsQuery extends EntityQuery[model.Entity.Project] { | } | } |} - |""".stripMargin.sparql + |""".stripMargin } private def textQueryPart: Option[Criteria.Filters.Query] => Fragment = { @@ -138,11 +138,23 @@ private case object ProjectsQuery extends EntityQuery[model.Entity.Project] { |""".stripMargin } - private def accessRightsAndVisibility(maybeUser: Option[AuthUser], visibilities: Set[projects.Visibility]): Fragment = - sparql""" - |$projectIdVar renku:projectVisibility $visibilityVar . - |${authSnippets.visibleProjects(maybeUser.map(_.id), visibilities)} - """.stripMargin + private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = + filters.maybeOwned match { + case Some(Criteria.Filters.Owned(ownerId)) => + fr"""|$projectIdVar renku:projectVisibility $visibilityVar . + |${visibilitiesPart(filters.visibilities)} + |${authSnippets.ownedProjects(ownerId)} + |""".stripMargin + case None => + fr"""|$projectIdVar renku:projectVisibility $visibilityVar . + |${authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities)} + |""".stripMargin + } + + private lazy val visibilitiesPart: Set[projects.Visibility] => Fragment = { + case vis if vis.nonEmpty => fr"""VALUES ($visibilityVar) {${vis.map(_.value)}}.""" + case _ => Fragment.empty + } private def namespacesPart(ns: Set[projects.Namespace]): Fragment = { val matchFrag = diff --git a/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala index c55b0983f2..0b6c3255a3 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala @@ -35,20 +35,20 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { override val entityType: EntityType = EntityType.Workflow - override val selectVariables = Set("?entityType", - "?matchingScore", - "?wkId", - "?name", - "?date", - "?maybeDescription", - "?keywords", - "?projectIdVisibilities", - "?workflowTypes" + override val selectVariables: Set[String] = Set("?entityType", + "?matchingScore", + "?wkId", + "?name", + "?date", + "?maybeDescription", + "?keywords", + "?projectIdVisibilities", + "?workflowTypes" ) private val authSnippets = SparqlSnippets(VarName("projectId")) - override def query(criteria: Criteria) = (criteria.filters whenRequesting entityType) { + override def query(criteria: Criteria): Option[Fragment] = (criteria.filters whenRequesting entityType) { import criteria._ // format: off sparql"""|{ @@ -86,7 +86,7 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { | FILTER (IF (BOUND(?childProjectsIds), !CONTAINS(STR(?childProjectsIds), STR(?projectId)), true)) | FILTER (IF (BOUND(?projectsIdsWhereInvalidated), !CONTAINS(STR(?projectsIdsWhereInvalidated), STR(?projectId)), true)) | - | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters.visibilities)} + | ${accessRightsAndVisibility(criteria.maybeUser, criteria.filters)} | | GRAPH ?projectId { | ?wkId a prov:Plan; @@ -96,6 +96,7 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { | ^renku:hasPlan ?projectId. | ?projectId renku:projectNamespace ?namespace. | ?projectId renku:projectVisibility ?visibility. + | ${maybeFilterOnVisibilities(filters.visibilities)} | BIND (CONCAT(STR(?projectId), STR('::'), STR(?visibility)) AS ?projectIdVisibility) | ${filters.maybeOnNamespace(VarName("namespace"))} | ${filters.maybeOnDateCreated(VarName("date"))} @@ -108,12 +109,22 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { | BIND ('workflow' AS ?entityType) | } |} - |""".stripMargin.sparql + |""".stripMargin // format: on } - private def accessRightsAndVisibility(maybeUser: Option[AuthUser], visibilities: Set[projects.Visibility]): Fragment = - authSnippets.visibleProjects(maybeUser.map(_.id), visibilities) + private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = + filters.maybeOwned match { + case Some(Criteria.Filters.Owned(ownerId)) => + authSnippets.ownedProjects(ownerId) + case None => + authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities) + } + + private lazy val maybeFilterOnVisibilities: Set[projects.Visibility] => Fragment = { + case vis if vis.nonEmpty => fr"""VALUES (?visibility) {${vis.map(_.value)}}.""" + case _ => Fragment.empty + } private def withoutQuerySnippet: Fragment = sparql""" @@ -124,7 +135,7 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { |} |""".stripMargin - def withQuerySnippet(query: LuceneQuery) = + private def withQuerySnippet(query: LuceneQuery) = sparql""" |{ | SELECT ?wkId (MAX(?score) AS ?matchingScore) (SAMPLE(?projId) AS ?projectId) @@ -186,7 +197,7 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { } yield Entity.Workflow(matchingScore, name, visibility, dateCreated, keywords, maybeDescription, workflowTypes) } - def workflowTypeFromString(str: String): Either[String, WorkflowType] = + private def workflowTypeFromString(str: String): Either[String, WorkflowType] = str match { case _ if str.contains(CompositePlan.Ontology.typeDef.clazz.id.show) => Right(WorkflowType.Composite) diff --git a/entities-search/src/main/scala/io/renku/entities/search/package.scala b/entities-search/src/main/scala/io/renku/entities/search/package.scala index 17280ba042..99b85d810e 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/package.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/package.scala @@ -35,7 +35,7 @@ package object search { lazy val query: LuceneQuery = filters.maybeQuery.map(q => LuceneQuery.fuzzy(q.value)).getOrElse(LuceneQuery.queryAll) - def whenRequesting(entityType: Filters.EntityType, predicates: Boolean*)(query: => String): Option[String] = { + def whenRequesting(entityType: Filters.EntityType, predicates: Boolean*)(query: => Fragment): Option[Fragment] = { val typeMatching = filters.entityTypes match { case t if t.isEmpty => true case t => t contains entityType @@ -43,8 +43,8 @@ package object search { Option.when(typeMatching && predicates.forall(_ == true))(query) } - def onQuery(snippet: String, matchingScoreVariableName: String = "?matchingScore"): String = - foldQuery(_ => snippet, s"BIND (xsd:float(1.0) AS $matchingScoreVariableName)") + def onQuery(snippet: Fragment, matchingScoreVariableName: VarName = VarName("matchingScore")): Fragment = + foldQuery(_ => snippet, fr"BIND (xsd:float(1.0) AS $matchingScoreVariableName)") def foldQuery[A](ifPresent: LuceneQuery => A, ifMissing: => A): A = if (query.isQueryAll) ifMissing diff --git a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala index 82a824cafe..8c11eb4072 100644 --- a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala +++ b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala @@ -35,7 +35,7 @@ import io.renku.generators.Generators._ import io.renku.graph.model._ import io.renku.graph.model.projects.Visibility import io.renku.graph.model.testentities.generators.EntitiesGenerators -import io.renku.graph.model.testentities.{Dataset, StepPlan} +import io.renku.graph.model.testentities.{Dataset, Project, StepPlan} import io.renku.graph.model.tools.AdditionalMatchers import io.renku.http.rest.paging.PagingRequest import io.renku.http.rest.paging.model._ @@ -544,6 +544,176 @@ class EntitiesFinderSpec } } + "findEntities - with owned filter" should { + + "return project entities where the given user is an owner" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val project = renkuProjectEntities(anyVisibility) + .modify(replaceMembers(Set(owner))) + .generateOne + + val results = IOBody { + provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> + finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + } + + val expected = List( + project.to[model.Entity.Project], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return project entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val publicProject = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(owner))) + .generateOne + val nonPublicProject = renkuProjectEntities(visibilityNonPublic) + .modify(replaceMembers(Set(owner))) + .generateOne + + val results = IOBody { + provisionTestProjects(publicProject, nonPublicProject) *> + finder.findEntities( + Criteria(Filters(maybeOwned = Owned(ownerId).some, visibilities = Set(projects.Visibility.Public))) + ) + } + + val expected = List( + publicProject.to[model.Entity.Project], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return dataset entities where the given user is an owner" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val dsAndProject @ _ -> project = renkuProjectEntities(anyVisibility) + .modify(replaceMembers(Set(owner))) + .addDataset( + datasetEntities(provenanceNonModified) + .modify(replaceDSCreators(NonEmptyList.of(personEntities.generateOne, owner.person))) + ) + .generateOne + + val results = IOBody { + provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> + finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + } + + val expected = List( + project.to[model.Entity.Project], + dsAndProject.to[model.Entity.Dataset], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return dataset entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val dsAndPublicProject @ _ -> publicProject = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(owner))) + .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(owner.person)))) + .generateOne + val _ -> nonPublicProject = renkuProjectEntities(visibilityNonPublic) + .modify(replaceMembers(Set(owner))) + .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(owner.person)))) + .generateOne + + val results = IOBody { + provisionTestProjects(publicProject, nonPublicProject) *> + finder.findEntities( + Criteria( + Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some, visibilities = Set(projects.Visibility.Public)) + ) + ) + } + + val expected = List( + publicProject.to[model.Entity.Project], + dsAndPublicProject.to[model.Entity.Dataset], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return workflow entities where the given user is an owner" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val project = renkuProjectEntities(anyVisibility) + .modify(replaceMembers(Set(owner))) + .withActivities( + activityEntities(stepPlanEntities().map(_.replaceCreators(List(personEntities.generateOne, owner.person)))) + ) + .generateOne + + val results = IOBody { + provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> + finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + } + + val expected = List( + project.to[model.Entity.Project], + (project.plans.head -> project).to[model.Entity.Workflow], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return workflow entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + + val ownerId = personGitLabIds.generateOne + val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + + val publicProject = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(owner))) + .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(owner.person))))) + .generateOne + + val nonPublicProject = renkuProjectEntities(visibilityNonPublic) + .modify(replaceMembers(Set(owner))) + .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(owner.person))))) + .generateOne + + val results = IOBody { + provisionTestProjects(publicProject, nonPublicProject) *> + finder.findEntities( + Criteria( + Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some, visibilities = Set(projects.Visibility.Public)) + ) + ) + } + + val expected = List( + publicProject.to[model.Entity.Project], + (publicProject.plans.head -> publicProject).to[model.Entity.Workflow], + owner.person.to[model.Entity.Person] + ).sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + } + "findEntities - with visibility filter" should { "return entities with matching visibility only" in new TestCase { diff --git a/entities-search/src/test/scala/io/renku/entities/search/Generators.scala b/entities-search/src/test/scala/io/renku/entities/search/Generators.scala index b62f72e89a..04f331bbfb 100644 --- a/entities-search/src/test/scala/io/renku/entities/search/Generators.scala +++ b/entities-search/src/test/scala/io/renku/entities/search/Generators.scala @@ -30,11 +30,11 @@ import testentities._ object Generators { - val queryParams: Gen[Filters.Query] = nonBlankStrings(minLength = 5).map(v => Filters.Query(v.value)) - val typeParams: Gen[Filters.EntityType] = Gen.oneOf(Filters.EntityType.all) - val sinceParams: Gen[Filters.Since] = localDatesNotInTheFuture.toGeneratorOf(Filters.Since) - val untilParams: Gen[Filters.Until] = localDatesNotInTheFuture.toGeneratorOf(Filters.Until) - val matchingScores: Gen[MatchingScore] = choose(MatchingScore.min.value, 10f).toGeneratorOf(MatchingScore) + val queryParams: Gen[Filters.Query] = nonBlankStrings(minLength = 5).map(v => Filters.Query(v.value)) + val typeParams: Gen[Filters.EntityType] = Gen.oneOf(Filters.EntityType.all) + val sinceParams: Gen[Filters.Since] = localDatesNotInTheFuture.toGeneratorOf(Filters.Since) + val untilParams: Gen[Filters.Until] = localDatesNotInTheFuture.toGeneratorOf(Filters.Until) + val matchingScores: Gen[MatchingScore] = choose(MatchingScore.min.value, 10f).toGeneratorOf(MatchingScore) val modelProjects: Gen[model.Entity.Project] = anyProjectEntities.map(_.to[model.Entity.Project]) val modelDatasets: Gen[model.Entity.Dataset] = diff --git a/entities-search/src/test/scala/io/renku/entities/searchgraphs/SearchInfoDatasets.scala b/entities-search/src/test/scala/io/renku/entities/searchgraphs/SearchInfoDatasets.scala index 561e2b4861..af2cf63e97 100644 --- a/entities-search/src/test/scala/io/renku/entities/searchgraphs/SearchInfoDatasets.scala +++ b/entities-search/src/test/scala/io/renku/entities/searchgraphs/SearchInfoDatasets.scala @@ -21,7 +21,6 @@ package io.renku.entities.searchgraphs import cats.effect.IO import cats.syntax.all._ import io.renku.graph.model.entities.EntityFunctions -import io.renku.graph.model.projects.Role import io.renku.graph.model.{RenkuUrl, datasets, entities, testentities} import io.renku.lock.Lock import io.renku.logging.{ExecutionTimeRecorder, TestExecutionTimeRecorder} @@ -70,7 +69,7 @@ trait SearchInfoDatasets { implicit val sparqlQueryTimeRecorder: SparqlQueryTimeRecorder[IO] = new SparqlQueryTimeRecorder[IO](execTimeRecorder) val ps = ProjectSparqlClient[IO](projectsDSConnectionInfo).map(ProjectAuthService[IO](_, renkuUrl)) - val members = project.members.flatMap(p => p.person.maybeGitLabId.map(id => ProjectMember(id, Role.Reader))) + val members = project.members.flatMap(m => m.person.maybeGitLabId.map(id => ProjectMember(id, m.role))) ps.use(_.update(ProjectAuthData(project.slug, members, project.visibility))) } diff --git a/knowledge-graph/README.md b/knowledge-graph/README.md index d19c0b3cfa..32d54d31d5 100644 --- a/knowledge-graph/README.md +++ b/knowledge-graph/README.md @@ -300,6 +300,7 @@ Allows finding `projects`, `datasets`, `workflows`, and `persons`. * `query` - to filter by matching field (e.g., title, keyword, description, etc. as specified below) * `type` - to filter by entity type(s); allowed values: `project`, `dataset`, `workflow`, and `person`; multiple `type` parameters allowed * `creator` - to filter by creator(s); the filter would require creator's name; multiple `creator` parameters allowed +* `owned` - to reduce the results to entities where the caller is an owner; this parameter does not require any value * `visibility` - to filter by visibility(ies) (restricted vs. public); allowed values: `public`, `internal`, `private`; multiple `visibility` parameters allowed * `namespace` - to filter by namespace(s); there might be multiple values given; for nested namespaces the whole path has be used, e.g. `group/subgroup` * `since` - to filter by entity's creation date to >= the given date @@ -328,12 +329,12 @@ When the `query` parameter is given, the match is done on the following fields: **Response** -| Status | Description | -|------------------------------|--------------------------------------------------| -| OK (200) | If results are found; `[]` if nothing is found | -| BAD_REQUEST (400) | If illegal values for query parameters are given | -| UNAUTHORIZED (401) | If given auth header cannot be authenticated | -| INTERNAL SERVER ERROR (500) | Otherwise | +| Status | Description | +|------------------------------|------------------------------------------------------------------------------------------------| +| OK (200) | If results are found; `[]` if nothing is found | +| BAD_REQUEST (400) | If illegal values for query parameters are given or `owned` specified but no auth user present | +| UNAUTHORIZED (401) | If given auth header cannot be authenticated | +| INTERNAL SERVER ERROR (500) | Otherwise | Response headers: diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala index 24a3026a28..5fefa874dc 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala @@ -134,12 +134,12 @@ private class MicroserviceRoutes[F[_]: Async]( import Sort.sort AuthedRoutes.of { - case req@GET -> Root / "knowledge-graph" / "entities" - :? query(maybeQuery) +& entityTypes(maybeTypes) +& creatorNames(maybeCreators) + case req @ GET -> Root / "knowledge-graph" / "entities" + :? query(maybeQuery) +& entityTypes(maybeTypes) +& creatorNames(maybeCreators) +& owned(maybeOwned) +& visibilities(maybeVisibilities) +& namespaces(maybeNamespaces) +& since(maybeSince) +& until(maybeUntil) +& sort(maybeSort) +& page(maybePage) +& perPage(maybePerPage) as maybeUser => - searchForEntities(maybeQuery, maybeTypes, maybeCreators, maybeVisibilities, maybeNamespaces, + searchForEntities(maybeQuery, maybeTypes, maybeCreators, maybeOwned, maybeVisibilities, maybeNamespaces, maybeSince, maybeUntil, maybeSort, maybePage, maybePerPage, maybeUser.option, req.req) } } @@ -226,6 +226,7 @@ private class MicroserviceRoutes[F[_]: Async]( maybeQuery: Option[ValidatedNel[ParseFailure, EntitiesSearchCriteria.Filters.Query]], types: ValidatedNel[ParseFailure, List[EntitiesSearchCriteria.Filters.EntityType]], creators: ValidatedNel[ParseFailure, List[persons.Name]], + maybeOwned: Boolean, visibilities: ValidatedNel[ParseFailure, List[model.projects.Visibility]], namespaces: ValidatedNel[ParseFailure, List[model.projects.Namespace]], maybeSince: Option[ValidatedNel[ParseFailure, EntitiesSearchCriteria.Filters.Since]], @@ -242,6 +243,11 @@ private class MicroserviceRoutes[F[_]: Async]( maybeQuery.map(_.map(Option.apply)).getOrElse(Validated.validNel(Option.empty[Query])), types.map(_.toSet), creators.map(_.toSet), + maybeOwned -> maybeUser match { + case (true, Some(authUser)) => Owned(authUser.id).some.validNel[ParseFailure] + case (true, None) => ParseFailure("'owned' parameter present but no access token", "").invalidNel[Option[Owned]] + case _ => Option.empty[Owned].validNel[ParseFailure] + }, visibilities.map(_.toSet), namespaces.map(_.toSet), maybeSince.map(_.map(Option.apply)).getOrElse(Validated.validNel(Option.empty[Since])), @@ -249,22 +255,35 @@ private class MicroserviceRoutes[F[_]: Async]( sorting.map(Sorting.fromListOrDefault(_, Sort.default)), PagingRequest(maybePage, maybePerPage), (maybeSince -> maybeUntil) - .mapN(_ -> _) + .mapN(Tuple2.apply) .map { case (Valid(since), Valid(until)) if (since.value compareTo until.value) > 0 => ParseFailure("'since' parameter > 'until'", "").invalidNel[Unit] case _ => ().validNel[ParseFailure] } .getOrElse(().validNel[ParseFailure]) - ).mapN { case (maybeQuery, types, creators, visibilities, namespaces, maybeSince, maybeUntil, sorting, paging, _) => - `GET /entities`( - EntitiesSearchCriteria(Filters(maybeQuery, types, creators, visibilities, namespaces, maybeSince, maybeUntil), - sorting, - paging, - maybeUser - ), - request - ) + ).mapN { + case (maybeQuery, + types, + creators, + maybeOwned, + visibilities, + namespaces, + maybeSince, + maybeUntil, + sorting, + paging, + _ + ) => + `GET /entities`( + EntitiesSearchCriteria( + Filters(maybeQuery, types, creators, maybeOwned, visibilities, namespaces, maybeSince, maybeUntil), + sorting, + paging, + maybeUser + ), + request + ) }.fold(toBadRequest, identity) } diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala index d70fc1ea4c..ec88f18b64 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala @@ -24,7 +24,7 @@ import io.renku.entities.search.Criteria.Filters._ import io.renku.entities.viewings.search.RecentEntitiesFinder import io.renku.graph.model._ import io.renku.http.rest.paging.model.PerPage -import org.http4s.dsl.io.{OptionalMultiQueryParamDecoderMatcher, OptionalValidatingQueryParamDecoderMatcher} +import org.http4s.dsl.io.{FlagQueryParamMatcher, OptionalMultiQueryParamDecoderMatcher, OptionalValidatingQueryParamDecoderMatcher} import org.http4s.{ParseFailure, QueryParamDecoder, QueryParameterValue} import java.time.LocalDate @@ -85,6 +85,10 @@ object QueryParamDecoders { val parameterName: String = "creator" } + object owned extends FlagQueryParamMatcher("owned") { + val parameterName: String = "owned" + } + private implicit val visibilityParameterDecoder: QueryParamDecoder[projects.Visibility] = (value: QueryParameterValue) => projects.Visibility diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala index 44aa19e235..ac515fc815 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala @@ -50,13 +50,13 @@ private class EndpointDocsImpl()(implicit gitLabUrl: GitLabUrl, renkuApiUrl: ren GET( "Cross-Entity Search", "Finds entities by the given criteria", - Uri / "entities" :? query & `type` & creator & visibility & namespace & since & until & sort & page & perPage, + Uri / "entities" :? query & `type` & creator & owned & visibility & namespace & since & until & sort & page & perPage, Status.Ok -> Response("Found entities", Contents(MediaType.`application/json`("Sample response", example)), responseHeaders ), Status.BadRequest -> Response( - "In case of invalid query parameters", + "In case of invalid query parameters or `owned` parameter specified but no auth user present", Contents(MediaType.`application/json`("Reason", Message.Info("Invalid parameters"))) ), Status.Unauthorized -> Response( @@ -87,6 +87,12 @@ private class EndpointDocsImpl()(implicit gitLabUrl: GitLabUrl, renkuApiUrl: ren "to filter by creator(s); the filter would require creator's name; multiple creator parameters allowed".some, required = false ) + private lazy val owned = Parameter.Query( + "owned", + Schema.String, + "to reduce the results to entities where the caller is an owner; this parameter does not require any value".some, + required = false + ) private lazy val visibility = Parameter.Query( "visibility", Schema.String, diff --git a/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala b/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala index 27193d29e6..800e801a36 100644 --- a/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala +++ b/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala @@ -275,9 +275,7 @@ class MicroserviceRoutesSpec "uri" -> "criteria", uri"/knowledge-graph/entities" -> Criteria(), queryParams - .map(query => - uri"/knowledge-graph/entities" +? ("query" -> query.value) -> Criteria(Filters(maybeQuery = query.some)) - ) + .map(q => uri"/knowledge-graph/entities" +? ("query" -> q.value) -> Criteria(Filters(maybeQuery = q.some))) .generateOne, typeParams .map(t => uri"/knowledge-graph/entities" +? ("type" -> t.value) -> Criteria(Filters(entityTypes = Set(t)))) @@ -380,6 +378,7 @@ class MicroserviceRoutesSpec ) } { (uri, criteria) => s"read the query parameters from $uri, pass them to the endpoint and return received response" in new TestCase { + val request = Request[IO](GET, uri) val responseBody = jsons.generateOne @@ -395,6 +394,34 @@ class MicroserviceRoutesSpec } } + "read the 'owned' query parameter from the uri and pass it to the endpoint when auth user present" in new TestCase { + + val authUser = authUsers.generateOne + val maybeAuthUser = MaybeAuthUser(authUser) + val criteria = Criteria(Filters(maybeOwned = Filters.Owned(authUser.id).some), maybeUser = maybeAuthUser.option) + val request = Request[IO](GET, uri"/knowledge-graph/entities" +? "owned") + + val responseBody = jsons.generateOne + (entitiesEndpoint.`GET /entities` _) + .expects(criteria, request) + .returning(Response[IO](Ok).withEntity(responseBody).pure[IO]) + + val response = routes(maybeAuthUser).call(request) + + response.status shouldBe Ok + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.body[Json] shouldBe responseBody + } + + s"return $BadRequest 'owned' query parameter given but no auth user present" in new TestCase { + + val response = routes().call(Request[IO](GET, uri"/knowledge-graph/entities" +? "owned")) + + response.status shouldBe BadRequest + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.body[Message] shouldBe Message.Error("'owned' parameter present but no access token") + } + s"return $BadRequest for invalid parameter values" in new TestCase { val response = routes() .call( @@ -1115,9 +1142,10 @@ class MicroserviceRoutesSpec .expects(identifier, maybeAuthUser) .returning(returning) - def givenDSSameAsAuthorizer(sameAs: model.datasets.SameAs, - maybeAuthUser: Option[AuthUser], - returning: EitherT[IO, EndpointSecurityException, AuthContext[model.datasets.SameAs]] + private def givenDSSameAsAuthorizer( + sameAs: model.datasets.SameAs, + maybeAuthUser: Option[AuthUser], + returning: EitherT[IO, EndpointSecurityException, AuthContext[model.datasets.SameAs]] ) = (datasetSameAsAuthorizer.authorize _) .expects(sameAs, maybeAuthUser) .returning(returning) diff --git a/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala b/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala index 3c9788400d..76dc5481e8 100644 --- a/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala +++ b/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala @@ -19,9 +19,9 @@ package io.renku.projectauth.util import cats.syntax.all._ -import io.renku.graph.model.{persons, projects} import io.renku.graph.model.projects.Visibility -import io.renku.projectauth.ProjectAuth +import io.renku.graph.model.{persons, projects} +import io.renku.projectauth.{ProjectAuth, ProjectMember} import io.renku.triplesstore.client.sparql.{Fragment, VarName} import io.renku.triplesstore.client.syntax._ @@ -46,6 +46,13 @@ final class SparqlSnippets(val projectId: VarName) { |} | """.stripMargin + def ownedProjects(userId: persons.GitLabId): Fragment = + fr"""|GRAPH renku:ProjectAuth { + | $projectId a schema:Project; + | renku:memberRole ${ProjectMember(userId, projects.Role.Owner).encoded}. + |} + | """.stripMargin + def visibleProjects(userId: Option[persons.GitLabId], selectedVisibility: Set[Visibility]): Fragment = { val visibilities = if (selectedVisibility.isEmpty) Visibility.all @@ -89,6 +96,6 @@ object SparqlSnippets { def apply(projectVar: VarName): SparqlSnippets = new SparqlSnippets(projectVar) - val default = + val default: SparqlSnippets = SparqlSnippets(VarName("projectId")) } diff --git a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/DatasetEntitiesGenerators.scala b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/DatasetEntitiesGenerators.scala index e65a976b36..712b71f88f 100644 --- a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/DatasetEntitiesGenerators.scala +++ b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/DatasetEntitiesGenerators.scala @@ -300,13 +300,13 @@ trait DatasetEntitiesGenerators { } implicit def identificationLens[P <: Dataset.Provenance]: Lens[Dataset[P], Identification] = - Lens[Dataset[P], Identification](_.identification)(identification => ds => ds.copy(identification = identification)) + Lens[Dataset[P], Identification](_.identification)(identification => _.copy(identification = identification)) implicit def provenanceLens[P <: Dataset.Provenance]: Lens[Dataset[P], P] = - Lens[Dataset[P], P](_.provenance)(prov => ds => ds.copy(provenance = prov)) + Lens[Dataset[P], P](_.provenance)(prov => _.copy(provenance = prov)) implicit def additionalInfoLens[P <: Dataset.Provenance]: Lens[Dataset[P], AdditionalInfo] = - Lens[Dataset[P], AdditionalInfo](_.additionalInfo)(additionalInfo => ds => ds.copy(additionalInfo = additionalInfo)) + Lens[Dataset[P], AdditionalInfo](_.additionalInfo)(additionalInfo => _.copy(additionalInfo = additionalInfo)) implicit def creatorsLens[P <: Dataset.Provenance]: Lens[P, NonEmptyList[Person]] = Lens[P, NonEmptyList[Person]](_.creators) { crts => @@ -319,6 +319,9 @@ trait DatasetEntitiesGenerators { } } + def replaceDSCreators[P <: Dataset.Provenance](creators: NonEmptyList[Person]): Dataset[P] => Dataset[P] = + provenanceLens[P].modify(creatorsLens[P].replace(creators)) + def replaceDSName[P <: Dataset.Provenance](to: datasets.Name): Dataset[P] => Dataset[P] = identificationLens[P].modify(_.copy(name = to)) diff --git a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/ProjectEntitiesGenerators.scala b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/ProjectEntitiesGenerators.scala index 630c13234d..0feb607c40 100644 --- a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/ProjectEntitiesGenerators.scala +++ b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/ProjectEntitiesGenerators.scala @@ -110,17 +110,15 @@ trait ProjectEntitiesGenerators { def removeCreator[P <: Project](): P => P = creatorLens.modify(_ => None) def replaceProjectCreator[P <: Project](to: Option[Person]): P => P = - _.fold(_.copy(maybeCreator = to), _.copy(maybeCreator = to), _.copy(maybeCreator = to), _.copy(maybeCreator = to)) - .asInstanceOf[P] + creatorLens[P].replace(to) def replaceVisibility[P <: Project](to: Visibility): P => P = _.fold(_.copy(visibility = to), _.copy(visibility = to), _.copy(visibility = to), _.copy(visibility = to)) .asInstanceOf[P] - def removeMembers[P <: Project](): P => P = membersLens.modify(_ => Set.empty) + def removeMembers[P <: Project](): P => P = membersLens.replace(Set.empty) - def replaceMembers[P <: Project](to: Set[Project.Member]): P => P = - _.fold(_.copy(members = to), _.copy(members = to), _.copy(members = to), _.copy(members = to)).asInstanceOf[P] + def replaceMembers[P <: Project](to: Set[Project.Member]): P => P = membersLens.replace(to) def replaceProjectName[P <: Project](to: projects.Name): P => P = _.fold(_.copy(name = to), _.copy(name = to), _.copy(name = to), _.copy(name = to)).asInstanceOf[P] From da53e39687d9a736e75c1797d96051a788a04413 Mon Sep 17 00:00:00 2001 From: RenkuBot <53332360+RenkuBot@users.noreply.github.com> Date: Wed, 8 Nov 2023 08:26:55 +0100 Subject: [PATCH 03/13] chore: Update rdf4j-queryparser-sparql from 4.3.7 to 4.3.8 (#1782) Co-authored-by: RenkuBot --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index a123ddc0be..df7b134f5a 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -26,7 +26,7 @@ object Dependencies { val luceneQueryParser = "9.8.0" val owlapi = "5.5.0" val pureconfig = "0.17.4" - val rdf4jQueryParserSparql = "4.3.7" + val rdf4jQueryParserSparql = "4.3.8" val refined = "0.11.0" val refinedPureconfig = "0.11.0" val scalacheck = "1.17.0" From 4a211619a9e52afcc6a5e0a35ade4a61d64cd456 Mon Sep 17 00:00:00 2001 From: RenkuBot <53332360+RenkuBot@users.noreply.github.com> Date: Wed, 8 Nov 2023 08:27:12 +0100 Subject: [PATCH 04/13] chore: Update sentry-logback from 6.33.0 to 6.33.1 (#1781) Co-authored-by: RenkuBot --- project/Dependencies.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index df7b134f5a..704c48cc53 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -33,7 +33,7 @@ object Dependencies { val scalamock = "5.2.0" val scalatest = "3.2.17" val scalatestScalacheck = "3.2.14.0" - val sentryLogback = "6.33.0" + val sentryLogback = "6.33.1" val skunk = "0.6.1" val swaggerParser = "2.1.18" val testContainersScala = "0.41.0" From 95e0e52ebef59e4ee95886d24921cc36c75e9482 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Thu, 9 Nov 2023 16:15:09 +0100 Subject: [PATCH 05/13] feat: Cross-Entity Search results quality improvements (#1783) * feat: custom Lucene analyzer for the Projects dataset in the TS - for writing * feat: KeywordAnalyzer for the Projects dataset in the TS - for reading * feat: TS migrations to update the projects-ds.ttl and kick off reindexing --- .../CrossEntitiesSearchSpec.scala | 3 +- .../entities/search/EntitiesFinderSpec.scala | 12 +- .../search/PersonsEntitiesFinderSpec.scala | 3 +- .../src/main/resources/projects-ds.ttl | 79 +++++++++++-- .../migrations/Migrations.scala | 105 +++++++----------- .../client/sparql/QueryTokenizer.scala | 44 ++++++-- .../client/sparql/QueryTokenizerSpec.scala | 25 ++++- 7 files changed, 177 insertions(+), 94 deletions(-) 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 38f1878314..e43256da0b 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 @@ -33,6 +33,7 @@ import io.renku.graph.model.projects.Role import io.renku.graph.model.testentities._ import io.renku.http.client.UrlEncoder._ import org.http4s.Status.Ok +import org.scalacheck.Gen.alphaLowerChar import tooling.{AcceptanceSpec, ApplicationServices} class CrossEntitiesSearchSpec extends AcceptanceSpec with ApplicationServices with TSProvisioning { @@ -41,7 +42,7 @@ class CrossEntitiesSearchSpec extends AcceptanceSpec with ApplicationServices wi val user = authUsers.generateOne - val commonPhrase = nonBlankStrings(minLength = 5).generateOne + val commonPhrase = nonBlankStrings(minLength = 5, charsGenerator = alphaLowerChar).generateOne val testProject = renkuProjectEntities(visibilityPublic, creatorGen = cliShapedPersons) .modify(replaceProjectName(sentenceContaining(commonPhrase).generateAs[projects.Name])) diff --git a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala index 8c11eb4072..d0d657b014 100644 --- a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala +++ b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala @@ -42,6 +42,7 @@ import io.renku.http.rest.paging.model._ import io.renku.http.rest.{SortBy, Sorting} import io.renku.testtools.IOSpec import io.renku.triplesstore._ +import org.scalacheck.Gen.alphaLowerChar import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -155,7 +156,8 @@ class EntitiesFinderSpec } "return entities which name matches the given query, sorted by name" in new TestCase { - val query = nonBlankStrings(minLength = 6).generateOne + + val query = nonBlankStrings(minLength = 6, charsGenerator = alphaLowerChar).generateOne val person = personEntities .map(replacePersonName(to = sentenceContaining(query).generateAs(persons.Name))) @@ -204,7 +206,7 @@ class EntitiesFinderSpec } "return entities which keywords matches the given query, sorted by name" in new TestCase { - val query = nonBlankStrings(minLength = 6).generateOne + val query = nonBlankStrings(minLength = 6, charsGenerator = alphaLowerChar).generateOne val soleProject = renkuProjectEntities(visibilityPublic) .modify( @@ -253,7 +255,7 @@ class EntitiesFinderSpec } "return entities which description matches the given query, sorted by name" in new TestCase { - val query = nonBlankStrings(minLength = 6).generateOne + val query = nonBlankStrings(minLength = 6, charsGenerator = alphaLowerChar).generateOne val soleProject = renkuProjectEntities(visibilityPublic) .modify( @@ -294,7 +296,7 @@ class EntitiesFinderSpec } "return project entities which namespace matches the given query, sorted by name" in new TestCase { - val query = nonBlankStrings(minLength = 6).generateOne + val query = nonBlankStrings(minLength = 6, charsGenerator = alphaLowerChar).generateOne val soleProject = renkuProjectEntities(visibilityPublic) .modify(_.copy(slug = projects.Slug(s"$query/${relativePaths(maxSegments = 2).generateOne}"))) @@ -311,7 +313,7 @@ class EntitiesFinderSpec } "return entities which creator name matches the given query, sorted by name" in new TestCase { - val query = nonBlankStrings(minLength = 6).generateOne + val query = nonBlankStrings(minLength = 6, charsGenerator = alphaLowerChar).generateOne val projectCreator = personEntities.generateOne.copy(name = sentenceContaining(query).generateAs(persons.Name)) val soleProject = renkuProjectEntities(visibilityPublic) diff --git a/entities-search/src/test/scala/io/renku/entities/search/PersonsEntitiesFinderSpec.scala b/entities-search/src/test/scala/io/renku/entities/search/PersonsEntitiesFinderSpec.scala index d81a0c17bc..d036a2fcc7 100644 --- a/entities-search/src/test/scala/io/renku/entities/search/PersonsEntitiesFinderSpec.scala +++ b/entities-search/src/test/scala/io/renku/entities/search/PersonsEntitiesFinderSpec.scala @@ -28,6 +28,7 @@ import io.renku.graph.model._ import io.renku.graph.model.testentities._ import io.renku.testtools.IOSpec import io.renku.triplesstore.{InMemoryJenaForSpec, ProjectsDataset} +import org.scalacheck.Gen.alphaLowerChar import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -44,7 +45,7 @@ class PersonsEntitiesFinderSpec "return a single person if there are multiple with the same name" in new TestCase { // person merging is a temporary solution until we start to return persons ids - val query = nonBlankStrings(minLength = 3).generateOne + val query = nonBlankStrings(minLength = 3, charsGenerator = alphaLowerChar).generateOne val sharedName = sentenceContaining(query).generateAs(persons.Name) val person1SameName = personEntities.map(_.copy(name = sharedName)).generateOne diff --git a/graph-commons/src/main/resources/projects-ds.ttl b/graph-commons/src/main/resources/projects-ds.ttl index 1cd3fa85e9..94bf81f8dd 100644 --- a/graph-commons/src/main/resources/projects-ds.ttl +++ b/graph-commons/src/main/resources/projects-ds.ttl @@ -20,26 +20,83 @@ :text_dataset a text:TextDataset ; text:dataset :tdb_projects ; - text:index <#indexLucene> ; + text:index :indexLucene ; . :tdb_projects a tdb2:DatasetTDB2 ; tdb2:location "/fuseki/databases/projects" . -<#indexLucene> a text:TextIndexLucene ; +:indexLucene a text:TextIndexLucene ; text:directory ; - text:entityMap <#entMap> ; + text:defineAnalyzers ( + [ + text:defineAnalyzer :preservingAndLetterAnalyzer ; + text:analyzer [ + a text:ConfigurableAnalyzer ; + text:tokenizer text:WhitespaceTokenizer ; + text:filters ( :preservingAndLetterFilter ) + ] + ] + [ + text:defineFilter :preservingAndLetterFilter ; + text:filter [ + a text:GenericFilter ; + text:class "org.apache.lucene.analysis.miscellaneous.WordDelimiterGraphFilter" ; + text:params ( + [ + text:paramName "configurationFlags" ; + # 227 means setting "preserveOriginal", "1", "stemEnglishPossessive", "0" when the WordDelimiterGraphFilterFactory is used + text:paramValue 227 + ] + [ + text:paramName "protWords" ; + text:paramType text:TypeSet ; + text:paramValue () + ] + ) + ] + ] + ) ; + text:analyzer [ + a text:DefinedAnalyzer ; + text:useAnalyzer :preservingAndLetterAnalyzer + ] ; + text:queryAnalyzer [ + a text:KeywordAnalyzer + ] ; + text:entityMap :entMap . -<#entMap> a text:EntityMap ; +:entMap a text:EntityMap ; text:defaultField "name" ; text:entityField "uri" ; text:map ( - [ text:field "name" ; text:predicate schema:name ] - [ text:field "description" ; text:predicate schema:description ] - [ text:field "slug" ; text:predicate renku:slug ] - [ text:field "projectNamespaces" ; text:predicate renku:projectNamespaces ] - [ text:field "keywords" ; text:predicate schema:keywords ] - [ text:field "keywordsConcat" ; text:predicate renku:keywordsConcat ] - [ text:field "creatorsNamesConcat" ; text:predicate renku:creatorsNamesConcat ] + [ + text:field "name" ; + text:predicate schema:name + ] + [ + text:field "description" ; + text:predicate schema:description + ] + [ + text:field "slug" ; + text:predicate renku:slug + ] + [ + text:field "projectNamespaces" ; + text:predicate renku:projectNamespaces + ] + [ + text:field "keywords" ; + text:predicate schema:keywords + ] + [ + text:field "keywordsConcat" ; + text:predicate renku:keywordsConcat + ] + [ + text:field "creatorsNamesConcat" ; + text:predicate renku:creatorsNamesConcat + ] ) . diff --git a/triples-generator/src/main/scala/io/renku/triplesgenerator/events/consumers/tsmigrationrequest/migrations/Migrations.scala b/triples-generator/src/main/scala/io/renku/triplesgenerator/events/consumers/tsmigrationrequest/migrations/Migrations.scala index 0bec2318cf..3a8053fa4b 100644 --- a/triples-generator/src/main/scala/io/renku/triplesgenerator/events/consumers/tsmigrationrequest/migrations/Migrations.scala +++ b/triples-generator/src/main/scala/io/renku/triplesgenerator/events/consumers/tsmigrationrequest/migrations/Migrations.scala @@ -33,72 +33,45 @@ private[tsmigrationrequest] object Migrations { def apply[F[_]: Async: ReProvisioningStatus: Logger: MetricsRegistry: SparqlQueryTimeRecorder]( config: Config - ): F[List[Migration[F]]] = for { - datasetsCreator <- DatasetsCreator[F] - datasetsRemover <- DatasetsRemover[F] - reProvisioning <- ReProvisioning[F](config) - removeNotLinkedPersons <- RemoveNotLinkedPersons[F] - fixMultipleProjectCreatedDates <- FixMultipleProjectCreatedDates[F] - addRenkuPlanWhereMissing <- AddRenkuPlanWhereMissing[F] - migrationToV10 <- v10migration.MigrationToV10[F] - v10VersionSetter <- V10VersionUpdater[F] - projectsDateViewedCreator <- ProjectsDateViewedCreator[F] - projectDateViewedDeduplicator <- ProjectDateViewedDeduplicator[F] - personViewedEntityDeduplicator <- PersonViewedEntityDeduplicator[F] - provisionProjectsGraph <- projectsgraph.ProvisionProjectsGraph[F] - addProjectDateModified <- datemodified.AddProjectDateModified[F] - fixMultipleProjectVersions <- FixMultipleProjectVersions[F] - addProjectSlug <- projectslug.AddProjectSlug[F] - datasetsGraphPersonRemover <- DatasetsGraphPersonRemover[F] - projectsGraphPersonRemover <- ProjectsGraphPersonRemover[F] - reindexLuceneStdTokenizer <- lucenereindex.ReindexLucene[F](suffix = "- std tokenizer") - projectsDSRecreatorSearchFlattening <- TSDatasetRecreator[F, ProjectsTTL]("- search flattening", ProjectsTTL) - projectsGraphKeywordsFlattener <- ProjectsGraphKeywordsFlattener[F] - projectsGraphImagesFlattener <- ProjectsGraphImagesFlattener[F] - datasetsGraphKeywordsFlattener <- DatasetsGraphKeywordsFlattener[F] - datasetsGraphImagesFlattener <- DatasetsGraphImagesFlattener[F] - datasetsGraphCreatorsFlattener <- DatasetsGraphCreatorsFlattener[F] - datasetsGraphSlugsVisibsFlattener <- DatasetsGraphSlugsVisibilitiesFlattener[F] - projectMembersRemover <- ProjectMembersRemover[F] - datasetsGraphKeywordsRemover <- DatasetsGraphKeywordsRemover[F] - datasetsGraphImagesRemover <- DatasetsGraphImagesRemover[F] - projectsGraphKeywordsRemover <- ProjectsGraphKeywordsRemover[F] - projectsGraphImagesRemover <- ProjectsGraphImagesRemover[F] - migrations <- validateNames( - datasetsCreator, - datasetsRemover, - reProvisioning, - removeNotLinkedPersons, - fixMultipleProjectCreatedDates, - addRenkuPlanWhereMissing, - migrationToV10, - v10VersionSetter, - projectsDateViewedCreator, - projectDateViewedDeduplicator, - personViewedEntityDeduplicator, - provisionProjectsGraph, - addProjectDateModified, - fixMultipleProjectVersions, - addProjectSlug, - datasetsGraphPersonRemover, - projectsGraphPersonRemover, - reindexLuceneStdTokenizer, - projectsDSRecreatorSearchFlattening, - projectsGraphKeywordsFlattener, - projectsGraphImagesFlattener, - datasetsGraphKeywordsFlattener, - datasetsGraphImagesFlattener, - datasetsGraphCreatorsFlattener, - datasetsGraphSlugsVisibsFlattener, - projectMembersRemover, - datasetsGraphKeywordsRemover, - datasetsGraphImagesRemover, - projectsGraphKeywordsRemover, - projectsGraphImagesRemover - ) - } yield migrations + ): F[List[Migration[F]]] = + List( + DatasetsCreator[F], + DatasetsRemover[F], + ReProvisioning[F](config), + RemoveNotLinkedPersons[F], + FixMultipleProjectCreatedDates[F], + AddRenkuPlanWhereMissing[F], + v10migration.MigrationToV10[F], + V10VersionUpdater[F], + ProjectsDateViewedCreator[F], + ProjectDateViewedDeduplicator[F], + PersonViewedEntityDeduplicator[F], + projectsgraph.ProvisionProjectsGraph[F], + datemodified.AddProjectDateModified[F], + FixMultipleProjectVersions[F], + projectslug.AddProjectSlug[F], + DatasetsGraphPersonRemover[F], + ProjectsGraphPersonRemover[F], + lucenereindex.ReindexLucene[F](suffix = "- std tokenizer"), + TSDatasetRecreator[F, ProjectsTTL]("- search flattening", ProjectsTTL), + ProjectsGraphKeywordsFlattener[F], + ProjectsGraphImagesFlattener[F], + DatasetsGraphKeywordsFlattener[F], + DatasetsGraphImagesFlattener[F], + DatasetsGraphCreatorsFlattener[F], + DatasetsGraphSlugsVisibilitiesFlattener[F], + ProjectMembersRemover[F], + DatasetsGraphKeywordsRemover[F], + DatasetsGraphImagesRemover[F], + ProjectsGraphKeywordsRemover[F], + ProjectsGraphImagesRemover[F], + TSDatasetRecreator[F, ProjectsTTL]("- custom tokenizer", ProjectsTTL), + lucenereindex.ReindexLucene[F](suffix = "- custom tokenizer") + ).sequence.flatMap(validateNames[F](_)) - private[migrations] def validateNames[F[_]: MonadThrow: Logger](migrations: Migration[F]*): F[List[Migration[F]]] = { + private[migrations] def validateNames[F[_]: MonadThrow: Logger]( + migrations: List[Migration[F]] + ): F[List[Migration[F]]] = { val groupedByName = migrations.groupBy(_.name) val problematicMigrations = groupedByName.collect { case (name, ms) if ms.size > 1 => name @@ -107,6 +80,6 @@ private[tsmigrationrequest] object Migrations { val error = show"$categoryName: there are multiple migrations with name: ${problematicMigrations.mkString("; ")}" Logger[F] .error(error) >> new Exception(error).raiseError[F, List[Migration[F]]].map(_ => List.empty[Migration[F]]) - } else migrations.toList.pure[F] + } else migrations.pure[F] } } diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/QueryTokenizer.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/QueryTokenizer.scala index 8cd1c53a39..194419caac 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/QueryTokenizer.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/QueryTokenizer.scala @@ -18,10 +18,11 @@ package io.renku.triplesstore.client.sparql -import org.apache.lucene.analysis.Tokenizer import org.apache.lucene.analysis.core.LetterTokenizer +import org.apache.lucene.analysis.custom.CustomAnalyzer import org.apache.lucene.analysis.standard.StandardTokenizer import org.apache.lucene.analysis.tokenattributes.{CharTermAttribute, CharTermAttributeImpl} +import org.apache.lucene.analysis.{Analyzer, TokenStream, Tokenizer} import org.apache.lucene.util.AttributeFactory import org.apache.lucene.util.AttributeFactory.StaticImplementationAttributeFactory @@ -42,13 +43,23 @@ object QueryTokenizer { def luceneLetters: QueryTokenizer = new LuceneTokenizer(new LetterTokenizer(_)) + def lucenePreservingWithLetters: QueryTokenizer = + new LuceneAnalyzer( + CustomAnalyzer + .builder() + .withTokenizer("whitespace") + .addTokenFilter("wordDelimiterGraph", "preserveOriginal", "1", "stemEnglishPossessive", "0") + .build() + ) + def splitOn(c: Char): QueryTokenizer = (input: String) => input.split(c).toList def default: QueryTokenizer = - luceneStandard + lucenePreservingWithLetters private final class LuceneTokenizer(createDelegate: AttributeFactory => Tokenizer) extends QueryTokenizer { + override def split(input: String): List[String] = { val tokenizer = createDelegate( new StaticImplementationAttributeFactory[CharTermAttributeImpl]( @@ -59,15 +70,32 @@ object QueryTokenizer { } ) tokenizer.setReader(new StringReader(input)) - tokenizer.reset() val attr = tokenizer.addAttribute(classOf[CharTermAttribute]) - @annotation.tailrec - def loop(result: List[String]): List[String] = - if (tokenizer.incrementToken()) loop(attr.toString :: result) - else result + tokenizer.reset() - loop(Nil).reverse + collectTokens(tokenizer, attr).reverse } } + + private final class LuceneAnalyzer(analyzer: Analyzer) extends QueryTokenizer { + + override def split(input: String): List[String] = { + + val tokenStream = analyzer.tokenStream("DUMMY_FIELD", input) + val attr = tokenStream.addAttribute(classOf[CharTermAttribute]) + + tokenStream.reset() + + collectTokens(tokenStream, attr).reverse + } + } + + @annotation.tailrec + private def collectTokens(tokenStream: TokenStream, + attr: CharTermAttribute, + results: List[String] = Nil + ): List[String] = + if (tokenStream.incrementToken()) collectTokens(tokenStream, attr, attr.toString :: results) + else results } diff --git a/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/QueryTokenizerSpec.scala b/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/QueryTokenizerSpec.scala index 7e57abfb78..5859959d7b 100644 --- a/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/QueryTokenizerSpec.scala +++ b/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/QueryTokenizerSpec.scala @@ -22,8 +22,9 @@ import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should class QueryTokenizerSpec extends AnyFlatSpec with should.Matchers { - val standard = QueryTokenizer.luceneStandard - val letter = QueryTokenizer.luceneLetters + val standard = QueryTokenizer.luceneStandard + val letter = QueryTokenizer.luceneLetters + def preservingWithLetters = QueryTokenizer.lucenePreservingWithLetters "standard" should "split on whitespace" in { val input = " one two\tthree four\nfive " @@ -58,4 +59,24 @@ class QueryTokenizerSpec extends AnyFlatSpec with should.Matchers { val input = "01_test 02 bar" letter.split(input) shouldBe List("test", "bar") } + + "preservingWithWhitespace" should "split on whitespaces" in { + val text = " one two\tthree four\nfive " + preservingWithLetters.split(text) shouldBe List("one", "two", "three", "four", "five") + } + + it should "split on non-letters and preserve the group" in { + val input = "one_two_three" + preservingWithLetters.split(input) shouldBe List(input, "one", "two", "three") + } + + it should "split on case changes and preserve the group" in { + val input = "camelCase" + preservingWithLetters.split(input) shouldBe List("camelCase", "camel", "Case") + } + + it should "split on whitespaces, preserve the groups and split the groups on non-letters" in { + val input = "01_test 02 bar 03-foo" + preservingWithLetters.split(input) shouldBe List("01_test", "01", "test", "02", "bar", "03-foo", "03", "foo") + } } From 8bbda5cd8fa32ad4832a2b672c820198e395ecd9 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Fri, 10 Nov 2023 11:28:18 +0100 Subject: [PATCH 06/13] fix: LuceneQuery sparql encoder to properly encode special chars (#1785) --- .../renku/entities/search/PersonsQuery.scala | 2 +- .../triplesstore/LuceneQueryEncoderSpec.scala | 86 +++++++++++-------- .../client/sparql/LuceneQuery.scala | 7 +- .../client/sparql/LuceneQueryEncoder.scala | 2 +- .../client/sparql/StringInterpolator.scala | 2 +- .../triplesstore/client/SyntaxTest.scala | 3 +- .../sparql/StringInterpolatorSpec.scala | 2 +- 7 files changed, 59 insertions(+), 45 deletions(-) diff --git a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala index 45a5af1828..22ed06be57 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala @@ -60,7 +60,7 @@ private case object PersonsQuery extends EntityQuery[model.Entity.Person] { private def textPart(filters: Criteria.Filters) = filters.onQuery( - snippet = fr"""(?id ?score) text:query (schema:name ${filters.query.query.asTripleObject}).""", + snippet = fr"""(?id ?score) text:query (schema:name ${filters.query}).""", matchingScoreVariableName = VarName("score") ) diff --git a/graph-commons/src/test/scala/io/renku/triplesstore/LuceneQueryEncoderSpec.scala b/graph-commons/src/test/scala/io/renku/triplesstore/LuceneQueryEncoderSpec.scala index 6b63285849..244d6a3f98 100644 --- a/graph-commons/src/test/scala/io/renku/triplesstore/LuceneQueryEncoderSpec.scala +++ b/graph-commons/src/test/scala/io/renku/triplesstore/LuceneQueryEncoderSpec.scala @@ -18,15 +18,17 @@ package io.renku.triplesstore -import eu.timepit.refined.api.Refined import eu.timepit.refined.auto._ import io.renku.generators.Generators.Implicits._ -import io.renku.generators.Generators.{NonBlank, nonEmptyStrings, sentenceContaining} -import io.renku.graph.model.projects -import io.renku.graph.model.testentities._ +import io.renku.generators.Generators.{NonBlank, nonEmptyStrings} +import io.renku.generators.jsonld.JsonLDGenerators.entityIds +import io.renku.graph.model.testentities.{schema, _} import io.renku.testtools.IOSpec import io.renku.triplesstore.SparqlQuery.Prefixes +import io.renku.triplesstore.client.model.Quad import io.renku.triplesstore.client.sparql.LuceneQuery +import io.renku.triplesstore.client.syntax._ +import org.scalacheck.Gen import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -37,54 +39,62 @@ class LuceneQueryEncoderSpec with ProjectsDataset with should.Matchers { + private val specialChars = + List[NonBlank]("\\", "+", "-", "&", "|", "!", "(", ")", "{", "}", "[", "]", "^", "\"", "~", "*", "?", ":", "/") + "queryAsString" should { - List[NonBlank]("\\", - "+", - "-", - "&", - "|", - "!", - "(", - ")", - "{", - "}", - "[", - "]", - "^", - "\"", - "~", - "*", - "?", - ":", - "/" - ) foreach { specialChar => + specialChars foreach { specialChar => s"escape '$specialChar' so it can be used as a string in the Lucene search" in { - val query = s"$specialChar${nonEmptyStrings(minLength = 3).generateOne}" - val name = sentenceContaining(Refined.unsafeApply(query)).generateAs(projects.Name) - val project = renkuProjectEntities(visibilityPublic) - .modify(replaceProjectName(name)) - .generateOne + val name = s"$specialChar${nonEmptyStrings(minLength = 3).generateOne}" + val quad = Quad(entityIds.generateOne, entityIds.generateOne, schema / "name", name.asTripleObject) - upload(to = projectsDataset, project) + insert(to = projectsDataset, quad) runSelect( on = projectsDataset, SparqlQuery.of( "lucene test query", Prefixes of (schema -> "schema", text -> "text"), - s"""SELECT ?name - WHERE { - GRAPH ?g { - ?id text:query (schema:name '${LuceneQuery.escape(query).query}'); - schema:name ?name - } - } - """.stripMargin + sparql"""|SELECT ?name + |WHERE { + | GRAPH ?g { + | ?id text:query (schema:name ${LuceneQuery.escape(name).asTripleObject}); + | schema:name ?name + | } + |}""".stripMargin ) ).unsafeRunSync() shouldBe List(Map("name" -> name.value)) } } } + + "tripleObjectEncoder" should { + + "encode the query as triple object" in { + + val name = s"${specialCharsGen.generateOne}${nonEmptyStrings(minLength = 3).generateOne}" + val quad = Quad(entityIds.generateOne, entityIds.generateOne, schema / "name", name.asTripleObject) + + insert(to = projectsDataset, quad) + + runSelect( + on = projectsDataset, + SparqlQuery.of( + "lucene test query", + Prefixes of (schema -> "schema", text -> "text"), + sparql"""|SELECT ?name + |WHERE { + | GRAPH ?g { + | ?id text:query (schema:name ${LuceneQuery.fuzzy(name).asTripleObject}); + | schema:name ?name + | } + |}""".stripMargin + ) + ).unsafeRunSync() shouldBe List(Map("name" -> name.value)) + } + } + + private lazy val specialCharsGen: Gen[NonBlank] = Gen.oneOf(specialChars) } diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQuery.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQuery.scala index 1fb3361f0c..ff41296204 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQuery.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQuery.scala @@ -18,6 +18,9 @@ package io.renku.triplesstore.client.sparql +import cats.syntax.all._ +import io.renku.triplesstore.client.model.TripleObjectEncoder +import io.renku.triplesstore.client.syntax._ import org.apache.lucene.queryparser.flexible.standard.QueryParserUtil final class LuceneQuery(val query: String) extends AnyVal { @@ -41,6 +44,6 @@ object LuceneQuery { .mkString(" ") ) - implicit val sparqlEncoder: SparqlEncoder[LuceneQuery] = - SparqlEncoder.instance(q => Fragment(s"'${q.query}'")) + implicit val tripleObjectEncoder: TripleObjectEncoder[LuceneQuery] = + TripleObjectEncoder[String].contramap(_.query) } diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQueryEncoder.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQueryEncoder.scala index f01cdc8af2..e18d902e31 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQueryEncoder.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/LuceneQueryEncoder.scala @@ -22,5 +22,5 @@ import org.apache.lucene.queryparser.flexible.standard.QueryParserUtil private object LuceneQueryEncoder { - def queryAsString(v: String): String = QueryParserUtil.escape(v).replace("\\", "\\\\") + def queryAsString(v: String): String = QueryParserUtil.escape(v) } diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/StringInterpolator.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/StringInterpolator.scala index e32394bab5..034d345cd8 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/StringInterpolator.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/sparql/StringInterpolator.scala @@ -36,7 +36,7 @@ class StringInterpolator(private val sc: StringContext) { } private lazy val makeValue: ((Any, Int)) => String = { - case (a: LuceneQuery, _) => a.asSparql.sparql + case (a: LuceneQuery, _) => a.asTripleObject.asSparql.sparql case (a: String, _) => a.asTripleObject.asSparql.sparql case (a: Char, _) => a.toString.asTripleObject.asSparql.sparql case (a: Float, _) => a.asTripleObject.asSparql.sparql diff --git a/triples-store-client/src/test/scala/io/renku/triplesstore/client/SyntaxTest.scala b/triples-store-client/src/test/scala/io/renku/triplesstore/client/SyntaxTest.scala index 9dbdeb6eb8..b05d4b3db4 100644 --- a/triples-store-client/src/test/scala/io/renku/triplesstore/client/SyntaxTest.scala +++ b/triples-store-client/src/test/scala/io/renku/triplesstore/client/SyntaxTest.scala @@ -29,7 +29,8 @@ class SyntaxTest extends AnyFlatSpec with should.Matchers { it should "interpolate a lucene query as string" in { val query = LuceneQuery.escape("this is a query?") - fr"?id text:query (schema:name $query)" shouldBe Fragment(s"?id text:query (schema:name '${query.query}')") + fr"?id text:query (schema:name $query)" shouldBe + Fragment(s"?id text:query (schema:name ${query.query.asTripleObject.asSparql.sparql})") } it should "interpolate strings" in { diff --git a/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/StringInterpolatorSpec.scala b/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/StringInterpolatorSpec.scala index dbb12e0fdc..6f43c93168 100644 --- a/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/StringInterpolatorSpec.scala +++ b/triples-store-client/src/test/scala/io/renku/triplesstore/client/sparql/StringInterpolatorSpec.scala @@ -34,7 +34,7 @@ class StringInterpolatorSpec extends AnyWordSpec with should.Matchers { "encode a LuceneQuery if used" in { val value = LuceneQuery(s"{${nonEmptyStrings(minLength = 3).generateOne}") - fr"$value" shouldBe value.asSparql + fr"$value" shouldBe value.asTripleObject.asSparql } "encode a String if used" in { From 0102c0d2c9e27e589ad25a5c5081b4be36734e33 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Fri, 10 Nov 2023 13:55:39 +0100 Subject: [PATCH 07/13] feat: Secret config reader to work with base64 and ASCII values (#1774) --- .../scala/io/renku/config/ConfigLoader.scala | 23 +++++++-- .../scala/io/renku/crypto/AesCrypto.scala | 26 ++++++---- .../renku/cache/CacheConfigLoaderSpec.scala | 3 +- .../scala/io/renku/crypto/SecretSpec.scala | 47 +++++++++++++++---- .../generators/CommonGraphGenerators.scala | 10 ++-- .../repository/AccessTokenCryptoSpec.scala | 2 +- .../crypto/HookTokenCryptoSpec.scala | 2 +- 7 files changed, 84 insertions(+), 29 deletions(-) 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 69919a3635..f9f9b27422 100644 --- a/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala +++ b/graph-commons/src/main/scala/io/renku/config/ConfigLoader.scala @@ -78,10 +78,25 @@ object ConfigLoader { .getOrElse(Left(CannotConvert(stringValue, ttApply.getClass.toString, "Not an int value"))) } - implicit def base64ByteVectorReader: ConfigReader[ByteVector] = - ConfigReader.fromString { str => + def asciiByteVectorReader: ConfigReader[ByteVector] = + ConfigReader.fromString { v => ByteVector - .fromBase64Descriptive(str) - .leftMap(err => new FailureReason { override lazy val description: String = s"Cannot read base64: $err" }) + .encodeAscii(v) + .leftMap(err => + new FailureReason { + override lazy val description: String = s"Only ASCII characters allowed: $err" + } + ) + } + + def base64ByteVectorReader: ConfigReader[ByteVector] = + ConfigReader.fromString { v => + ByteVector + .fromBase64Descriptive(v) + .leftMap(err => + new FailureReason { + override lazy val description: String = s"Not a valid Base64 value: $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 774ef9ffa0..88e097fb65 100644 --- a/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala +++ b/graph-commons/src/main/scala/io/renku/crypto/AesCrypto.scala @@ -20,12 +20,13 @@ package io.renku.crypto import cats.MonadThrow import cats.syntax.all._ -import io.renku.config.ConfigLoader.base64ByteVectorReader +import io.renku.config.ConfigLoader.{asciiByteVectorReader, base64ByteVectorReader} import io.renku.crypto.AesCrypto.Secret import pureconfig.ConfigReader import pureconfig.error.FailureReason import scodec.bits.ByteVector +import java.nio.charset.CharacterCodingException import java.nio.charset.StandardCharsets.UTF_8 import java.util.Base64 import javax.crypto.Cipher @@ -63,31 +64,40 @@ abstract class AesCrypto[F[_]: MonadThrow, NONENCRYPTED, ENCRYPTED](secret: Secr object AesCrypto { final class Secret private (val value: ByteVector) extends AnyVal { - override def toString: String = "" - def toBase64: String = value.toBase64 + override def toString: String = "" + def decodeAscii: Either[CharacterCodingException, String] = value.decodeAscii } object Secret { def apply(value: ByteVector): Either[String, Secret] = { val expectedLengths = List(16L, 24L, 32L) - if (expectedLengths.contains(value.length)) Right(new Secret(value)) + if (expectedLengths contains value.length) Right(new Secret(value)) else Left(s"Expected ${expectedLengths.mkString(" or ")} bytes, but got ${value.length}") } def unsafe(value: ByteVector): Secret = apply(value).fold(sys.error, identity) - def fromBase64(b64: String): Either[String, Secret] = + private 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 => + base64SecretReader orElse asciiSecretReader + + private lazy val base64SecretReader = + base64ByteVectorReader.emap { bv => + Secret(bv.takeWhile(_ != 10)) + .leftMap(err => new FailureReason { override lazy val description: String = s"Cannot read AES secret: $err" }) + } + + private lazy val asciiSecretReader = + asciiByteVectorReader.emap { bv => + Secret(bv).leftMap(err => new FailureReason { override lazy val description: String = s"Cannot read AES secret: $err" } ) - ) + } } } diff --git a/graph-commons/src/test/scala/io/renku/cache/CacheConfigLoaderSpec.scala b/graph-commons/src/test/scala/io/renku/cache/CacheConfigLoaderSpec.scala index 1531f3fbe0..84b326b7e2 100644 --- a/graph-commons/src/test/scala/io/renku/cache/CacheConfigLoaderSpec.scala +++ b/graph-commons/src/test/scala/io/renku/cache/CacheConfigLoaderSpec.scala @@ -27,6 +27,7 @@ import scala.concurrent.duration._ class CacheConfigLoaderSpec extends AnyWordSpec with should.Matchers { "CacheConfigLoader" should { + "load from given config" in { val cfg = ConfigFactory.parseString("""evict-strategy = oldest |ignore-empty-values = true @@ -39,7 +40,7 @@ class CacheConfigLoaderSpec extends AnyWordSpec with should.Matchers { |""".stripMargin) val cc = CacheConfigLoader.unsafeRead(cfg) - cc shouldBe CacheConfig(EvictStrategy.Oldest, true, 5.seconds, Periodic(1000, 10.minutes)) + cc shouldBe CacheConfig(EvictStrategy.Oldest, ignoreEmptyValues = true, 5.seconds, Periodic(1000, 10.minutes)) } } } diff --git a/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala b/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala index 0df838cb22..f7953c3ff2 100644 --- a/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala +++ b/graph-commons/src/test/scala/io/renku/crypto/SecretSpec.scala @@ -23,6 +23,7 @@ import io.renku.crypto.AesCrypto.Secret import io.renku.generators.CommonGraphGenerators.aesCryptoSecrets import io.renku.generators.Generators.Implicits._ import org.scalacheck.Gen +import org.scalacheck.Gen.hexChar import org.scalatest.EitherValues import org.scalatest.matchers.should import org.scalatest.wordspec.AnyWordSpec @@ -31,44 +32,72 @@ import pureconfig.ConfigSource import scodec.bits.Bases.Alphabets import scodec.bits.ByteVector +import scala.jdk.CollectionConverters._ + class SecretSpec extends AnyWordSpec with should.Matchers with EitherValues with ScalaCheckPropertyChecks { "secretReader" should { val keyName = "secret" - "decode Base64 encoded secret" in { + "read an ASCII secret" in { + forAll(aesCryptoSecrets) { secret => - val config = ConfigFactory.parseString(s"""$keyName = "${secret.toBase64}"""") + val config = ConfigFactory.parseMap( + Map(keyName -> secret.decodeAscii.fold(throw _, identity)).asJava + ) - ConfigSource.fromConfig(config).at(keyName).load[Secret].value.toBase64 shouldBe secret.toBase64 + ConfigSource.fromConfig(config).at(keyName).load[Secret].value shouldBe secret } } - "decode Base64 encoded secret ending with LF char" in { + // we need to keep this functionality due to backward compatibility + // where Base64 encoded values were coming to the reader + "read a Base64 encoded secret" in { + + val secret = generateHexSecret + + val config = ConfigFactory.parseMap(Map(keyName -> secret.toBase64).asJava) + + ConfigSource.fromConfig(config).at(keyName).load[Secret].value.value shouldBe secret.takeWhile(_ != 10) + } + + "decode Base64 encoded secret ending with a LF char" in { val secret = aesCryptoSecrets.generateOne - val config = ConfigFactory.parseString(s"""$keyName = "${(secret.value :+ 10.toByte).toBase64}"""") + val config = ConfigFactory.parseMap( + Map(keyName -> (secret.value :+ 10.toByte).toBase64).asJava + ) - ConfigSource.fromConfig(config).at(keyName).load[Secret].value.toBase64 shouldBe secret.toBase64 + ConfigSource.fromConfig(config).at(keyName).load[Secret].value shouldBe secret } "fail and not print the secret if decoding fails" in { - val value = Gen + val secret = Gen .listOfN(2, Gen.hexChar) .map(_.mkString.toLowerCase) .map(ByteVector.fromValidHex(_, Alphabets.HexLowercase)) .generateOne .toBase64 - val config = ConfigFactory.parseString(s"""$keyName = "$value"""") + val config = ConfigFactory.parseMap(Map(keyName -> secret).asJava) val failure = ConfigSource.fromConfig(config).at(keyName).load[Secret].left.value.prettyPrint() failure should include("Cannot read AES secret") - failure should not include value + failure should not include secret } } + + private def generateHexSecret = { + val secretLength = Gen.oneOf(16, 24, 32).generateOne * 2 + hexChar + .toGeneratorOfList(min = secretLength, max = secretLength) + .map(_.mkString.toLowerCase) + .map(ByteVector.fromValidHex(_, Alphabets.HexLowercase)) + .retryUntil(_.takeWhile(_ != 10.toByte).length == secretLength / 2) + .generateOne + } } 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 45927fd280..bfb50d5578 100644 --- a/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala +++ b/graph-commons/src/test/scala/io/renku/generators/CommonGraphGenerators.scala @@ -49,8 +49,8 @@ import io.renku.microservices.{MicroserviceBaseUrl, MicroserviceIdentifier} import io.renku.triplesstore._ import org.http4s.Status import org.http4s.Status._ +import org.scalacheck.Gen.asciiPrintableChar import org.scalacheck.{Arbitrary, Gen} -import scodec.bits.Bases.Alphabets import scodec.bits.ByteVector import scala.language.implicitConversions @@ -63,10 +63,10 @@ object CommonGraphGenerators { .oneOf(16, 24, 32) .flatMap { length => Gen - .listOfN(length * 2, Gen.hexChar) - .map(_.mkString.toLowerCase) - .map(ByteVector.fromValidHex(_, Alphabets.HexLowercase)) - .retryUntil(_.takeWhile(_ != 10.toByte).length == length) // this is to prevent LF chars to be generated + .listOfN(length, asciiPrintableChar) + .map(_.mkString) + .map(_.getBytes("US-ASCII")) + .map(ByteVector(_)) .map(Secret.unsafe) } 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 723c925ee0..84f403625d 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 @@ -86,7 +86,7 @@ class AccessTokenCryptoSpec extends AnyWordSpec with should.Matchers with TableD val config = ConfigFactory.parseMap( Map( "projects-tokens" -> Map( - "secret" -> aesCryptoSecrets.generateOne.toBase64 + "secret" -> aesCryptoSecrets.generateOne.decodeAscii.fold(throw _, identity) ).asJava ).asJava ) 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 7524dddeed..c1d261b02e 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 @@ -80,7 +80,7 @@ class HookTokenCryptoSpec extends AnyWordSpec with should.Matchers with TryValue Map( "services" -> Map( "gitlab" -> Map( - "hook-token-secret" -> aesCryptoSecrets.generateOne.toBase64 + "hook-token-secret" -> aesCryptoSecrets.generateOne.decodeAscii.fold(throw _, identity) ).asJava ).asJava ).asJava From a6fd5e1047a913ecf099bb4fc3acef702d1e2942 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Fri, 10 Nov 2023 16:52:54 +0100 Subject: [PATCH 08/13] feat: role filter to replace owned in the Cross-Entity Search API (#1786) --- .../io/renku/entities/search/Criteria.scala | 2 +- .../renku/entities/search/DatasetsQuery.scala | 10 +- .../renku/entities/search/PersonsQuery.scala | 20 +- .../renku/entities/search/ProjectsQuery.scala | 10 +- .../entities/search/WorkflowsQuery.scala | 10 +- .../entities/search/EntitiesFinderSpec.scala | 174 +++++++++++------- knowledge-graph/README.md | 19 +- .../knowledgegraph/MicroserviceRoutes.scala | 30 +-- .../knowledgegraph/QueryParamDecoders.scala | 13 +- .../entities/EndpointDocs.scala | 18 +- .../MicroserviceRoutesSpec.scala | 34 +++- .../projectauth/util/SparqlSnippets.scala | 17 +- .../io/renku/projectauth/Generators.scala | 2 +- .../scala/io/renku/graph/model/projects.scala | 24 ++- .../graph/model/RenkuTinyTypeGenerators.scala | 5 +- .../generators/EntitiesGenerators.scala | 4 +- .../RenkuProjectEntitiesGenerators.scala | 2 +- .../consumers/membersync/Generators.scala | 5 +- 18 files changed, 243 insertions(+), 156 deletions(-) diff --git a/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala b/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala index d79224b46f..b1e465949e 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/Criteria.scala @@ -42,7 +42,7 @@ object Criteria { final case class Filters(maybeQuery: Option[Filters.Query] = None, entityTypes: Set[Filters.EntityType] = Set.empty, creators: Set[persons.Name] = Set.empty, - maybeOwned: Option[Filters.Owned] = None, + roles: Set[projects.Role] = Set.empty, visibilities: Set[projects.Visibility] = Set.empty, namespaces: Set[projects.Namespace] = Set.empty, maybeSince: Option[Filters.Since] = None, diff --git a/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala index 50aec95dbd..c010116426 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/DatasetsQuery.scala @@ -134,15 +134,15 @@ object DatasetsQuery extends EntityQuery[Entity.Dataset] { } private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = - filters.maybeOwned match { - case Some(Criteria.Filters.Owned(ownerId)) => + maybeUser.map(_.id) -> filters.roles match { + case Some(userId) -> roles if roles.nonEmpty => fr"""|$projIdVar renku:projectVisibility ?visibility . |${visibilitiesPart(filters.visibilities)} - |${authSnippets.ownedProjects(ownerId)} + |${authSnippets.projectsWithRoles(userId, roles)} |""".stripMargin - case None => + case maybeUserId -> _ => fr"""|$projIdVar renku:projectVisibility ?visibility . - |${authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities)} + |${authSnippets.visibleProjects(maybeUserId, filters.visibilities)} |""".stripMargin } diff --git a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala index 22ed06be57..5c7a91db24 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/PersonsQuery.scala @@ -22,7 +22,8 @@ import io.circe.Decoder import io.renku.entities.search.Criteria.Filters.EntityType import io.renku.entities.search.model.{Entity, MatchingScore} import io.renku.graph.model.entities.Person.gitLabSameAsAdditionalType -import io.renku.graph.model.{GraphClass, persons} +import io.renku.graph.model.{GraphClass, persons, projects} +import io.renku.http.server.security.model.AuthUser import io.renku.triplesstore.client.sparql.{Fragment, VarName} import io.renku.triplesstore.client.syntax._ @@ -46,7 +47,7 @@ private case object PersonsQuery extends EntityQuery[model.Entity.Person] { | GRAPH ${GraphClass.Persons.id} { | ?id a schema:Person; | schema:name ?name. - | ${filterOnOwned(criteria.filters.maybeOwned)} + | ${filterOnRoles(criteria)} | } | ${filters.maybeOnCreatorName(VarName("name"))} | } @@ -64,13 +65,20 @@ private case object PersonsQuery extends EntityQuery[model.Entity.Person] { matchingScoreVariableName = VarName("score") ) - private lazy val filterOnOwned: Option[Criteria.Filters.Owned] => Fragment = { - case Some(Criteria.Filters.Owned(userId)) => + private def filterOnRoles(criteria: Criteria): Fragment = criteria.maybeUser -> criteria.filters.roles match { + case Some(AuthUser(id, _)) -> roles if roles contains projects.Role.Owner => fr"""|?id schema:sameAs ?sameAsId. |?sameAsId schema:additionalType ${gitLabSameAsAdditionalType.asTripleObject}; - | schema:identifier ${userId.asObject}. + | schema:identifier ${id.asObject}. |""".stripMargin - case None => Fragment.empty + case _ -> roles if roles.nonEmpty => + // this is a hack to filter out all the persons + // the assumption is that the caller cannot have an explicit Maintainer or Reader role on a Person + fr"""|FILTER EXISTS { + | ?id renku:nonexisting true. + |} + |""".stripMargin + case _ => Fragment.empty } override def decoder[EE >: Entity.Person]: Decoder[EE] = { implicit cursor => diff --git a/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala index 220b4cffdc..ccfa090efd 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/ProjectsQuery.scala @@ -139,15 +139,15 @@ private case object ProjectsQuery extends EntityQuery[model.Entity.Project] { } private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = - filters.maybeOwned match { - case Some(Criteria.Filters.Owned(ownerId)) => + maybeUser.map(_.id) -> filters.roles match { + case Some(userId) -> roles if roles.nonEmpty => fr"""|$projectIdVar renku:projectVisibility $visibilityVar . |${visibilitiesPart(filters.visibilities)} - |${authSnippets.ownedProjects(ownerId)} + |${authSnippets.projectsWithRoles(userId, roles)} |""".stripMargin - case None => + case maybeUserId -> _ => fr"""|$projectIdVar renku:projectVisibility $visibilityVar . - |${authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities)} + |${authSnippets.visibleProjects(maybeUserId, filters.visibilities)} |""".stripMargin } diff --git a/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala b/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala index 0b6c3255a3..d2de89926f 100644 --- a/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala +++ b/entities-search/src/main/scala/io/renku/entities/search/WorkflowsQuery.scala @@ -114,11 +114,11 @@ private case object WorkflowsQuery extends EntityQuery[model.Entity.Workflow] { } private def accessRightsAndVisibility(maybeUser: Option[AuthUser], filters: Criteria.Filters): Fragment = - filters.maybeOwned match { - case Some(Criteria.Filters.Owned(ownerId)) => - authSnippets.ownedProjects(ownerId) - case None => - authSnippets.visibleProjects(maybeUser.map(_.id), filters.visibilities) + maybeUser.map(_.id) -> filters.roles match { + case Some(userId) -> roles if roles.nonEmpty => + authSnippets.projectsWithRoles(userId, roles) + case maybeUserId -> _ => + authSnippets.visibleProjects(maybeUserId, filters.visibilities) } private lazy val maybeFilterOnVisibilities: Set[projects.Visibility] => Fragment = { diff --git a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala index d0d657b014..64023066ca 100644 --- a/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala +++ b/entities-search/src/test/scala/io/renku/entities/search/EntitiesFinderSpec.scala @@ -29,13 +29,13 @@ import io.renku.entities.search.Generators._ import io.renku.entities.search.diff.SearchDiffInstances import io.renku.entities.search.model.Entity import io.renku.entities.searchgraphs.SearchInfoDatasets -import io.renku.generators.CommonGraphGenerators.sortingDirections +import io.renku.generators.CommonGraphGenerators.{authUsers, sortingDirections} import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.model._ import io.renku.graph.model.projects.Visibility import io.renku.graph.model.testentities.generators.EntitiesGenerators -import io.renku.graph.model.testentities.{Dataset, Project, StepPlan} +import io.renku.graph.model.testentities.{Dataset, StepPlan} import io.renku.graph.model.tools.AdditionalMatchers import io.renku.http.rest.paging.PagingRequest import io.renku.http.rest.paging.model._ @@ -546,170 +546,216 @@ class EntitiesFinderSpec } } - "findEntities - with owned filter" should { + "findEntities - with role filter" should { - "return project entities where the given user is an owner" in new TestCase { + "return project entities where the given user has the given role" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val project = renkuProjectEntities(anyVisibility) - .modify(replaceMembers(Set(owner))) + .modify(replaceMembers(Set(member))) .generateOne val results = IOBody { provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> - finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + finder.findEntities( + Criteria(Filters(roles = Set(member.role)), maybeUser = authUsers.generateOne.copy(id = memberId).some) + ) } val expected = List( - project.to[model.Entity.Project], - owner.person.to[model.Entity.Person] - ).sortBy(_.name)(nameOrdering) + project.to[model.Entity.Project].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected } - "return project entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + "return project entities where the given user has the given role - case when filtering on visibility is set" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val publicProject = renkuProjectEntities(visibilityPublic) - .modify(replaceMembers(Set(owner))) + .modify(replaceMembers(Set(member))) .generateOne val nonPublicProject = renkuProjectEntities(visibilityNonPublic) - .modify(replaceMembers(Set(owner))) + .modify(replaceMembers(Set(member))) .generateOne val results = IOBody { provisionTestProjects(publicProject, nonPublicProject) *> finder.findEntities( - Criteria(Filters(maybeOwned = Owned(ownerId).some, visibilities = Set(projects.Visibility.Public))) + Criteria(Filters(roles = Set(member.role), visibilities = Set(projects.Visibility.Public)), + maybeUser = authUsers.generateOne.copy(id = memberId).some + ) ) } val expected = List( - publicProject.to[model.Entity.Project], - owner.person.to[model.Entity.Person] - ).sortBy(_.name)(nameOrdering) + publicProject.to[model.Entity.Project].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected } - "return dataset entities where the given user is an owner" in new TestCase { + "return dataset entities where the given user has the given role" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val dsAndProject @ _ -> project = renkuProjectEntities(anyVisibility) - .modify(replaceMembers(Set(owner))) + .modify(replaceMembers(Set(member))) .addDataset( datasetEntities(provenanceNonModified) - .modify(replaceDSCreators(NonEmptyList.of(personEntities.generateOne, owner.person))) + .modify(replaceDSCreators(NonEmptyList.of(personEntities.generateOne, member.person))) ) .generateOne val results = IOBody { provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> - finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + finder.findEntities( + Criteria(Filters(roles = Set(member.role)), maybeUser = authUsers.generateOne.copy(id = memberId).some) + ) } val expected = List( - project.to[model.Entity.Project], - dsAndProject.to[model.Entity.Dataset], - owner.person.to[model.Entity.Person] - ).sortBy(_.name)(nameOrdering) + project.to[model.Entity.Project].some, + dsAndProject.to[model.Entity.Dataset].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected } - "return dataset entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + "return dataset entities where the given user has the given role - case when filtering on visibility is set" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val dsAndPublicProject @ _ -> publicProject = renkuProjectEntities(visibilityPublic) - .modify(replaceMembers(Set(owner))) - .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(owner.person)))) + .modify(replaceMembers(Set(member))) + .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(member.person)))) .generateOne val _ -> nonPublicProject = renkuProjectEntities(visibilityNonPublic) - .modify(replaceMembers(Set(owner))) - .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(owner.person)))) + .modify(replaceMembers(Set(member))) + .addDataset(datasetEntities(provenanceNonModified).modify(replaceDSCreators(NonEmptyList.of(member.person)))) .generateOne val results = IOBody { provisionTestProjects(publicProject, nonPublicProject) *> finder.findEntities( - Criteria( - Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some, visibilities = Set(projects.Visibility.Public)) + Criteria(Filters(roles = Set(member.role), visibilities = Set(projects.Visibility.Public)), + maybeUser = authUsers.generateOne.copy(id = memberId).some ) ) } val expected = List( - publicProject.to[model.Entity.Project], - dsAndPublicProject.to[model.Entity.Dataset], - owner.person.to[model.Entity.Person] - ).sortBy(_.name)(nameOrdering) + publicProject.to[model.Entity.Project].some, + dsAndPublicProject.to[model.Entity.Dataset].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected } - "return workflow entities where the given user is an owner" in new TestCase { + "return workflow entities where the given user has the given role" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val project = renkuProjectEntities(anyVisibility) - .modify(replaceMembers(Set(owner))) + .modify(replaceMembers(Set(member))) .withActivities( - activityEntities(stepPlanEntities().map(_.replaceCreators(List(personEntities.generateOne, owner.person)))) + activityEntities(stepPlanEntities().map(_.replaceCreators(List(personEntities.generateOne, member.person)))) ) .generateOne val results = IOBody { provisionTestProjects(project, projectEntities(visibilityPublic).generateOne) *> - finder.findEntities(Criteria(Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some))) + finder.findEntities( + Criteria(Filters(roles = Set(member.role)), maybeUser = authUsers.generateOne.copy(id = memberId).some) + ) } val expected = List( - project.to[model.Entity.Project], - (project.plans.head -> project).to[model.Entity.Workflow], - owner.person.to[model.Entity.Person] - ).sortBy(_.name)(nameOrdering) + project.to[model.Entity.Project].some, + (project.plans.head -> project).to[model.Entity.Workflow].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected } - "return workflow entities where the given user is an owner - case when filtering on visibility is set" in new TestCase { + "return workflow entities where the given user has the given role - case when filtering on visibility is set" in new TestCase { - val ownerId = personGitLabIds.generateOne - val owner = Project.Member(personEntities(ownerId.some).generateOne, projects.Role.Owner) + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne val publicProject = renkuProjectEntities(visibilityPublic) - .modify(replaceMembers(Set(owner))) - .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(owner.person))))) + .modify(replaceMembers(Set(member))) + .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(member.person))))) .generateOne val nonPublicProject = renkuProjectEntities(visibilityNonPublic) - .modify(replaceMembers(Set(owner))) - .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(owner.person))))) + .modify(replaceMembers(Set(member))) + .withActivities(activityEntities(stepPlanEntities().map(_.replaceCreators(List(member.person))))) .generateOne val results = IOBody { provisionTestProjects(publicProject, nonPublicProject) *> + finder.findEntities( + Criteria(Filters(roles = Set(member.role), visibilities = Set(projects.Visibility.Public)), + maybeUser = authUsers.generateOne.copy(id = memberId).some + ) + ) + } + + val expected = List( + publicProject.to[model.Entity.Project].some, + (publicProject.plans.head -> publicProject).to[model.Entity.Workflow].some, + Option.when(member.role == projects.Role.Owner)(member.person.to[model.Entity.Person]) + ).flatten.sortBy(_.name)(nameOrdering) + + results.results shouldMatchTo expected + } + + "return project entities where the given user has one of the given roles" in new TestCase { + + val memberId = personGitLabIds.generateOne + val member = projectMemberEntities(memberId.some).generateOne + + val roleProject1 = projects.Role.Owner + val project1 = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(member.copy(role = roleProject1)))) + .generateOne + val roleProject2 = projects.Role.Maintainer + val project2 = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(member.copy(role = roleProject2)))) + .generateOne + val roleProject3 = projects.Role.Reader + val project3 = renkuProjectEntities(visibilityPublic) + .modify(replaceMembers(Set(member.copy(role = roleProject3)))) + .generateOne + + val results = IOBody { + provisionTestProjects(project1, project2, project3) *> finder.findEntities( Criteria( - Filters(maybeOwned = Criteria.Filters.Owned(ownerId).some, visibilities = Set(projects.Visibility.Public)) + Filters(entityTypes = Set(EntityType.Project), + roles = Set(projects.Role.Maintainer, projects.Role.Reader) + ), + maybeUser = authUsers.generateOne.copy(id = memberId).some ) ) } val expected = List( - publicProject.to[model.Entity.Project], - (publicProject.plans.head -> publicProject).to[model.Entity.Workflow], - owner.person.to[model.Entity.Person] + project2.to[model.Entity.Project], + project3.to[model.Entity.Project] ).sortBy(_.name)(nameOrdering) results.results shouldMatchTo expected diff --git a/knowledge-graph/README.md b/knowledge-graph/README.md index 32d54d31d5..31c4aa1f49 100644 --- a/knowledge-graph/README.md +++ b/knowledge-graph/README.md @@ -300,7 +300,7 @@ Allows finding `projects`, `datasets`, `workflows`, and `persons`. * `query` - to filter by matching field (e.g., title, keyword, description, etc. as specified below) * `type` - to filter by entity type(s); allowed values: `project`, `dataset`, `workflow`, and `person`; multiple `type` parameters allowed * `creator` - to filter by creator(s); the filter would require creator's name; multiple `creator` parameters allowed -* `owned` - to reduce the results to entities where the caller is an owner; this parameter does not require any value +* `role` - to filter by the caller role(s) on the entity; allowed values: `owner`, `maintainer`, `developer`, `reporter` and `guest`; multiple parameters allowed; an authorization header needs to be passed otherwise a `BAD_REQUEST (400)` status will be returned * `visibility` - to filter by visibility(ies) (restricted vs. public); allowed values: `public`, `internal`, `private`; multiple `visibility` parameters allowed * `namespace` - to filter by namespace(s); there might be multiple values given; for nested namespaces the whole path has be used, e.g. `group/subgroup` * `since` - to filter by entity's creation date to >= the given date @@ -327,14 +327,19 @@ When the `query` parameter is given, the match is done on the following fields: * the `page` query parameter is optional and defaults to `1`. * the `per_page` query parameter is optional and defaults to `20`; max value is `100`. +**Security** +The API takes an authorization token (optional) the request header as: +- `Authorization: Bearer ` with oauth token obtained from gitlab +- `PRIVATE-TOKEN: ` with user's personal access token in gitlab + **Response** -| Status | Description | -|------------------------------|------------------------------------------------------------------------------------------------| -| OK (200) | If results are found; `[]` if nothing is found | -| BAD_REQUEST (400) | If illegal values for query parameters are given or `owned` specified but no auth user present | -| UNAUTHORIZED (401) | If given auth header cannot be authenticated | -| INTERNAL SERVER ERROR (500) | Otherwise | +| Status | Description | +|------------------------------|-----------------------------------------------------------------------------------------------| +| OK (200) | If results are found; `[]` if nothing is found | +| BAD_REQUEST (400) | If illegal values for query parameters are given or `role` specified but no auth user present | +| UNAUTHORIZED (401) | If given auth header cannot be authenticated | +| INTERNAL SERVER ERROR (500) | Otherwise | Response headers: diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala index 5fefa874dc..e92d1050ed 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/MicroserviceRoutes.scala @@ -135,11 +135,11 @@ private class MicroserviceRoutes[F[_]: Async]( AuthedRoutes.of { case req @ GET -> Root / "knowledge-graph" / "entities" - :? query(maybeQuery) +& entityTypes(maybeTypes) +& creatorNames(maybeCreators) +& owned(maybeOwned) + :? query(maybeQuery) +& entityTypes(maybeTypes) +& creatorNames(maybeCreators) +& roles(maybeRoles) +& visibilities(maybeVisibilities) +& namespaces(maybeNamespaces) +& since(maybeSince) +& until(maybeUntil) +& sort(maybeSort) +& page(maybePage) +& perPage(maybePerPage) as maybeUser => - searchForEntities(maybeQuery, maybeTypes, maybeCreators, maybeOwned, maybeVisibilities, maybeNamespaces, + searchForEntities(maybeQuery, maybeTypes, maybeCreators, maybeRoles, maybeVisibilities, maybeNamespaces, maybeSince, maybeUntil, maybeSort, maybePage, maybePerPage, maybeUser.option, req.req) } } @@ -226,7 +226,7 @@ private class MicroserviceRoutes[F[_]: Async]( maybeQuery: Option[ValidatedNel[ParseFailure, EntitiesSearchCriteria.Filters.Query]], types: ValidatedNel[ParseFailure, List[EntitiesSearchCriteria.Filters.EntityType]], creators: ValidatedNel[ParseFailure, List[persons.Name]], - maybeOwned: Boolean, + roles: ValidatedNel[ParseFailure, List[model.projects.Role]], visibilities: ValidatedNel[ParseFailure, List[model.projects.Visibility]], namespaces: ValidatedNel[ParseFailure, List[model.projects.Namespace]], maybeSince: Option[ValidatedNel[ParseFailure, EntitiesSearchCriteria.Filters.Since]], @@ -243,10 +243,11 @@ private class MicroserviceRoutes[F[_]: Async]( maybeQuery.map(_.map(Option.apply)).getOrElse(Validated.validNel(Option.empty[Query])), types.map(_.toSet), creators.map(_.toSet), - maybeOwned -> maybeUser match { - case (true, Some(authUser)) => Owned(authUser.id).some.validNel[ParseFailure] - case (true, None) => ParseFailure("'owned' parameter present but no access token", "").invalidNel[Option[Owned]] - case _ => Option.empty[Owned].validNel[ParseFailure] + maybeUser -> roles match { + case (Some(_), roles) if roles.isValid && roles.map(_.nonEmpty).getOrElse(false) => roles.map(_.toSet) + case (None, roles) if roles.isValid && roles.map(_.nonEmpty).getOrElse(false) => + ParseFailure("'role' parameter present but no access token", "").invalidNel[Set[model.projects.Role]] + case (_, roles) => roles.map(_.toSet) }, visibilities.map(_.toSet), namespaces.map(_.toSet), @@ -263,21 +264,10 @@ private class MicroserviceRoutes[F[_]: Async]( } .getOrElse(().validNel[ParseFailure]) ).mapN { - case (maybeQuery, - types, - creators, - maybeOwned, - visibilities, - namespaces, - maybeSince, - maybeUntil, - sorting, - paging, - _ - ) => + case (maybeQuery, types, creators, roles, visibilities, namespaces, maybeSince, maybeUntil, sorting, paging, _) => `GET /entities`( EntitiesSearchCriteria( - Filters(maybeQuery, types, creators, maybeOwned, visibilities, namespaces, maybeSince, maybeUntil), + Filters(maybeQuery, types, creators, roles, visibilities, namespaces, maybeSince, maybeUntil), sorting, paging, maybeUser diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala index ec88f18b64..9b5722fc11 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/QueryParamDecoders.scala @@ -24,7 +24,7 @@ import io.renku.entities.search.Criteria.Filters._ import io.renku.entities.viewings.search.RecentEntitiesFinder import io.renku.graph.model._ import io.renku.http.rest.paging.model.PerPage -import org.http4s.dsl.io.{FlagQueryParamMatcher, OptionalMultiQueryParamDecoderMatcher, OptionalValidatingQueryParamDecoderMatcher} +import org.http4s.dsl.io.{OptionalMultiQueryParamDecoderMatcher, OptionalValidatingQueryParamDecoderMatcher} import org.http4s.{ParseFailure, QueryParamDecoder, QueryParameterValue} import java.time.LocalDate @@ -85,8 +85,15 @@ object QueryParamDecoders { val parameterName: String = "creator" } - object owned extends FlagQueryParamMatcher("owned") { - val parameterName: String = "owned" + private implicit val roleParameterDecoder: QueryParamDecoder[projects.Role] = + (value: QueryParameterValue) => + projects.Role + .from(value.value) + .leftMap(_ => parsingFailure(roles.parameterName)) + .toValidatedNel + + object roles extends OptionalMultiQueryParamDecoderMatcher[projects.Role]("role") { + val parameterName: String = "role" } private implicit val visibilityParameterDecoder: QueryParamDecoder[projects.Visibility] = diff --git a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala index ac515fc815..5cb0bb19e9 100644 --- a/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala +++ b/knowledge-graph/src/main/scala/io/renku/knowledgegraph/entities/EndpointDocs.scala @@ -18,7 +18,7 @@ package io.renku.knowledgegraph.entities -import cats.MonadThrow +import cats.{MonadThrow, Show} import cats.implicits._ import eu.timepit.refined.auto._ import io.circe.Json @@ -34,6 +34,7 @@ import io.renku.knowledgegraph.docs import io.renku.knowledgegraph.docs.model.Operation.GET import io.renku.knowledgegraph.docs.model._ import io.renku.knowledgegraph.entities.ModelEncoders._ +import io.renku.tinytypes.TinyType import java.time.Instant @@ -50,13 +51,13 @@ private class EndpointDocsImpl()(implicit gitLabUrl: GitLabUrl, renkuApiUrl: ren GET( "Cross-Entity Search", "Finds entities by the given criteria", - Uri / "entities" :? query & `type` & creator & owned & visibility & namespace & since & until & sort & page & perPage, + Uri / "entities" :? query & `type` & creator & role & visibility & namespace & since & until & sort & page & perPage, Status.Ok -> Response("Found entities", Contents(MediaType.`application/json`("Sample response", example)), responseHeaders ), Status.BadRequest -> Response( - "In case of invalid query parameters or `owned` parameter specified but no auth user present", + "In case of invalid query parameters or `role` specified but no auth user present", Contents(MediaType.`application/json`("Reason", Message.Info("Invalid parameters"))) ), Status.Unauthorized -> Response( @@ -87,16 +88,16 @@ private class EndpointDocsImpl()(implicit gitLabUrl: GitLabUrl, renkuApiUrl: ren "to filter by creator(s); the filter would require creator's name; multiple creator parameters allowed".some, required = false ) - private lazy val owned = Parameter.Query( - "owned", + private lazy val role = Parameter.Query( + "role", Schema.String, - "to reduce the results to entities where the caller is an owner; this parameter does not require any value".some, + s"to filter by the caller role(s) on the entity; allowed values: ${toAllowedOptions(projects.Role.all.toList)}; multiple parameters allowed; an authorization header needs to be passed otherwise a `BAD_REQUEST (400)` status will be returned".some, required = false ) private lazy val visibility = Parameter.Query( "visibility", Schema.String, - "to filter by visibility(ies) (restricted vs. public); allowed values: 'public', 'internal', 'private'; multiple visibility parameters allowed".some, + s"to filter by visibility(ies) (restricted vs. public); allowed values: ${toAllowedOptions(projects.Visibility.all)}; multiple parameters allowed".some, required = false ) private lazy val namespace = Parameter.Query( @@ -136,6 +137,9 @@ private class EndpointDocsImpl()(implicit gitLabUrl: GitLabUrl, renkuApiUrl: ren required = false ) + private def toAllowedOptions[TT <: TinyType: Show](options: Iterable[TT]) = + options.map(v => show"'$v'").mkString(", ") + private lazy val responseHeaders = Map( "Total" -> Header("The total number of entities".some, Schema.Integer), "Total-Pages" -> Header("The total number of pages".some, Schema.Integer), diff --git a/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala b/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala index 800e801a36..a7a5a9ee14 100644 --- a/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala +++ b/knowledge-graph/src/test/scala/io/renku/knowledgegraph/MicroserviceRoutesSpec.scala @@ -394,12 +394,13 @@ class MicroserviceRoutesSpec } } - "read the 'owned' query parameter from the uri and pass it to the endpoint when auth user present" in new TestCase { + "read the 'role' query parameter from the uri and pass it to the endpoint when auth user present" in new TestCase { val authUser = authUsers.generateOne val maybeAuthUser = MaybeAuthUser(authUser) - val criteria = Criteria(Filters(maybeOwned = Filters.Owned(authUser.id).some), maybeUser = maybeAuthUser.option) - val request = Request[IO](GET, uri"/knowledge-graph/entities" +? "owned") + val role = projectRoles.generateOne + val criteria = Criteria(Filters(roles = Set(role)), maybeUser = maybeAuthUser.option) + val request = Request[IO](GET, uri"/knowledge-graph/entities" +? ("role" -> role)) val responseBody = jsons.generateOne (entitiesEndpoint.`GET /entities` _) @@ -413,13 +414,34 @@ class MicroserviceRoutesSpec response.body[Json] shouldBe responseBody } - s"return $BadRequest 'owned' query parameter given but no auth user present" in new TestCase { + "read multiple 'role' query parameters from the uri and pass it to the endpoint when auth user present" in new TestCase { - val response = routes().call(Request[IO](GET, uri"/knowledge-graph/entities" +? "owned")) + val authUser = authUsers.generateOne + val maybeAuthUser = MaybeAuthUser(authUser) + val roles = projectRoles.generateSet(min = 2) + val criteria = Criteria(Filters(roles = roles), maybeUser = maybeAuthUser.option) + val request = Request[IO](GET, uri"/knowledge-graph/entities" ++? ("role" -> roles.toList.map(_.show))) + + val responseBody = jsons.generateOne + (entitiesEndpoint.`GET /entities` _) + .expects(criteria, request) + .returning(Response[IO](Ok).withEntity(responseBody).pure[IO]) + + val response = routes(maybeAuthUser).call(request) + + response.status shouldBe Ok + response.contentType shouldBe Some(`Content-Type`(application.json)) + response.body[Json] shouldBe responseBody + } + + s"return $BadRequest when the 'role' query parameter given but no auth user present" in new TestCase { + + val response = + routes().call(Request[IO](GET, uri"/knowledge-graph/entities" +? ("role" -> projectRoles.generateOne))) response.status shouldBe BadRequest response.contentType shouldBe Some(`Content-Type`(application.json)) - response.body[Message] shouldBe Message.Error("'owned' parameter present but no access token") + response.body[Message] shouldBe Message.Error("'role' parameter present but no access token") } s"return $BadRequest for invalid parameter values" in new TestCase { diff --git a/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala b/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala index 76dc5481e8..f5c37f269d 100644 --- a/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala +++ b/project-auth/src/main/scala/io/renku/projectauth/util/SparqlSnippets.scala @@ -46,12 +46,17 @@ final class SparqlSnippets(val projectId: VarName) { |} | """.stripMargin - def ownedProjects(userId: persons.GitLabId): Fragment = - fr"""|GRAPH renku:ProjectAuth { - | $projectId a schema:Project; - | renku:memberRole ${ProjectMember(userId, projects.Role.Owner).encoded}. - |} - | """.stripMargin + def projectsWithRoles(userId: persons.GitLabId, roles: Set[projects.Role]): Fragment = + roles match { + case rls if rls.isEmpty => Fragment.empty + case rls => + fr"""|GRAPH renku:ProjectAuth { + | $projectId a schema:Project; + | renku:memberRole ?memberRole. + | VALUES (?memberRole) { ${rls.map(ProjectMember(userId, _).encoded)} } + |} + | """.stripMargin + } def visibleProjects(userId: Option[persons.GitLabId], selectedVisibility: Set[Visibility]): Fragment = { val visibilities = diff --git a/project-auth/src/test/scala/io/renku/projectauth/Generators.scala b/project-auth/src/test/scala/io/renku/projectauth/Generators.scala index 80676712eb..2a83b054c0 100644 --- a/project-auth/src/test/scala/io/renku/projectauth/Generators.scala +++ b/project-auth/src/test/scala/io/renku/projectauth/Generators.scala @@ -31,7 +31,7 @@ object Generators { Gen.oneOf(fixedIds).map(persons.GitLabId.apply) val memberGen: Gen[ProjectMember] = for { - role <- RenkuTinyTypeGenerators.roleGen + role <- RenkuTinyTypeGenerators.projectRoles id <- gitLabIds } yield ProjectMember(id, role) diff --git a/renku-model-tiny-types/src/main/scala/io/renku/graph/model/projects.scala b/renku-model-tiny-types/src/main/scala/io/renku/graph/model/projects.scala index d50959b087..2d373639f7 100644 --- a/renku-model-tiny-types/src/main/scala/io/renku/graph/model/projects.scala +++ b/renku-model-tiny-types/src/main/scala/io/renku/graph/model/projects.scala @@ -23,7 +23,7 @@ import cats.kernel.Order import cats.syntax.all._ import eu.timepit.refined.api.Refined import eu.timepit.refined.numeric.Positive -import io.circe.{Decoder, Encoder} +import io.circe.Decoder import io.renku.graph.model.views.{EntityIdJsonLDOps, NonBlankTTJsonLDOps, TinyTypeJsonLDOps, UrlResourceRenderer} import io.renku.jsonld.{EntityId, JsonLDDecoder, JsonLDEncoder} import io.renku.tinytypes._ @@ -232,20 +232,20 @@ object projects { ) } - sealed trait Role extends Ordered[Role] { - def asString: String + sealed trait Role extends StringTinyType with Product with Ordered[Role] { + def asString: String = value } - object Role { + object Role extends TinyTypeFactory[Role](RoleInstantiator) { case object Owner extends Role { - val asString = "owner" + override val value: String = "owner" override def compare(that: Role): Int = if (that == this) 0 else 1 } case object Maintainer extends Role { - val asString = "maintainer" + override val value: String = "maintainer" override def compare(that: Role): Int = if (that == this) 0 @@ -254,7 +254,7 @@ object projects { } case object Reader extends Role { - val asString = "reader" + override val value: String = "reader" override def compare(that: Role): Int = if (that == this) 0 @@ -271,7 +271,7 @@ object projects { fromString(str).fold(sys.error, identity) /** Translated from here: https://docs.gitlab.com/ee/api/members.html#roles */ - def fromGitLabAccessLevel(accessLevel: Int) = + def fromGitLabAccessLevel(accessLevel: Int): Role = accessLevel match { case n if n >= 50 => Owner case n if n >= 40 => Maintainer @@ -293,8 +293,12 @@ object projects { implicit val jsonDecoder: Decoder[Role] = Decoder.decodeString.emap(fromString) + } - implicit val jsonEncoder: Encoder[Role] = - Encoder.encodeString.contramap(_.asString) + private object RoleInstantiator extends (String => Role) { + override def apply(value: String): Role = + Role + .fromString(value) + .getOrElse(throw new IllegalArgumentException(s"'$value' unknown Role")) } } diff --git a/renku-model-tiny-types/src/test/scala/io/renku/graph/model/RenkuTinyTypeGenerators.scala b/renku-model-tiny-types/src/test/scala/io/renku/graph/model/RenkuTinyTypeGenerators.scala index 13428b7ab9..512075a85b 100644 --- a/renku-model-tiny-types/src/test/scala/io/renku/graph/model/RenkuTinyTypeGenerators.scala +++ b/renku-model-tiny-types/src/test/scala/io/renku/graph/model/RenkuTinyTypeGenerators.scala @@ -23,7 +23,6 @@ import io.renku.generators.Generators import io.renku.generators.Generators.Implicits._ import io.renku.generators.Generators._ import io.renku.graph.model.images.{ImageResourceId, ImageUri} -import io.renku.graph.model.projects.Role import io.renku.graph.model.versions.{CliVersion, SchemaVersion} import io.renku.tinytypes.InstantTinyType import org.scalacheck.Gen @@ -35,9 +34,6 @@ import scala.util.Random trait RenkuTinyTypeGenerators { - val roleGen: Gen[Role] = - Gen.oneOf(Role.all.toList) - def associationResourceIdGen: Gen[associations.ResourceId] = Generators.validatedUrls.map(_.value).map(associations.ResourceId) @@ -125,6 +121,7 @@ trait RenkuTinyTypeGenerators { implicit val projectDescriptions: Gen[projects.Description] = Generators.paragraphs() map (v => projects.Description(v.value)) implicit val projectVisibilities: Gen[projects.Visibility] = Gen.oneOf(projects.Visibility.all.toList) + implicit val projectRoles: Gen[projects.Role] = Gen.oneOf(projects.Role.all.toList) def projectCreatedDates(min: Instant = Instant.EPOCH): Gen[projects.DateCreated] = Generators.timestamps(min, max = Instant.now()).toGeneratorOf(projects.DateCreated) diff --git a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/EntitiesGenerators.scala b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/EntitiesGenerators.scala index fa060718db..519bb5767d 100644 --- a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/EntitiesGenerators.scala +++ b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/EntitiesGenerators.scala @@ -98,7 +98,7 @@ trait EntitiesGenerators ): Gen[GitLabMember] = for { user <- gitLabUserGen(gitLabIds, maybeEmails) - role <- roleGen + role <- projectRoles } yield GitLabMember(user, Role.toGitLabAccessLevel(role)) def personEntities( @@ -118,7 +118,7 @@ trait EntitiesGenerators ): Gen[Project.Member] = for { p <- personEntities(maybeGitLabIds, maybeEmails) - role <- roleGen + role <- projectRoles } yield Project.Member(p, role) def replacePersonName(to: persons.Name): Person => Person = _.copy(name = to) diff --git a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/RenkuProjectEntitiesGenerators.scala b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/RenkuProjectEntitiesGenerators.scala index c51068d294..fdd6f57c94 100644 --- a/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/RenkuProjectEntitiesGenerators.scala +++ b/renku-model/src/test/scala/io/renku/graph/model/testentities/generators/RenkuProjectEntitiesGenerators.scala @@ -148,7 +148,7 @@ trait RenkuProjectEntitiesGenerators { name <- personNames username <- personUsernames gitLabId <- personGitLabIds - role <- roleGen + role <- projectRoles } yield GitLabMember(name, username, gitLabId, None, Role.toGitLabAccessLevel(role)) implicit lazy val projectMembersWithEmail: Gen[GitLabMember] = for { diff --git a/triples-generator/src/test/scala/io/renku/triplesgenerator/events/consumers/membersync/Generators.scala b/triples-generator/src/test/scala/io/renku/triplesgenerator/events/consumers/membersync/Generators.scala index 3286b1173a..e8ff059f14 100644 --- a/triples-generator/src/test/scala/io/renku/triplesgenerator/events/consumers/membersync/Generators.scala +++ b/triples-generator/src/test/scala/io/renku/triplesgenerator/events/consumers/membersync/Generators.scala @@ -18,8 +18,7 @@ package io.renku.triplesgenerator.events.consumers.membersync -import io.renku.graph.model.GraphModelGenerators.{personGitLabIds, personNames} -import io.renku.graph.model.RenkuTinyTypeGenerators +import io.renku.graph.model.GraphModelGenerators.{personGitLabIds, personNames, projectRoles} import io.renku.graph.model.projects.Role import org.scalacheck.Gen @@ -28,7 +27,7 @@ private trait Generators { implicit val gitLabProjectMembers: Gen[GitLabProjectMember] = for { id <- personGitLabIds name <- personNames - role <- RenkuTinyTypeGenerators.roleGen + role <- projectRoles } yield GitLabProjectMember(id, name, Role.toGitLabAccessLevel(role)) } From 362ce91e1e8426cb7f14ba7d7cdd0c06ac0cc33f Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Fri, 10 Nov 2023 18:17:29 +0100 Subject: [PATCH 09/13] chore: KG's README fixed --- knowledge-graph/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/knowledge-graph/README.md b/knowledge-graph/README.md index 31c4aa1f49..5103076e53 100644 --- a/knowledge-graph/README.md +++ b/knowledge-graph/README.md @@ -300,7 +300,7 @@ Allows finding `projects`, `datasets`, `workflows`, and `persons`. * `query` - to filter by matching field (e.g., title, keyword, description, etc. as specified below) * `type` - to filter by entity type(s); allowed values: `project`, `dataset`, `workflow`, and `person`; multiple `type` parameters allowed * `creator` - to filter by creator(s); the filter would require creator's name; multiple `creator` parameters allowed -* `role` - to filter by the caller role(s) on the entity; allowed values: `owner`, `maintainer`, `developer`, `reporter` and `guest`; multiple parameters allowed; an authorization header needs to be passed otherwise a `BAD_REQUEST (400)` status will be returned +* `role` - to filter by the caller role(s) on the entity; allowed values: `owner`, `maintainer` and `reader`; multiple parameters allowed; an authorization header needs to be passed otherwise a `BAD_REQUEST (400)` status will be returned * `visibility` - to filter by visibility(ies) (restricted vs. public); allowed values: `public`, `internal`, `private`; multiple `visibility` parameters allowed * `namespace` - to filter by namespace(s); there might be multiple values given; for nested namespaces the whole path has be used, e.g. `group/subgroup` * `since` - to filter by entity's creation date to >= the given date From 95315c9c4da527a4756bd953e8a20f1e02bbd326 Mon Sep 17 00:00:00 2001 From: RenkuBot <53332360+RenkuBot@users.noreply.github.com> Date: Wed, 15 Nov 2023 07:46:16 +0100 Subject: [PATCH 10/13] chore: Update http4s-circe, http4s-dsl, ... from 0.23.23 to 0.23.24 (#1788) Co-authored-by: RenkuBot --- project/Dependencies.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/project/Dependencies.scala b/project/Dependencies.scala index 704c48cc53..4bb14ab1b4 100644 --- a/project/Dependencies.scala +++ b/project/Dependencies.scala @@ -15,8 +15,8 @@ object Dependencies { val circeOptics = "0.15.0" val diffx = "0.9.0" val fs2 = "3.9.3" - val http4s = "0.23.23" - val http4sEmber = "0.23.23" + val http4s = "0.23.24" + val http4sEmber = "0.23.24" val http4sPrometheus = "0.24.6" val ip4s = "3.4.0" val jsonld4s = "0.13.0" From 496c151fb35bbe93e042273ff98af55724757b02 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Wed, 15 Nov 2023 10:44:04 +0100 Subject: [PATCH 11/13] fix: Postgres lock to renew connection on EofException (#1789) --- .../scala/io/renku/lock/PostgresLock.scala | 20 ++++- .../io/renku/interpreters/TestLogger.scala | 4 + .../io/renku/lock/PostgresLockSpec.scala | 81 +++++++++++++++---- 3 files changed, 85 insertions(+), 20 deletions(-) diff --git a/graph-commons/src/main/scala/io/renku/lock/PostgresLock.scala b/graph-commons/src/main/scala/io/renku/lock/PostgresLock.scala index 375609e9f9..84079f8cb3 100644 --- a/graph-commons/src/main/scala/io/renku/lock/PostgresLock.scala +++ b/graph-commons/src/main/scala/io/renku/lock/PostgresLock.scala @@ -18,13 +18,14 @@ package io.renku.lock -import cats.Applicative import cats.data.Kleisli import cats.effect._ import cats.syntax.all._ +import cats.{Applicative, ApplicativeThrow} import org.typelevel.log4cats.Logger import skunk._ import skunk.codec.all._ +import skunk.exception.EofException import skunk.implicits._ import scala.concurrent.duration._ @@ -42,11 +43,18 @@ object PostgresLock { createPolling[F, A](session, interval, tryAdvisoryLockSql, advisoryUnlockSql) /** Obtains an exclusive lock, retrying periodically via non-blocking waits */ - def exclusive[F[_]: Temporal: Logger, A: LongKey]( + def exclusive[F[_]: ApplicativeThrow: Temporal: Logger, A: LongKey]( session: Resource[F, Session[F]], interval: FiniteDuration = 0.5.seconds ): Lock[F, A] = - Kleisli(key => session.flatMap(exclusive_[F, A](_, interval).run(key))) + Kleisli { key => + def createLock: Resource[F, Unit] = + session + .flatMap(exclusive_[F, A](_, interval).run(key)) + .handleErrorWith[Unit, Throwable](_ => Resource.eval(Temporal[F].sleep(interval * 2)) >> createLock) + + createLock + } /** Obtains a shared lock, retrying periodically via non-blocking waits. */ def shared_[F[_]: Temporal: Logger, A: LongKey]( @@ -74,7 +82,11 @@ object PostgresLock { .attempt .flatMap { case Right(v) => v.pure[F] - case Left(ex) => Logger[F].warn(ex)(s"Acquiring postgres advisory lock failed! Retry in $interval.").as(false) + case Left(ex) if ex.isInstanceOf[EofException] => + Logger[F].warn(ex)("Session get closed while acquiring postgres advisory lock! Renewing connection.") >> + ex.raiseError[F, Boolean] + case Left(ex) => + Logger[F].warn(ex)(s"Acquiring postgres advisory lock failed! Retry in $interval.").as(false) } .flatTap { case false => diff --git a/graph-commons/src/test/scala/io/renku/interpreters/TestLogger.scala b/graph-commons/src/test/scala/io/renku/interpreters/TestLogger.scala index ac4b5d639b..4d5bcee443 100644 --- a/graph-commons/src/test/scala/io/renku/interpreters/TestLogger.scala +++ b/graph-commons/src/test/scala/io/renku/interpreters/TestLogger.scala @@ -57,6 +57,10 @@ class TestLogger[F[_]: Async] extends Logger[F] with should.Matchers { def logged(expected: LogEntry*): Assertion = invocations should contain allElementsOf expected + def loggedF(expected: LogEntry*): F[Assertion] = F.delay { + invocations should contain allElementsOf expected + } + def notLogged(expected: LogEntry): Assertion = invocations should not contain expected diff --git a/graph-commons/src/test/scala/io/renku/lock/PostgresLockSpec.scala b/graph-commons/src/test/scala/io/renku/lock/PostgresLockSpec.scala index d1fe06d4b2..95ec90ced1 100644 --- a/graph-commons/src/test/scala/io/renku/lock/PostgresLockSpec.scala +++ b/graph-commons/src/test/scala/io/renku/lock/PostgresLockSpec.scala @@ -22,16 +22,18 @@ import cats.effect._ import cats.effect.std.CountDownLatch import cats.effect.testing.scalatest.AsyncIOSpec import cats.syntax.all._ +import com.dimafeng.testcontainers.PostgreSQLContainer import fs2.{Pipe, concurrent} import io.renku.interpreters.TestLogger import org.scalatest.matchers.should import org.scalatest.wordspec.AsyncWordSpec import org.typelevel.log4cats.Logger +import skunk._ import skunk.data._ +import skunk.exception.EofException +import skunk.implicits._ import skunk.net.protocol.{Describe, Parse} import skunk.util.Typer -import skunk._ -import skunk.implicits._ import scala.concurrent.duration._ @@ -106,22 +108,11 @@ class PostgresLockSpec extends AsyncWordSpec with AsyncIOSpec with should.Matche } yield () } - "log if acquiring fails" in withContainers { cnt => + "log and retry if acquiring fails" in withContainers { cnt => implicit val logger: TestLogger[IO] = TestLogger() - val exception = new Exception("boom") - val makeSession = (Resource.eval(Ref.of[IO, Int](0)), session(cnt)).mapN { (counter, goodSession) => - new PostgresLockSpec.TestSession { - override def unique[A, B](query: Query[A, B])(args: A) = - counter.getAndUpdate(_ + 1).flatMap { - case n if n < 2 => IO.raiseError(exception) - case _ => goodSession.unique(query)(args) - } - - override def execute[A](command: Command[A])(args: A) = - goodSession.execute(command)(args) - } - } + val exception = new Exception("boom") + val makeSession = createFailingSessionThatRecovers(cnt, exception) val interval = 50.millis def lock = makeSession.map(PostgresLock.exclusive_[IO, String](_, interval)) @@ -135,6 +126,28 @@ class PostgresLockSpec extends AsyncWordSpec with AsyncIOSpec with should.Matche ) } } + + "recreate the lock with a new session once the connection has been closed" in withContainers { cnt => + implicit val logger: TestLogger[IO] = TestLogger() + + val exception = EofException(bytesRequested = 1, bytesRead = 0) + val sessionResource = createFailingSessionThatDoesNotRecover(cnt, exception) + + val interval = 50.millis + + val lock = PostgresLock.exclusive[IO, String](sessionResource, interval) + + for { + _ <- session(cnt).use(resetLockTable) + res <- lock.run("p1").use(_ => IO.unit).assertNoException + _ <- logger.loggedF( + TestLogger.Level.Warn( + "Session get closed while acquiring postgres advisory lock! Renewing connection.", + exception + ) + ) + } yield res + } } "PostgresLock.shared" should { @@ -231,6 +244,42 @@ class PostgresLockSpec extends AsyncWordSpec with AsyncIOSpec with should.Matche } } } + + private def createFailingSessionThatRecovers(cnt: PostgreSQLContainer, + exception: Exception + ): Resource[IO, Session[IO]] = + (Resource.eval(Ref.of[IO, Int](0)), session(cnt)) + .mapN { (counter, goodSession) => + new PostgresLockSpec.TestSession { + override def unique[A, B](query: Query[A, B])(args: A) = + counter.getAndUpdate(_ + 1) >>= { + case n if n < 2 => IO.raiseError(exception) + case _ => goodSession.unique(query)(args) + } + + override def execute[A](command: Command[A])(args: A) = + goodSession.execute(command)(args) + } + } + + private def createFailingSessionThatDoesNotRecover(cnt: PostgreSQLContainer, + exception: Exception + ): Resource[IO, Session[IO]] = { + val counter = Ref.unsafe[IO, Int](0) + + session(cnt).map { goodSession => + new PostgresLockSpec.TestSession { + override def unique[A, B](query: Query[A, B])(args: A) = + counter.getAndUpdate(_ + 1) >>= { + case n if n < 2 => IO.raiseError(exception) + case _ => goodSession.unique(query)(args) + } + + override def execute[A](command: Command[A])(args: A) = + goodSession.execute(command)(args) + } + } + } } object PostgresLockSpec { From 536bf68147901588c8595d9121d85777b02915b1 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Wed, 15 Nov 2023 11:29:19 +0100 Subject: [PATCH 12/13] chore: http4s client init warn muted -> idle timeout 10% more than timeout --- .../scala/io/renku/http/client/RestClient.scala | 12 +++++++++--- .../scala/io/renku/triplesstore/TSClientImpl.scala | 13 +++++++------ 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/graph-commons/src/main/scala/io/renku/http/client/RestClient.scala b/graph-commons/src/main/scala/io/renku/http/client/RestClient.scala index 480ff5e167..26e2344f0b 100644 --- a/graph-commons/src/main/scala/io/renku/http/client/RestClient.scala +++ b/graph-commons/src/main/scala/io/renku/http/client/RestClient.scala @@ -117,9 +117,15 @@ abstract class RestClient[F[_]: Async: Logger, ThrottlingTarget]( private def httpClientBuilder: EmberClientBuilder[F] = { implicit val network: Network[F] = Network.forAsync(Async[F]) - val clientBuilder = EmberClientBuilder.default[F] - val updatedIdleTimeout = idleTimeoutOverride map clientBuilder.withIdleConnectionTime getOrElse clientBuilder - requestTimeoutOverride map updatedIdleTimeout.withTimeout getOrElse updatedIdleTimeout + val clientBuilder = EmberClientBuilder.default[F] + val updatedIdleTimeout = idleTimeoutOverride map clientBuilder.withIdleConnectionTime getOrElse clientBuilder + val updatedBothTimeouts = requestTimeoutOverride map updatedIdleTimeout.withTimeout getOrElse updatedIdleTimeout + idleTimeoutOverride -> requestTimeoutOverride match { + case None -> None => + // in the case no timeout overrides given, the idle timeout is set at 10% more than the timeout to mute the http4s warning + updatedBothTimeouts withIdleConnectionTime updatedBothTimeouts.timeout * 1.1 + case _ => updatedBothTimeouts + } } private def measureExecutionTime[ResultType](block: => F[ResultType], request: HttpRequest[F]): F[ResultType] = diff --git a/graph-commons/src/main/scala/io/renku/triplesstore/TSClientImpl.scala b/graph-commons/src/main/scala/io/renku/triplesstore/TSClientImpl.scala index 908080cfab..40143c37c9 100644 --- a/graph-commons/src/main/scala/io/renku/triplesstore/TSClientImpl.scala +++ b/graph-commons/src/main/scala/io/renku/triplesstore/TSClientImpl.scala @@ -64,12 +64,13 @@ class TSClientImpl[F[_]: Async: Logger: SparqlQueryTimeRecorder]( maxRetries: Int Refined NonNegative = MaxRetriesAfterConnectionTimeout, timeout: Duration = 20.minutes, printQueries: Boolean = false -) extends RestClient(Throttler.noThrottling, - Option(implicitly[SparqlQueryTimeRecorder[F]].instance), - retryInterval, - maxRetries, - timeout.some, - timeout.some +) extends RestClient( + Throttler.noThrottling, + Option(implicitly[SparqlQueryTimeRecorder[F]].instance), + retryInterval, + maxRetries, + idleTimeoutOverride = (timeout * 1.1).some, + requestTimeoutOverride = timeout.some ) with TSClient[F] with RdfMediaTypes { From e351fccfc322441376f424ac8ec455e80818a537 Mon Sep 17 00:00:00 2001 From: Jakub Chrobasik Date: Wed, 15 Nov 2023 12:23:57 +0100 Subject: [PATCH 13/13] chore: idle timeout 10% more than timeout on FusekiClient --- .../io/renku/triplesstore/client/http/FusekiClient.scala | 5 ++--- .../io/renku/triplesstore/client/http/SparqlClient.scala | 1 + 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/FusekiClient.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/FusekiClient.scala index 8d3402b5ac..68e0b8d846 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/FusekiClient.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/FusekiClient.scala @@ -49,8 +49,7 @@ object FusekiClient { EmberClientBuilder .default[F] .withTimeout(timeout) + .withIdleConnectionTime(timeout * 1.1) .build - .map { c => - new DefaultFusekiClient[F](c, connectionConfig) - } + .map(new DefaultFusekiClient[F](_, connectionConfig)) } diff --git a/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/SparqlClient.scala b/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/SparqlClient.scala index 6bb0c20294..97c88e122b 100644 --- a/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/SparqlClient.scala +++ b/triples-store-client/src/main/scala/io/renku/triplesstore/client/http/SparqlClient.scala @@ -55,6 +55,7 @@ object SparqlClient { EmberClientBuilder .default[F] .withTimeout(timeout) + .withIdleConnectionTime(timeout * 1.1) .build .map(c => new DefaultSparqlClient[F](c, connectionConfig)) }