Skip to content

Commit

Permalink
fix: Create correct solr query in case only sort terms are specfifi…
Browse files Browse the repository at this point in the history
…ed (#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
  • Loading branch information
eikek authored Mar 8, 2024
1 parent e8bb153 commit ad8c1cc
Show file tree
Hide file tree
Showing 20 changed files with 147 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 =>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package io.renku.search.query

import io.bullet.borer.{Decoder, Encoder}
import cats.kernel.Order

enum SortableField:
case Name
Expand All @@ -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(", ")

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*

Expand All @@ -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)
Original file line number Diff line number Diff line change
Expand Up @@ -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.*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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 ()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -35,6 +36,7 @@ import io.bullet.borer.derivation.key

class SolrClientSpec
extends CatsEffectSuite
with LoggingConfigure
with ScalaCheckEffectSuite
with SolrSpec
with SolrTruncate:
Expand Down

0 comments on commit ad8c1cc

Please sign in to comment.