From a1318229002883ca2f9a7f98fd388ecc12d7167f Mon Sep 17 00:00:00 2001 From: Eike Kettner Date: Wed, 23 Oct 2024 10:56:41 +0200 Subject: [PATCH] Refactor querying for renku enitities - Move additional query constraints into single place (SearchSolrClient) - Have a separate class for converting a user query into the final solr query (to be easier to test and have a dedicated place for this important piece) - Add more tests for query amendments --- .../search/solr/client/RenkuEntityQuery.scala | 75 +++++++++++++++++++ .../solr/client/SearchSolrClientImpl.scala | 35 +-------- .../solr/query/LuceneQueryInterpreter.scala | 11 +-- .../renku/search/solr/query/SolrQuery.scala | 25 ------- .../renku/search/solr/query/SolrToken.scala | 2 +- .../solr/client/RenkuEntityQuerySpec.scala | 69 +++++++++++++++++ .../query/LuceneQueryInterpreterSpec.scala | 25 ------- .../io/renku/solr/client/QueryData.scala | 4 +- .../io/renku/solr/client/SolrClientImpl.scala | 8 +- 9 files changed, 156 insertions(+), 98 deletions(-) create mode 100644 modules/search-solr-client/src/main/scala/io/renku/search/solr/client/RenkuEntityQuery.scala create mode 100644 modules/search-solr-client/src/test/scala/io/renku/search/solr/client/RenkuEntityQuerySpec.scala diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/RenkuEntityQuery.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/RenkuEntityQuery.scala new file mode 100644 index 00000000..17736497 --- /dev/null +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/RenkuEntityQuery.scala @@ -0,0 +1,75 @@ +/* + * Copyright 2024 Swiss Data Science Center (SDSC) + * A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and + * Eidgenössische Technische Hochschule Zürich (ETHZ). + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.renku.search.solr.client + +import io.renku.search.solr.SearchRole +import io.renku.search.solr.documents.DocumentKind +import io.renku.search.solr.query.SolrQuery +import io.renku.search.solr.query.SolrToken +import io.renku.search.solr.schema.EntityDocumentSchema +import io.renku.solr.client.* +import io.renku.solr.client.facet.Facet +import io.renku.solr.client.facet.Facets +import io.renku.solr.client.schema.FieldName + +/** Convert the user query into a final query that is send to SOLR. */ +object RenkuEntityQuery: + private val typeTerms = Facet.Terms( + EntityDocumentSchema.Fields.entityType, + EntityDocumentSchema.Fields.entityType + ) + + private val creatorDetails: FieldName = FieldName("creatorDetails") + private val namespaceDetails: FieldName = FieldName("namespaceDetails") + + def apply(role: SearchRole, sq: SolrQuery, limit: Int, offset: Int): QueryData = + QueryData(QueryString(sq.query.value, limit, offset)) + .addFilter( + SolrToken.kindIs(DocumentKind.FullEntity).value + ) + .addFilter(constrainRole(role).map(_.value)*) + .withSort(sq.sort) + .withFacet(Facets(typeTerms)) + .withFields(FieldName.all, FieldName.score) + .addSubQuery( + creatorDetails, + SubQuery( + "{!terms f=id v=$row.createdBy}", + "{!terms f=_kind v=fullentity}", + 1 + ).withFields(FieldName.all) + ) + .addSubQuery( + namespaceDetails, + SubQuery( + "{!terms f=namespace v=$row.namespace}", + "(_type:User OR _type:Group) AND _kind:fullentity", + 1 + ).withFields(FieldName.all) + ) + + private def constrainRole(role: SearchRole) = role match + case SearchRole.Anonymous => + Seq(SolrToken.publicOnly) + + case SearchRole.User(id) => + Seq(SolrToken.forUser(id)) + + case SearchRole.Admin(_) => + Seq.empty diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/SearchSolrClientImpl.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/SearchSolrClientImpl.scala index 2d10bae0..e89e170b 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/SearchSolrClientImpl.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/client/SearchSolrClientImpl.scala @@ -30,10 +30,7 @@ import io.renku.search.solr.SearchRole import io.renku.search.solr.documents.* import io.renku.search.solr.query.LuceneQueryInterpreter import io.renku.search.solr.query.SolrToken -import io.renku.search.solr.schema.EntityDocumentSchema import io.renku.solr.client.* -import io.renku.solr.client.facet.{Facet, Facets} -import io.renku.solr.client.schema.FieldName private class SearchSolrClientImpl[F[_]: Async](solrClient: SolrClient[F]) extends SearchSolrClient[F]: @@ -41,13 +38,6 @@ private class SearchSolrClientImpl[F[_]: Async](solrClient: SolrClient[F]) private val logger = scribe.cats.effect[F] private val interpreter = LuceneQueryInterpreter.forSync[F] - private val creatorDetails: FieldName = FieldName("creatorDetails") - private val namespaceDetails: FieldName = FieldName("namespaceDetails") - - private val typeTerms = Facet.Terms( - EntityDocumentSchema.Fields.entityType, - EntityDocumentSchema.Fields.entityType - ) val underlying: SolrClient[F] = solrClient override def upsert[D: Encoder](documents: Seq[D]): F[UpsertResponse] = @@ -67,30 +57,9 @@ private class SearchSolrClientImpl[F[_]: Async](solrClient: SolrClient[F]) ): F[QueryResponse[EntityDocument]] = for { solrQuery <- interpreter(role).run(query) + queryData = RenkuEntityQuery(role, solrQuery, limit, offset) _ <- logger.info(s"Query: '${query.render}' -> Solr: '$solrQuery'") - res <- solrClient - .query[EntityDocument]( - QueryData(QueryString(solrQuery.query.value, limit, offset)) - .withSort(solrQuery.sort) - .withFacet(Facets(typeTerms)) - .withFields(FieldName.all, FieldName.score) - .addSubQuery( - creatorDetails, - SubQuery( - "{!terms f=id v=$row.createdBy}", - "{!terms f=_kind v=fullentity}", - 1 - ).withFields(FieldName.all) - ) - .addSubQuery( - namespaceDetails, - SubQuery( - "{!terms f=namespace v=$row.namespace}", - "(_type:User OR _type:Group) AND _kind:fullentity", - 1 - ).withFields(FieldName.all) - ) - ) + res <- solrClient.query[EntityDocument](queryData) } yield res override def query[D: Decoder](query: QueryData): F[QueryResponse[D]] = diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryInterpreter.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryInterpreter.scala index e44bd92a..d12fb40e 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryInterpreter.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryInterpreter.scala @@ -20,7 +20,6 @@ package io.renku.search.solr.query import cats.Monad import cats.effect.Sync -import cats.syntax.all.* import io.renku.search.query.Query import io.renku.search.solr.SearchRole @@ -34,15 +33,7 @@ final class LuceneQueryInterpreter[F[_]: Monad] private val encoder = SolrTokenEncoder[F, Query] def run(ctx: Context[F], query: Query): F[SolrQuery] = - amendQuery(ctx.role)(encoder.encode(ctx, query)) - - private def amendQuery(role: SearchRole)(sq: F[SolrQuery]): F[SolrQuery] = - sq.map { query => - role match - case SearchRole.Anonymous => query.asAnonymous - case SearchRole.User(id) => query.asUser(id) - case SearchRole.Admin(_) => query.asAdmin - } + encoder.encode(ctx, query) object LuceneQueryInterpreter: def forSync[F[_]: Sync](role: SearchRole): QueryInterpreter.WithContext[F] = diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrQuery.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrQuery.scala index 50bae6d4..50fd39e7 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrQuery.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrQuery.scala @@ -21,9 +21,7 @@ package io.renku.search.solr.query import cats.Monoid import cats.syntax.all.* -import io.renku.search.model.Id import io.renku.search.query.Order -import io.renku.search.solr.documents.DocumentKind import io.renku.solr.client.SolrSort final case class SolrQuery( @@ -34,29 +32,6 @@ final case class SolrQuery( def ++(next: SolrQuery): SolrQuery = SolrQuery(query && next.query, sort ++ next.sort) - def asAnonymous: SolrQuery = - SolrQuery( - List( - query.parens, - SolrToken.publicOnly, - SolrToken.kindIs(DocumentKind.FullEntity) - ).foldAnd, - sort - ) - - def asUser(id: Id): SolrQuery = - SolrQuery( - List( - query.parens, - SolrToken.forUser(id), - SolrToken.kindIs(DocumentKind.FullEntity) - ).foldAnd, - sort - ) - - def asAdmin: SolrQuery = - SolrQuery(List(query, SolrToken.kindIs(DocumentKind.FullEntity)).foldAnd, sort) - object SolrQuery: val empty: SolrQuery = SolrQuery(SolrToken.empty, SolrSort.empty) diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrToken.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrToken.scala index 4a48d801..a96aebb3 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrToken.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrToken.scala @@ -119,7 +119,7 @@ object SolrToken: s"${field.name}:$value" def fieldExists(field: FieldName): SolrToken = - fieldIs(field, "*") + fieldIs(field, "[* TO *]") def unsafeFromString(s: String): SolrToken = s diff --git a/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/RenkuEntityQuerySpec.scala b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/RenkuEntityQuerySpec.scala new file mode 100644 index 00000000..5a9dacb0 --- /dev/null +++ b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/RenkuEntityQuerySpec.scala @@ -0,0 +1,69 @@ +/* + * Copyright 2024 Swiss Data Science Center (SDSC) + * A partnership between École Polytechnique Fédérale de Lausanne (EPFL) and + * Eidgenössische Technische Hochschule Zürich (ETHZ). + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.renku.search.solr.client + +import io.renku.search.model.Id +import io.renku.search.solr.SearchRole +import io.renku.search.solr.documents.DocumentKind +import io.renku.search.solr.query.SolrQuery +import io.renku.search.solr.query.SolrToken +import io.renku.solr.client.QueryData +import io.renku.solr.client.SolrSort +import munit.FunSuite + +class RenkuEntityQuerySpec extends FunSuite: + val adminRole: SearchRole = SearchRole.admin(Id("admin")) + + def query(q: String, role: SearchRole) = + RenkuEntityQuery( + role, + SolrQuery(SolrToken.unsafeFromString(q), SolrSort.empty), + 10, + 0 + ) + + def assertFilter(q: QueryData, fq: String, fqn: String*) = + (fq +: fqn).foreach { f => + assert(q.filter.exists(_ == f), s"Expected filter query not found: $f [$q]") + } + + def assertFilterNot(q: QueryData, fq: String, fqn: String*) = + (fq +: fqn).foreach { f => + assert(q.filter.forall(_ != f), s"Filter query found: $f") + } + + test("amend query with auth data"): + assertFilter( + query("help", SearchRole.user(Id("13"))), + SolrToken.forUser(Id("13")).value + ) + assertFilter( + query("help", SearchRole.Anonymous), + SolrToken.publicOnly.value + ) + assertFilterNot( + query("help", adminRole), + SolrToken.publicOnly.value + ) + + test("only full entities"): + assertFilter( + query("bla", adminRole), + SolrToken.kindIs(DocumentKind.FullEntity).value + ) diff --git a/modules/search-solr-client/src/test/scala/io/renku/search/solr/query/LuceneQueryInterpreterSpec.scala b/modules/search-solr-client/src/test/scala/io/renku/search/solr/query/LuceneQueryInterpreterSpec.scala index c380228e..0d366af0 100644 --- a/modules/search-solr-client/src/test/scala/io/renku/search/solr/query/LuceneQueryInterpreterSpec.scala +++ b/modules/search-solr-client/src/test/scala/io/renku/search/solr/query/LuceneQueryInterpreterSpec.scala @@ -63,31 +63,6 @@ class LuceneQueryInterpreterSpec extends SearchSolrSuite with ScalaCheckEffectSu val q = LuceneQueryInterpreter[Id].run(ctx, userQuery) QueryData(QueryString(q.query.value, 10, 0)).withSort(q.sort) - test("amend query with auth data"): - assertEquals( - query("help", SearchRole.user(model.Id("13"))).query, - "((content_all:help~) AND (visibility:public OR members_all:13) AND _kind:fullentity)" - ) - assertEquals( - query("help", SearchRole.Anonymous).query, - "((content_all:help~) AND visibility:public AND _kind:fullentity)" - ) - assertEquals( - query("help", adminRole).query, - "(content_all:help~ AND _kind:fullentity)" - ) - - test("amend empty query with auth data"): - assertEquals( - query("", SearchRole.user(model.Id("13"))).query, - "((visibility:public OR members_all:13) AND _kind:fullentity)" - ) - assertEquals( - query("", SearchRole.Anonymous).query, - "(visibility:public AND _kind:fullentity)" - ) - assertEquals(query("", adminRole).query, "(_kind:fullentity)") - test("valid content_all query") { IO(solrClientWithSchema()).flatMap { client => List("hello world", "bla:test") diff --git a/modules/solr-client/src/main/scala/io/renku/solr/client/QueryData.scala b/modules/solr-client/src/main/scala/io/renku/solr/client/QueryData.scala index 4a599e83..d6765ffe 100644 --- a/modules/solr-client/src/main/scala/io/renku/solr/client/QueryData.scala +++ b/modules/solr-client/src/main/scala/io/renku/solr/client/QueryData.scala @@ -20,6 +20,7 @@ package io.renku.solr.client import io.bullet.borer.Encoder import io.bullet.borer.derivation.MapBasedCodecs.deriveEncoder +import io.renku.solr.client.SolrSort.Direction import io.renku.solr.client.facet.Facets import io.renku.solr.client.schema.FieldName @@ -38,7 +39,8 @@ final case class QueryData( def withSort(sort: SolrSort): QueryData = copy(sort = sort) def withFields(field: FieldName*) = copy(fields = field) - def addFilter(q: String): QueryData = copy(filter = filter :+ q) + def withFilter(fq: Seq[String]): QueryData = copy(filter = fq) + def addFilter(q: String*): QueryData = copy(filter = filter ++ q) def withFacet(facet: Facets): QueryData = copy(facet = facet) def withLimit(limit: Int): QueryData = copy(limit = limit) def withOffset(offset: Int): QueryData = copy(offset = offset) diff --git a/modules/solr-client/src/main/scala/io/renku/solr/client/SolrClientImpl.scala b/modules/solr-client/src/main/scala/io/renku/solr/client/SolrClientImpl.scala index 684aa188..c94c70e3 100644 --- a/modules/solr-client/src/main/scala/io/renku/solr/client/SolrClientImpl.scala +++ b/modules/solr-client/src/main/scala/io/renku/solr/client/SolrClientImpl.scala @@ -24,6 +24,7 @@ import cats.data.NonEmptyList import cats.effect.Async import cats.syntax.all.* +import io.bullet.borer.Json import io.bullet.borer.{Decoder, Encoder} import io.renku.search.http.borer.BorerEntityJsonCodec import io.renku.search.http.{HttpClientDsl, ResponseLogging} @@ -52,9 +53,10 @@ private class SolrClientImpl[F[_]: Async](val config: SolrConfig, underlying: Cl def query[A: Decoder](query: QueryData): F[QueryResponse[A]] = val req = Method.POST(query, solrUrl / "query").withBasicAuth(credentials) - underlying - .expectOr[QueryResponse[A]](req)(ResponseLogging.Error(logger, req)) - .flatTap(r => logger.trace(s"Query response: $r")) + logger.debug(s"SOLR Query: ${Json.encode(query).toUtf8String}") >> + underlying + .expectOr[QueryResponse[A]](req)(ResponseLogging.Error(logger, req)) + .flatTap(r => logger.trace(s"Query response: $r")) def delete(q: QueryString): F[Unit] = val req = Method.POST(DeleteRequest(q.q), makeUpdateUrl).withBasicAuth(credentials)