From ad8c1ccd26560ca9be0b7387442b22096194e544 Mon Sep 17 00:00:00 2001 From: eikek <701128+eikek@users.noreply.github.com> Date: Fri, 8 Mar 2024 14:49:09 +0100 Subject: [PATCH] fix: Create correct solr query in case only `sort` terms are specfified (#51) - fixes the solr query generated when the user only specifies sort terms - moves `def generateOne` extension methods to commons module - adds a `LoggingConfigure` trait for tests to avoid logging too much stuff. If mixed in, the verbosity is set to `0` meaning only ERROR messages will be printed. It is not applied to all specs --- .../scala/io/renku/logging/LoggingSetup.scala | 6 ++- .../io/renku/search/GeneratorSyntax.scala | 44 +++++++++++++++++ .../io/renku/search/LoggingConfigure.scala | 38 +++++++++++++++ .../redis/client/RedisClientGenerators.scala | 2 - .../redis/client/RedisQueueClientSpec.scala | 1 + .../renku/queue/client/QueueClientSpec.scala | 1 + .../io/renku/search/api/SearchApiSpec.scala | 1 + .../ProjectCreatedProvisionerSpec.scala | 1 + .../user/UserAddedProvisionerSpec.scala | 1 + .../io/renku/search/query/SortableField.scala | 2 + .../renku/search/query/QueryGenerators.scala | 2 + .../solr/query/LuceneQueryEncoders.scala | 6 ++- .../renku/search/solr/query/SolrQuery.scala | 1 + .../search/solr/query/SolrTokenEncoder.scala | 3 ++ .../client/SearchSolrClientGenerators.scala | 8 +--- .../solr/client/SearchSolrClientSpec.scala | 1 + .../query/LuceneQueryInterpreterSpec.scala | 47 ++++++++++++++----- .../io/renku/solr/client/QueryData.scala | 2 + .../solr/client/SolrClientGenerator.scala | 2 - .../io/renku/solr/client/SolrClientSpec.scala | 2 + 20 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 modules/commons/src/test/scala/io/renku/search/GeneratorSyntax.scala create mode 100644 modules/commons/src/test/scala/io/renku/search/LoggingConfigure.scala diff --git a/modules/commons/src/main/scala/io/renku/logging/LoggingSetup.scala b/modules/commons/src/main/scala/io/renku/logging/LoggingSetup.scala index 65645810..e8d92aad 100644 --- a/modules/commons/src/main/scala/io/renku/logging/LoggingSetup.scala +++ b/modules/commons/src/main/scala/io/renku/logging/LoggingSetup.scala @@ -28,7 +28,11 @@ object LoggingSetup: println(s">> Setting up logging with verbosity=$verbosity") val root = scribe.Logger.root.clearHandlers().clearModifiers() verbosity match - case n if n <= 0 => + case n if n < 0 => + () + + case 0 => + root.withMinimumLevel(Level.Error).replace() () case 1 => diff --git a/modules/commons/src/test/scala/io/renku/search/GeneratorSyntax.scala b/modules/commons/src/test/scala/io/renku/search/GeneratorSyntax.scala new file mode 100644 index 00000000..79fc7de6 --- /dev/null +++ b/modules/commons/src/test/scala/io/renku/search/GeneratorSyntax.scala @@ -0,0 +1,44 @@ +/* + * 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 + +import fs2.Stream +import cats.effect.IO +import org.scalacheck.Gen +import cats.arrow.FunctionK + +trait GeneratorSyntax: + + extension [A](self: Gen[A]) + @annotation.tailrec + final def generateOne: A = + self.sample match + case Some(a) => a + case None => generateOne + + def generateSome: Option[A] = Some(generateOne) + + def stream: Stream[Gen, A] = + Stream.repeatEval(self) + + extension [A](self: Stream[Gen, A]) + def toIO: Stream[IO, A] = + self.translate(FunctionK.lift[Gen, IO]([X] => (gx: Gen[X]) => IO(gx.generateOne))) + +object GeneratorSyntax extends GeneratorSyntax diff --git a/modules/commons/src/test/scala/io/renku/search/LoggingConfigure.scala b/modules/commons/src/test/scala/io/renku/search/LoggingConfigure.scala new file mode 100644 index 00000000..d0587ecd --- /dev/null +++ b/modules/commons/src/test/scala/io/renku/search/LoggingConfigure.scala @@ -0,0 +1,38 @@ +/* + * 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 + +import io.renku.logging.LoggingSetup + +trait LoggingConfigure extends munit.FunSuite: + + def defaultVerbosity: Int = 0 + + override def beforeAll(): Unit = + setLoggingVerbosity(defaultVerbosity) + super.beforeAll() + + def setLoggingVerbosity(level: Int): Unit = + LoggingSetup.doConfigure(level) + + def withVerbosity[T](level: Int)(body: => T): T = + val verbosity = defaultVerbosity + LoggingSetup.doConfigure(level) + try body + finally LoggingSetup.doConfigure(verbosity) diff --git a/modules/redis-client/src/test/scala/io/renku/redis/client/RedisClientGenerators.scala b/modules/redis-client/src/test/scala/io/renku/redis/client/RedisClientGenerators.scala index 6c129519..a64ae474 100644 --- a/modules/redis-client/src/test/scala/io/renku/redis/client/RedisClientGenerators.scala +++ b/modules/redis-client/src/test/scala/io/renku/redis/client/RedisClientGenerators.scala @@ -41,5 +41,3 @@ object RedisClientGenerators: part1 <- Gen.chooseNum(3, 10) part2 <- Gen.chooseNum(3, 10) yield MessageId(s"$part1.$part2") - - extension [V](gen: Gen[V]) def generateOne: V = gen.sample.getOrElse(generateOne) diff --git a/modules/redis-client/src/test/scala/io/renku/redis/client/RedisQueueClientSpec.scala b/modules/redis-client/src/test/scala/io/renku/redis/client/RedisQueueClientSpec.scala index 5c7e0502..a0aa56e1 100644 --- a/modules/redis-client/src/test/scala/io/renku/redis/client/RedisQueueClientSpec.scala +++ b/modules/redis-client/src/test/scala/io/renku/redis/client/RedisQueueClientSpec.scala @@ -25,6 +25,7 @@ import dev.profunktor.redis4cats.streams.data.XAddMessage import dev.profunktor.redis4cats.streams.{RedisStream, Streaming} import fs2.* import fs2.concurrent.SignallingRef +import io.renku.search.GeneratorSyntax.* import io.renku.redis.client.RedisClientGenerators.* import io.renku.redis.client.util.RedisSpec import munit.CatsEffectSuite diff --git a/modules/renku-redis-client/src/test/scala/io/renku/queue/client/QueueClientSpec.scala b/modules/renku-redis-client/src/test/scala/io/renku/queue/client/QueueClientSpec.scala index ceb6cca6..9b75fa2d 100644 --- a/modules/renku-redis-client/src/test/scala/io/renku/queue/client/QueueClientSpec.scala +++ b/modules/renku-redis-client/src/test/scala/io/renku/queue/client/QueueClientSpec.scala @@ -28,6 +28,7 @@ import io.renku.queue.client.DataContentType.{Binary, Json} import io.renku.queue.client.Generators.* import io.renku.redis.client.{MessageId, RedisClientGenerators} import io.renku.redis.client.RedisClientGenerators.* +import io.renku.search.GeneratorSyntax.* import munit.CatsEffectSuite class QueueClientSpec extends CatsEffectSuite with QueueSpec: diff --git a/modules/search-api/src/test/scala/io/renku/search/api/SearchApiSpec.scala b/modules/search-api/src/test/scala/io/renku/search/api/SearchApiSpec.scala index 3e60c674..ef46a2af 100644 --- a/modules/search-api/src/test/scala/io/renku/search/api/SearchApiSpec.scala +++ b/modules/search-api/src/test/scala/io/renku/search/api/SearchApiSpec.scala @@ -21,6 +21,7 @@ package io.renku.search.api import cats.effect.IO import cats.syntax.all.* import io.github.arainko.ducktape.* +import io.renku.search.GeneratorSyntax.* import io.renku.search.api.data.* import io.renku.search.model.users import io.renku.search.query.Query diff --git a/modules/search-provision/src/test/scala/io/renku/search/provision/project/ProjectCreatedProvisionerSpec.scala b/modules/search-provision/src/test/scala/io/renku/search/provision/project/ProjectCreatedProvisionerSpec.scala index a5d53511..ebec7d91 100644 --- a/modules/search-provision/src/test/scala/io/renku/search/provision/project/ProjectCreatedProvisionerSpec.scala +++ b/modules/search-provision/src/test/scala/io/renku/search/provision/project/ProjectCreatedProvisionerSpec.scala @@ -31,6 +31,7 @@ import io.renku.queue.client.Generators.messageHeaderGen import io.renku.queue.client.{DataContentType, QueueSpec} import io.renku.redis.client.RedisClientGenerators.* import io.renku.redis.client.{QueueName, RedisClientGenerators} +import io.renku.search.GeneratorSyntax.* import io.renku.search.model.{EntityType, projects, users} import io.renku.search.query.Query import io.renku.search.query.Query.Segment diff --git a/modules/search-provision/src/test/scala/io/renku/search/provision/user/UserAddedProvisionerSpec.scala b/modules/search-provision/src/test/scala/io/renku/search/provision/user/UserAddedProvisionerSpec.scala index cd170a65..93cc526e 100644 --- a/modules/search-provision/src/test/scala/io/renku/search/provision/user/UserAddedProvisionerSpec.scala +++ b/modules/search-provision/src/test/scala/io/renku/search/provision/user/UserAddedProvisionerSpec.scala @@ -31,6 +31,7 @@ import io.renku.queue.client.Generators.messageHeaderGen import io.renku.queue.client.{DataContentType, QueueSpec} import io.renku.redis.client.RedisClientGenerators.* import io.renku.redis.client.{QueueName, RedisClientGenerators} +import io.renku.search.GeneratorSyntax.* import io.renku.search.model.{EntityType, users} import io.renku.search.query.Query import io.renku.search.query.Query.Segment diff --git a/modules/search-query/src/main/scala/io/renku/search/query/SortableField.scala b/modules/search-query/src/main/scala/io/renku/search/query/SortableField.scala index 6fef6940..8ed18cf5 100644 --- a/modules/search-query/src/main/scala/io/renku/search/query/SortableField.scala +++ b/modules/search-query/src/main/scala/io/renku/search/query/SortableField.scala @@ -19,6 +19,7 @@ package io.renku.search.query import io.bullet.borer.{Decoder, Encoder} +import cats.kernel.Order enum SortableField: case Name @@ -30,6 +31,7 @@ enum SortableField: object SortableField: given Encoder[SortableField] = Encoder.forString.contramap(_.name) given Decoder[SortableField] = Decoder.forString.mapEither(fromString) + given Order[SortableField] = Order.by(_.name) private[this] val allNames: String = SortableField.values.map(_.name).mkString(", ") diff --git a/modules/search-query/src/test/scala/io/renku/search/query/QueryGenerators.scala b/modules/search-query/src/test/scala/io/renku/search/query/QueryGenerators.scala index ae936a0a..854c9626 100644 --- a/modules/search-query/src/test/scala/io/renku/search/query/QueryGenerators.scala +++ b/modules/search-query/src/test/scala/io/renku/search/query/QueryGenerators.scala @@ -19,6 +19,7 @@ package io.renku.search.query import cats.data.NonEmptyList +import cats.Order as CatsOrder import cats.syntax.all.* import io.renku.search.model.{CommonGenerators, ModelGenerators} import io.renku.search.model.projects.Visibility @@ -160,6 +161,7 @@ object QueryGenerators: val sortTerm: Gen[Order] = Gen.choose(1, 5).flatMap { len => + given CatsOrder[Order.OrderedBy] = CatsOrder.by(_.field) CommonGenerators.nelOfN(len, orderedBy).map(_.distinct).map(Order.apply) } diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryEncoders.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryEncoders.scala index acc9ef8e..480dc536 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryEncoders.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/LuceneQueryEncoders.scala @@ -148,4 +148,8 @@ trait LuceneQueryEncoders: given query[F[_]: Monad](using se: SolrTokenEncoder[F, List[Segment]] ): SolrTokenEncoder[F, Query] = - se.contramap(_.segments) + se.contramap[Query](_.segments) + .modify(q => + if (q.query.isEmpty) q.withQuery(SolrToken.allTypes) + else q + ) 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 8ceac60c..58a15cbe 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 @@ -27,6 +27,7 @@ final case class SolrQuery( query: SolrToken, sort: SolrSort ): + def withQuery(q: SolrToken): SolrQuery = copy(query = q) def ++(next: SolrQuery): SolrQuery = SolrQuery(query && next.query, sort ++ next.sort) diff --git a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrTokenEncoder.scala b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrTokenEncoder.scala index 86f8a2b1..c0cbf92f 100644 --- a/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrTokenEncoder.scala +++ b/modules/search-solr-client/src/main/scala/io/renku/search/solr/query/SolrTokenEncoder.scala @@ -22,11 +22,14 @@ import cats.{Applicative, Monad} import cats.syntax.all.* import scala.deriving.* import scala.collection.AbstractIterable +import cats.Functor trait SolrTokenEncoder[F[_], A]: def encode(ctx: Context[F], value: A): F[SolrQuery] final def contramap[B](f: B => A): SolrTokenEncoder[F, B] = SolrTokenEncoder.create((ctx, b) => encode(ctx, f(b))) + final def modify(f: SolrQuery => SolrQuery)(using Functor[F]): SolrTokenEncoder[F, A] = + SolrTokenEncoder.create((ctx, a) => encode(ctx, a).map(f)) object SolrTokenEncoder: def apply[F[_], A](using e: SolrTokenEncoder[F, A]): SolrTokenEncoder[F, A] = e diff --git a/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientGenerators.scala b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientGenerators.scala index f824edc7..0c137a80 100644 --- a/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientGenerators.scala +++ b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientGenerators.scala @@ -22,6 +22,7 @@ import cats.syntax.all.* import io.renku.search.model.* import io.renku.search.model.ModelGenerators.* import io.renku.search.solr.documents.* +import io.renku.search.GeneratorSyntax.* import org.scalacheck.Gen import org.scalacheck.cats.implicits.* @@ -48,11 +49,6 @@ object SearchSolrClientGenerators: def userDocumentGen: Gen[User] = (userIdGen, Gen.option(userFirstNameGen), Gen.option(userLastNameGen)) .flatMapN { case (id, f, l) => - val e = (f, l).flatMapN(userEmailGen(_, _).generateOption) + val e = (f, l).flatMapN(userEmailGen(_, _).generateSome) User(id, f, l, e) } - - extension [V](gen: Gen[V]) - def generateOne: V = gen.sample.getOrElse(generateOne) - def generateOption: Option[V] = Gen.option(gen).sample.getOrElse(generateOption) - def generateAs[D](f: V => D): D = f(generateOne) diff --git a/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientSpec.scala b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientSpec.scala index 4b81a855..ca29d2e2 100644 --- a/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientSpec.scala +++ b/modules/search-solr-client/src/test/scala/io/renku/search/solr/client/SearchSolrClientSpec.scala @@ -20,6 +20,7 @@ package io.renku.search.solr.client import cats.effect.IO import cats.syntax.all.* +import io.renku.search.GeneratorSyntax.* import io.renku.search.model.users import io.renku.search.query.Query import io.renku.search.solr.client.SearchSolrClientGenerators.* 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 b8bd7a6e..af9db3e9 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 @@ -32,16 +32,24 @@ import munit.CatsEffectSuite import java.time.Instant import java.time.ZoneId import io.renku.search.query.QueryGenerators -import munit.ScalaCheckSuite -import org.scalacheck.Prop +import munit.ScalaCheckEffectSuite +import org.scalacheck.effect.PropF +import io.renku.search.LoggingConfigure +import io.renku.search.solr.schema.EntityDocumentSchema.Fields +import io.renku.search.model.EntityType +import org.scalacheck.Test.Parameters class LuceneQueryInterpreterSpec extends CatsEffectSuite - with ScalaCheckSuite + with LoggingConfigure + with ScalaCheckEffectSuite with SolrSpec: override protected lazy val coreName: String = server.testCoreName2 + override protected def scalaCheckTestParameters: Parameters = + super.scalaCheckTestParameters.withMinSuccessfulTests(20) + given Decoder[Unit] = new Decoder { def read(r: Reader) = r.skipElement() @@ -55,7 +63,7 @@ class LuceneQueryInterpreterSpec val ctx = Context.fixed[Id](Instant.EPOCH, ZoneId.of("UTC")) val q = LuceneQueryInterpreter[Id].run(ctx, userQuery) - QueryData(QueryString(q.query.value, 0, 10)).withSort(q.sort) + QueryData(QueryString(q.query.value, 10, 0)).withSort(q.sort) def withSolr = withSolrClient().evalTap(c => SchemaMigrator[IO](c).migrate(Migrations.all).void) @@ -67,17 +75,32 @@ class LuceneQueryInterpreterSpec .traverse_(client.query[Unit]) } - property("generade valid solr queries"): - Prop.forAll(QueryGenerators.query) { q => + test("generade valid solr queries"): + PropF.forAllF(QueryGenerators.query) { q => withSolr .use { client => - client.query(query(q)) + client.query(query(q)).void } - .unsafeRunAndForget() } - test("generated queries are syntactically correct"): - val q = QueryGenerators.query.sample.get - withSolr.use { client => - client.query[Unit](query(q)) + test("sort only"): + val doc = Map( + Fields.id.name -> "one", + Fields.name.name -> "John", + Fields.entityType.name -> EntityType.User.name + ) + PropF.forAllF(QueryGenerators.sortTerm) { order => + val q = Query(Query.Segment.Sort(order)) + withSolr.use { client => + for { + _ <- client.insert(Seq(doc)) + r <- client.query[Map[String, String]]( + query(q).withFields(Fields.id, Fields.name, Fields.entityType).withLimit(2) + ) + _ = assert( + r.responseBody.docs.size >= 1, + s"Expected at least one result, but got: ${r.responseBody.docs}" + ) + } yield () + } } 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 41cb354d..252c9c0e 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 @@ -40,6 +40,8 @@ final case class QueryData( def withFields(field: FieldName*) = copy(fields = field) 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) object QueryData: diff --git a/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientGenerator.scala b/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientGenerator.scala index e69c34c9..7a7ccaca 100644 --- a/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientGenerator.scala +++ b/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientGenerator.scala @@ -25,8 +25,6 @@ import io.renku.search.model.CommonGenerators object SolrClientGenerator: - extension [V](gen: Gen[V]) def generateOne: V = gen.sample.getOrElse(generateOne) - private val fieldNameString: Gen[String] = Gen.choose(4, 12).flatMap(n => Gen.listOfN(n, Gen.alphaLowerChar)).map(_.mkString) diff --git a/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientSpec.scala b/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientSpec.scala index c2923395..2d4383ec 100644 --- a/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientSpec.scala +++ b/modules/solr-client/src/test/scala/io/renku/solr/client/SolrClientSpec.scala @@ -22,6 +22,7 @@ import cats.effect.IO import cats.syntax.all.* import io.bullet.borer.derivation.MapBasedCodecs import io.bullet.borer.{Decoder, Encoder} +import io.renku.search.LoggingConfigure import io.renku.solr.client.SolrClientSpec.Room import io.renku.solr.client.schema.* import io.renku.solr.client.util.{SolrSpec, SolrTruncate} @@ -35,6 +36,7 @@ import io.bullet.borer.derivation.key class SolrClientSpec extends CatsEffectSuite + with LoggingConfigure with ScalaCheckEffectSuite with SolrSpec with SolrTruncate: