Skip to content

Commit

Permalink
fix: do not return incomplete search results
Browse files Browse the repository at this point in the history
  • Loading branch information
olevski committed Oct 16, 2024
1 parent 7764da8 commit 9d1a287
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 28 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ private class SearchApiImpl[F[_]: Async](solrClient: SearchSolrClient[F])
override def query(
auth: AuthContext
)(query: QueryInput): F[Either[String, SearchResult]] =
logger.info(show"Running query '$query' as '$auth'") >>
val allResults = logger.info(show"Running query '$query' as '$auth'") >>
solrClient
.queryEntity(
auth.searchRole,
Expand All @@ -49,9 +49,32 @@ private class SearchApiImpl[F[_]: Async](solrClient: SearchSolrClient[F])
query.page.offset
)
.map(toApiResult(query.page))
.map(_.asRight[String])
.handleErrorWith(errorResponse(query.query.render))
.widen
val completeResults = allResults.map(searchRes =>
searchRes.copy(items =
searchRes.items.mapFilter(
_ match
case pr: SearchEntity.Project => pr.maybeCompleteProject
case gr: SearchEntity.Group => Some(gr)
case us: SearchEntity.User => us.maybeCompleteUser
case prns: SearchEntity.CompleteProject => Some(prns)
case usns: SearchEntity.CompleteUser => Some(usns)
)
)
)
val output = for
all <- allResults
complete <- completeResults
yield
val incomplete = all.items.length - complete.items.length
if (incomplete > 0)
logger.error(
s"Found ${incomplete}/${all.items.length} incomplete search results for query '${query.query.render}'"
)
else ()
complete.asRight[String]
output
.handleErrorWith(errorResponse(query.query.render))
.widen

private def errorResponse(
phrase: String
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,12 @@ object SearchEntity:
given Decoder[SearchEntity] = MapBasedCodecs.deriveDecoder[SearchEntity]
given Encoder[SearchEntity] = EncoderSupport.derive[SearchEntity]

sealed trait ProjectNamespace:
def namespace: UserOrGroup

sealed trait UserNamespace:
def namespace: Namespace

final case class Project(
id: Id,
name: Name,
Expand All @@ -54,28 +60,101 @@ object SearchEntity:
creationDate: CreationDate,
keywords: List[Keyword] = Nil,
score: Option[Double] = None
) extends SearchEntity
) extends SearchEntity {
def maybeCompleteProject: Option[CompleteProject] =
(namespace, createdBy.flatMap(_.maybeCompleteUser)) match
case (Some(namespace), Some(createdBy)) =>
Some(
CompleteProject(
id = id,
name = name,
slug = slug,
namespace = namespace,
repositories = repositories,
visibility = visibility,
description = description,
createdBy = createdBy,
creationDate = creationDate,
keywords = keywords,
score = score
)
)
case (None, _) => None
case (_, None) => None
}

object Project:
given Encoder[Project] =
EncoderSupport.deriveWithDiscriminator[Project](discriminatorField)
given Decoder[Project] = MapBasedCodecs.deriveDecoder
end Project

final case class CompleteProject(
id: Id,
name: Name,
namespace: UserOrGroup,
slug: Slug,
repositories: Seq[Repository],
visibility: Visibility,
description: Option[Description] = None,
createdBy: CompleteUser,
creationDate: CreationDate,
keywords: List[Keyword] = Nil,
score: Option[Double] = None
) extends SearchEntity
with ProjectNamespace

object CompleteProject:
given Encoder[CompleteProject] =
EncoderSupport.deriveWithDiscriminator[CompleteProject](discriminatorField)
given Decoder[CompleteProject] = MapBasedCodecs.deriveDecoder
end CompleteProject

final case class User(
id: Id,
namespace: Option[Namespace] = None,
firstName: Option[FirstName] = None,
lastName: Option[LastName] = None,
score: Option[Double] = None
) extends SearchEntity
with UserOrGroup
with UserOrGroup {
def maybeCompleteUser: Option[CompleteUser] =
namespace match {
case Some(ns: Namespace) =>
Some(
CompleteUser(
id = id,
namespace = ns,
firstName = firstName,
lastName = lastName,
score = score
)
)
case None => None
}
}

object User:
given Encoder[User] = EncoderSupport.deriveWithDiscriminator(discriminatorField)
given Decoder[User] = MapBasedCodecs.deriveDecoder
end User

final case class CompleteUser(
id: Id,
namespace: Namespace,
firstName: Option[FirstName] = None,
lastName: Option[LastName] = None,
score: Option[Double] = None
) extends SearchEntity
with UserOrGroup
with UserNamespace

object CompleteUser:
given Encoder[CompleteUser] =
EncoderSupport.deriveWithDiscriminator(discriminatorField)
given Decoder[CompleteUser] = MapBasedCodecs.deriveDecoder
end CompleteUser

final case class Group(
id: Id,
name: Name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,16 @@ trait ApiSchema extends ApiSchema.Primitives:
.derived[User]
.jsonExample(ApiSchema.exampleUser.widen)

given Schema[CompleteUser] = Schema
.derived[CompleteUser]
.jsonExample(ApiSchema.exampleCompleteUser.widen)

given Schema[Group] = Schema
.derived[Group]
.jsonExample(ApiSchema.exampleGroup.widen)

given (using
userSchema: Schema[User],
userSchema: Schema[CompleteUser],
groupSchema: Schema[Group]
): Schema[UserOrGroup] =
Schema
Expand All @@ -56,29 +60,29 @@ trait ApiSchema extends ApiSchema.Primitives:
)
.jsonExample(ApiSchema.exampleGroup: UserOrGroup)

given (using userSchema: Schema[User]): Schema[Project] = Schema
.derived[Project]
given (using userSchema: Schema[CompleteUser]): Schema[CompleteProject] = Schema
.derived[CompleteProject]
.modify(_.createdBy) { schemaOptUser =>
// this is necessary to include the `type` property into the schema of the createdBy property
// It is not added automagically, because we use the concrete type `User` and not `SearchEntity`
// (the sealed trait).
// Using `SearchEntity` results in a deadlock when evaluating the magnolia macros from tapir. I
// tried to make all components lazy, but didn't manage to solve it
val userType = userSchema.schemaType.asInstanceOf[SProduct[User]]
val df = SProductField[User, String](
val userType = userSchema.schemaType.asInstanceOf[SProduct[CompleteUser]]
val df = SProductField[CompleteUser, String](
FieldName("type"),
Schema.string,
_ => Some("User")
)
val nextUserSchema: Schema[User] =
val nextUserSchema: Schema[CompleteUser] =
userSchema.copy(schemaType = userType.copy(fields = df :: userType.fields))
schemaOptUser.copy(schemaType = SOption(nextUserSchema)(identity))
schemaOptUser.copy(schemaType = nextUserSchema.schemaType)
}
.jsonExample(ApiSchema.exampleProject.widen)

given (using
projectSchema: Schema[Project],
userSchema: Schema[User],
projectSchema: Schema[CompleteProject],
userSchema: Schema[CompleteUser],
groupSchema: Schema[Group],
ug: Schema[UserOrGroup]
): Schema[SearchEntity] =
Expand Down Expand Up @@ -155,6 +159,14 @@ object ApiSchema:
Some(2.1)
)

val exampleCompleteUser: SearchEntity.CompleteUser = CompleteUser(
Id("1CAF4C73F50D4514A041C9EDDB025A36"),
Namespace("renku/renku"),
Some(FirstName("Albert")),
Some(LastName("Einstein")),
Some(2.1)
)

val exampleGroup: SearchEntity.Group = Group(
Id("2CAF4C73F50D4514A041C9EDDB025A36"),
Name("SDSC"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import io.renku.search.model.*
import io.renku.search.query.Query
import io.renku.search.solr.client.SearchSolrSuite
import io.renku.search.solr.client.SolrDocumentGenerators.*
import io.renku.search.solr.documents.{EntityDocument, User as SolrUser}
import io.renku.search.solr.documents.{
EntityDocument,
Group as SolrGroup,
User as SolrUser
}
import io.renku.solr.client.DocVersion
import io.renku.solr.client.ResponseBody
import munit.CatsEffectSuite
Expand All @@ -41,24 +45,34 @@ class SearchApiSpec extends CatsEffectSuite with SearchSolrSuite:
private given Scribe[IO] = scribe.cats[IO]

test("do a lookup in Solr to find entities matching the given phrase"):
val group = SolrGroup.of(
ModelGenerators.idGen.generateOne,
Name("group-1"),
Namespace("group-1")
)
val user1 = SolrUser.of(ModelGenerators.idGen.generateOne, Some(Namespace("user-1")))
val user2 = SolrUser.of(ModelGenerators.idGen.generateOne, Some(Namespace("user-2")))
val project1 = projectDocumentGen(
"matching",
"matching description",
Gen.const(None),
Gen.const(None),
Gen.const(Some(user1)),
Gen.const(Some(user1)),
Gen.const(Visibility.Public)
).generateOne
val project2 = projectDocumentGen(
"disparate",
"disparate description",
Gen.const(None),
Gen.const(None),
Gen.const(Some(user2)),
Gen.const(Some(group)),
Gen.const(Visibility.Public)
).generateOne
for {
client <- IO(searchSolrClient())
searchApi = new SearchApiImpl[IO](client)
_ <- client.upsert((project1 :: project2 :: Nil).map(_.widen))
_ <- client.upsert(
(project1.stripDetails :: project2.stripDetails :: user1 :: user2 :: group :: Nil)
.map(_.widen)
)
results <- searchApi
.query(AuthContext.anonymous)(mkQuery("matching"))
.map(_.fold(err => fail(s"Calling Search API failed with $err"), identity))
Expand All @@ -72,18 +86,23 @@ class SearchApiSpec extends CatsEffectSuite with SearchSolrSuite:

test("return Project and User entities"):
val userId = ModelGenerators.idGen.generateOne
val user = SolrUser(userId, DocVersion.NotExists, FirstName("exclusive").some)
val user = SolrUser(
userId,
DocVersion.NotExists,
FirstName("exclusive").some,
namespace = Namespace("user").some
)
val project = projectDocumentGen(
"exclusive",
"exclusive description",
Gen.const(None),
Gen.const(None),
Gen.const(Some(user)),
Gen.const(Some(user)),
Gen.const(Visibility.Public)
).generateOne.copy(createdBy = userId)
for {
client <- IO(searchSolrClient())
searchApi = new SearchApiImpl[IO](client)
_ <- client.upsert[EntityDocument](project :: user :: Nil)
_ <- client.upsert[EntityDocument](project.stripDetails :: user :: Nil)
results <- searchApi
.query(AuthContext.anonymous)(mkQuery("exclusive"))
.map(_.fold(err => fail(s"Calling Search API failed with $err"), identity))
Expand All @@ -99,9 +118,11 @@ class SearchApiSpec extends CatsEffectSuite with SearchSolrSuite:
)

private def scoreToNone(e: SearchEntity): SearchEntity = e match
case e: SearchEntity.Project => e.copy(score = None)
case e: SearchEntity.User => e.copy(score = None)
case e: SearchEntity.Group => e.copy(score = None)
case e: SearchEntity.Project => e.copy(score = None)
case e: SearchEntity.User => e.copy(score = None)
case e: SearchEntity.Group => e.copy(score = None)
case e: SearchEntity.CompleteUser => e.copy(score = None)
case e: SearchEntity.CompleteProject => e.copy(score = None)

private def mkQuery(phrase: String): QueryInput =
QueryInput.pageOne(Query.parse(s"Fields $phrase").fold(sys.error, identity))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ final case class Project(
def modifyGroupMembers(f: EntityMembers => EntityMembers): Project =
setGroupMembers(f(toGroupMembers))

// Needed because a document that has this fields with Some(...) value will not be able
// to be inserted in Solr.
def stripDetails: Project =
copy(creatorDetails = None, namespaceDetails = None)

object Project:
given Encoder[Project] =
EncoderSupport.deriveWith(
Expand Down

0 comments on commit 9d1a287

Please sign in to comment.