diff --git a/search/src/main/scala/weco/api/search/models/DocumentFilter.scala b/search/src/main/scala/weco/api/search/models/DocumentFilter.scala index 059641ad63..c4597a379c 100644 --- a/search/src/main/scala/weco/api/search/models/DocumentFilter.scala +++ b/search/src/main/scala/weco/api/search/models/DocumentFilter.scala @@ -1,10 +1,10 @@ package weco.api.search.models import java.time.LocalDate +sealed trait DocumentFilter +sealed trait WorkFilter extends DocumentFilter -sealed trait WorkFilter - -sealed trait ImageFilter +sealed trait ImageFilter extends DocumentFilter case class ItemLocationTypeIdFilter(locationTypeIds: Seq[String]) extends WorkFilter diff --git a/search/src/main/scala/weco/api/search/models/SearchOptions.scala b/search/src/main/scala/weco/api/search/models/SearchOptions.scala index 08fb4c0aa2..9f4c7da1ab 100644 --- a/search/src/main/scala/weco/api/search/models/SearchOptions.scala +++ b/search/src/main/scala/weco/api/search/models/SearchOptions.scala @@ -7,9 +7,9 @@ import weco.api.search.models.request.{ WorkAggregationRequest } -sealed trait SearchOptions[DocumentFilter, AggregationRequest, MustQuery] { +sealed trait SearchOptions[DocFilter, AggregationRequest, MustQuery] { val searchQuery: Option[SearchQuery] - val filters: List[DocumentFilter] + val filters: List[DocFilter] val aggregations: List[AggregationRequest] val mustQueries: List[MustQuery] val sortBy: List[SortRequest] diff --git a/search/src/main/scala/weco/api/search/models/display/DisplayAggregation.scala b/search/src/main/scala/weco/api/search/models/display/DisplayAggregation.scala index 6879a1040c..9ef062d5b6 100644 --- a/search/src/main/scala/weco/api/search/models/display/DisplayAggregation.scala +++ b/search/src/main/scala/weco/api/search/models/display/DisplayAggregation.scala @@ -10,13 +10,18 @@ case class DisplayAggregation( ) case object DisplayAggregation { - def apply(agg: Aggregation[Json]): DisplayAggregation = + + def apply( + agg: Aggregation[Json], + retainEmpty: Json => Boolean + ): DisplayAggregation = DisplayAggregation( - buckets = agg.buckets.map { bucket => - DisplayAggregationBucket( - data = bucket.data, - count = bucket.count - ) + buckets = agg.buckets.collect { + case bucket if bucket.count > 0 || retainEmpty(bucket.data) => + DisplayAggregationBucket( + data = bucket.data, + count = bucket.count + ) } ) } diff --git a/search/src/main/scala/weco/api/search/models/display/DisplayImageAggregations.scala b/search/src/main/scala/weco/api/search/models/display/DisplayImageAggregations.scala index 5e813c4ae2..6c54d8bfe5 100644 --- a/search/src/main/scala/weco/api/search/models/display/DisplayImageAggregations.scala +++ b/search/src/main/scala/weco/api/search/models/display/DisplayImageAggregations.scala @@ -3,7 +3,7 @@ package weco.api.search.models.display import io.circe.Encoder import io.circe.generic.extras.JsonKey import io.circe.generic.extras.semiauto._ -import weco.api.search.models.ImageAggregations +import weco.api.search.models.{ImageAggregations, ImageFilter} import weco.json.JsonUtil._ case class DisplayImageAggregations( @@ -18,12 +18,38 @@ object DisplayImageAggregations { implicit def encoder: Encoder[DisplayImageAggregations] = deriveConfiguredEncoder - def apply(aggs: ImageAggregations): DisplayImageAggregations = + def apply( + aggs: ImageAggregations, + filters: Seq[ImageFilter] + ): DisplayImageAggregations = { + val bucketMatcher = FilterBucketMatcher(filters) + DisplayImageAggregations( - license = aggs.license.map(DisplayAggregation(_)), - `source.contributors.agent.label` = - aggs.sourceContributorAgents.map(DisplayAggregation(_)), - `source.genres.label` = aggs.sourceGenres.map(DisplayAggregation(_)), - `source.subjects.label` = aggs.sourceSubjects.map(DisplayAggregation(_)) + license = aggs.license.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(LicenseFilterAgg) + ) + ), + `source.contributors.agent.label` = aggs.sourceContributorAgents + .map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(ContributorsFilterAgg) + ) + ), + `source.genres.label` = aggs.sourceGenres.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(GenreFilterAgg) + ) + ), + `source.subjects.label` = aggs.sourceSubjects.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(SubjectLabelFilterAgg) + ) + ) ) + } } diff --git a/search/src/main/scala/weco/api/search/models/display/DisplayWorkAggregations.scala b/search/src/main/scala/weco/api/search/models/display/DisplayWorkAggregations.scala index 8ae1530b5a..0b60d0717e 100644 --- a/search/src/main/scala/weco/api/search/models/display/DisplayWorkAggregations.scala +++ b/search/src/main/scala/weco/api/search/models/display/DisplayWorkAggregations.scala @@ -3,8 +3,7 @@ package weco.api.search.models.display import io.circe.Encoder import io.circe.generic.extras.JsonKey import io.circe.generic.extras.semiauto._ -import weco.api.search.models.WorkAggregations -import weco.api.search.models.request.WorkAggregationRequest +import weco.api.search.models.{WorkAggregations, WorkFilter} import weco.json.JsonUtil._ case class DisplayWorkAggregations( @@ -20,23 +19,66 @@ case class DisplayWorkAggregations( ) object DisplayWorkAggregations { + implicit def encoder: Encoder[DisplayWorkAggregations] = deriveConfiguredEncoder def apply( aggs: WorkAggregations, - aggregationRequests: Seq[WorkAggregationRequest] - ): DisplayWorkAggregations = + filters: Seq[WorkFilter] + ): DisplayWorkAggregations = { + + val alwaysTrue = Function.const(true) _ + val bucketMatcher = FilterBucketMatcher(filters) + DisplayWorkAggregations( - workType = aggs.format.map(DisplayAggregation(_)), - `production.dates` = aggs.productionDates.map(DisplayAggregation(_)), - `genres.label` = aggs.genresLabel.map(DisplayAggregation(_)), - languages = aggs.languages.map(DisplayAggregation(_)), - `subjects.label` = aggs.subjectsLabel.map(DisplayAggregation(_)), - `contributors.agent.label` = - aggs.contributorsAgentsLabel.map(DisplayAggregation(_)), - `items.locations.license` = - aggs.itemsLocationsLicense.map(DisplayAggregation(_)), - availabilities = aggs.availabilities.map(DisplayAggregation(_)) + workType = aggs.format.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(FormatFilterAgg) + ) + ), + `production.dates` = aggs.productionDates + .map(DisplayAggregation(_, retainEmpty = alwaysTrue)), + `genres.label` = aggs.genresLabel.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(GenreFilterAgg) + ) + ), + languages = aggs.languages + .map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(LanguagesFilterAgg) + ) + ), + `subjects.label` = aggs.subjectsLabel.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(SubjectLabelFilterAgg) + ) + ), + `contributors.agent.label` = aggs.contributorsAgentsLabel + .map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(ContributorsFilterAgg) + ) + ), + `items.locations.license` = aggs.itemsLocationsLicense + .map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(LicenseFilterAgg) + ) + ), + availabilities = aggs.availabilities.map( + DisplayAggregation( + _, + retainEmpty = bucketMatcher.matchBucket(AvailabilitiesFilterAgg) + ) + ) ) + } } diff --git a/search/src/main/scala/weco/api/search/models/display/FilterBucketMatcher.scala b/search/src/main/scala/weco/api/search/models/display/FilterBucketMatcher.scala new file mode 100644 index 0000000000..52f6093a40 --- /dev/null +++ b/search/src/main/scala/weco/api/search/models/display/FilterBucketMatcher.scala @@ -0,0 +1,87 @@ +package weco.api.search.models.display + +import io.circe.Json +import weco.api.search.models.{ + AvailabilitiesFilter, + ContributorsFilter, + DocumentFilter, + FormatFilter, + GenreFilter, + LanguagesFilter, + LicenseFilter, + SubjectLabelFilter +} + +trait FilterAggregationMatcher { + def matchBucket(bucketData: Json): Boolean +} + +case class AggregationDataLabelInFilter(labels: Seq[String]) + extends FilterAggregationMatcher { + def matchBucket(bucketData: Json): Boolean = + bucketData.hcursor.get[String]("label") match { + case Right(value) => labels.contains(value) + case _ => false + } +} + +case class AggregationDataIdInFilter(labels: Seq[String]) + extends FilterAggregationMatcher { + def matchBucket(bucketData: Json): Boolean = + bucketData.hcursor.get[String]("id") match { + case Right(value) => labels.contains(value) + case _ => false + } +} + +case class NeverAggregationMatcher() extends FilterAggregationMatcher { + def matchBucket(bucketData: Json): Boolean = + false +} + +sealed trait FilterWithMatchingAggregation +case object FormatFilterAgg extends FilterWithMatchingAggregation +case object LanguagesFilterAgg extends FilterWithMatchingAggregation +case object GenreFilterAgg extends FilterWithMatchingAggregation +case object SubjectLabelFilterAgg extends FilterWithMatchingAggregation +case object ContributorsFilterAgg extends FilterWithMatchingAggregation +case object LicenseFilterAgg extends FilterWithMatchingAggregation +case object AvailabilitiesFilterAgg extends FilterWithMatchingAggregation + +class FilterBucketMatcher( + filters: Map[FilterWithMatchingAggregation, FilterAggregationMatcher] +) { + + def matchBucket( + aggregationType: FilterWithMatchingAggregation + )(bucketData: Json): Boolean = + filters + .getOrElse(aggregationType, NeverAggregationMatcher()) + .matchBucket(bucketData) +} + +object FilterBucketMatcher { + def apply(filters: Seq[DocumentFilter]) = + new FilterBucketMatcher( + filters collect { + case FormatFilter(ids) => + FormatFilterAgg -> AggregationDataIdInFilter(ids) + case LanguagesFilter(ids) => + LanguagesFilterAgg -> AggregationDataIdInFilter(ids) + case GenreFilter(labels) => + GenreFilterAgg -> AggregationDataLabelInFilter(labels) + case SubjectLabelFilter(labels) => + SubjectLabelFilterAgg -> + AggregationDataLabelInFilter(labels) + case ContributorsFilter(labels) => + ContributorsFilterAgg -> + AggregationDataLabelInFilter(labels) + case LicenseFilter(ids) => + LicenseFilterAgg -> AggregationDataIdInFilter(ids) + case AvailabilitiesFilter(availabilityIds) => + AvailabilitiesFilterAgg -> + AggregationDataIdInFilter(availabilityIds) + } toMap + ) + +} diff --git a/search/src/main/scala/weco/api/search/rest/DisplayResultList.scala b/search/src/main/scala/weco/api/search/rest/DisplayResultList.scala index bac11ab551..ace372a349 100644 --- a/search/src/main/scala/weco/api/search/rest/DisplayResultList.scala +++ b/search/src/main/scala/weco/api/search/rest/DisplayResultList.scala @@ -31,7 +31,7 @@ case class DisplayResultList[DisplayAggs]( object DisplayResultList extends CatalogueJsonUtil { def apply( resultList: ResultList[IndexedWork.Visible, WorkAggregations], - searchOptions: SearchOptions[_, WorkAggregationRequest, _], + searchOptions: SearchOptions[WorkFilter, WorkAggregationRequest, _], includes: WorksIncludes, requestUri: Uri ): DisplayResultList[DisplayWorkAggregations] = @@ -45,14 +45,14 @@ object DisplayResultList extends CatalogueJsonUtil { prevPage = prevPage, nextPage = nextPage, aggregations = resultList.aggregations.map( - DisplayWorkAggregations.apply(_, searchOptions.aggregations) + DisplayWorkAggregations.apply(_, searchOptions.filters) ) ) } def apply( resultList: ResultList[IndexedImage, ImageAggregations], - searchOptions: SearchOptions[_, _, _], + searchOptions: SearchOptions[ImageFilter, _, _], includes: MultipleImagesIncludes, requestUri: Uri ): DisplayResultList[DisplayImageAggregations] = @@ -65,8 +65,9 @@ object DisplayResultList extends CatalogueJsonUtil { results = resultList.results.map(_.display.withIncludes(includes)), prevPage = prevPage, nextPage = nextPage, - aggregations = - resultList.aggregations.map(DisplayImageAggregations.apply) + aggregations = resultList.aggregations.map( + DisplayImageAggregations.apply(_, searchOptions.filters) + ) ) } } diff --git a/search/src/test/scala/weco/api/search/works/WorksFilteredAggregationsTest.scala b/search/src/test/scala/weco/api/search/works/WorksFilteredAggregationsTest.scala index 2c2d48d333..db3b77e326 100644 --- a/search/src/test/scala/weco/api/search/works/WorksFilteredAggregationsTest.scala +++ b/search/src/test/scala/weco/api/search/works/WorksFilteredAggregationsTest.scala @@ -196,25 +196,7 @@ class WorksFilteredAggregationsTest extends AnyFunSpec with ApiWorksTestBase { "type" : "Format" }, "type" : "AggregationBucket" - }, - { - "count" : 0, - "data" : { - "id" : "d", - "label" : "Journals", - "type" : "Format" - }, - "type" : "AggregationBucket" - }, - { - "count" : 0, - "data" : { - "id" : "k", - "label" : "Pictures", - "type" : "Format" - }, - "type" : "AggregationBucket" - } + } ] } }, @@ -269,15 +251,6 @@ class WorksFilteredAggregationsTest extends AnyFunSpec with ApiWorksTestBase { "type" : "Format" }, "type" : "AggregationBucket" - }, - { - "count" : 0, - "data" : { - "id" : "k", - "label" : "Pictures", - "type" : "Format" - }, - "type" : "AggregationBucket" } ] } diff --git a/search/src/test/scala/weco/api/search/works/filtering/AggregatingTestCases.scala b/search/src/test/scala/weco/api/search/works/filtering/AggregatingTestCases.scala new file mode 100644 index 0000000000..e75115e2bf --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/AggregatingTestCases.scala @@ -0,0 +1,81 @@ +package weco.api.search.works.filtering + +import org.scalatest.funspec.AnyFunSpec +import weco.api.search.works.ApiWorksTestBase + +trait AggregatingTestCases extends AnyFunSpec with ApiWorksTestBase { + + this: SingleFieldFilterTest => + + val testWorks: Seq[String] + val aggregationName: String + + val allValuesParams: String + val allValuesResponse: String + + val redundantFilterParams: String + val redundantFilterBucket: String + + val unattestedValueParams: String + + describe(s"filtering and aggregating on $fieldName") { + it( + s"returns an aggregation over all values in $fieldName when filtering by $fieldName" + ) { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + assertJsonResponse( + routes, + path = s"$rootPath/works?$allValuesParams" + ) { + Status.OK -> allValuesResponse + } + } + } + + it( + "does not return an aggregation containing the filtered value if the value is not attested in the corpus" + ) { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + assertJsonResponse( + routes, + path = s"$rootPath/works?$unattestedValueParams" + ) { + Status.OK -> worksListResponseWithAggs( + Nil, + Map( + aggregationName -> Nil + ) + ) + } + } + + } + + it( + "returns an aggregation containing the filtered value even when the bucket count is zero" + ) { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + assertJsonResponse( + routes, + path = s"$rootPath/works?$redundantFilterParams" + ) { + Status.OK -> worksListResponseWithAggs( + Nil, + Map( + aggregationName -> Seq( + (0, redundantFilterBucket) + ) + ) + ) + } + } + } + } + +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByAvailabilityTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByAvailabilityTest.scala new file mode 100644 index 0000000000..99923835e4 --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByAvailabilityTest.scala @@ -0,0 +1,67 @@ +package weco.api.search.works.filtering + +class FilterAndAggregateByAvailabilityTest + extends SingleFieldFilterTest("availability") + with FilteringTestCases + with AggregatingTestCases { + val testWorks = Seq( + "works.examples.availabilities.open-only", + "works.examples.availabilities.closed-only", + "works.examples.availabilities.online-only", + "works.examples.availabilities.everywhere", + "works.examples.availabilities.nowhere" + ) + + val listingParams: String = "availabilities=open-shelves" + val listingResponse: String = worksListResponse( + ids = Seq( + "works.examples.availabilities.open-only", + "works.examples.availabilities.everywhere" + ) + ) + val multipleParams: String = "availabilities=open-shelves,closed-stores" + val multipleResponse: String = worksListResponse( + ids = Seq( + "works.examples.availabilities.open-only", + "works.examples.availabilities.everywhere", + "works.examples.availabilities.closed-only" + ) + ) + val searchingParams: String = "query=nowhere&availabilities=open-shelves" + val searchingResponse: String = worksListResponse(ids = Nil) + + val aggregationName: String = "availabilities" + val allValuesParams: String = + "availabilities=open-shelves&aggregations=availabilities" + val allValuesResponse: String = worksListResponseWithAggs( + Seq( + "works.examples.availabilities.open-only", + "works.examples.availabilities.everywhere" + ), + Map( + "availabilities" -> Seq( + (2, "closed-stores", "Closed stores"), + (2, "online", "Online"), + (2, "open-shelves", "Open shelves") + ).map { + case (count, identifier, label) => + (count, s""" + |{ + | "id" : "$identifier", + | "label" : "$label", + | "type" : "Availability" + | } + |""".stripMargin) + } + ) + ) + val redundantFilterParams: String = + "availabilities=online&genres.label=ThisIsNotAGenre&aggregations=availabilities" + val redundantFilterBucket: String = """{ + | "id" : "online", + | "label" : "Online", + | "type" : "Availability" + | }""".stripMargin + val unattestedValueParams: String = + "availabilities=on-the-moon&genres.label=ThisIsNotAGenre&aggregations=availabilities" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByContributorTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByContributorTest.scala new file mode 100644 index 0000000000..a68ab2aa1c --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByContributorTest.scala @@ -0,0 +1,89 @@ +package weco.api.search.works.filtering + +import org.scalatest.prop.TableFor3 + +class FilterAndAggregateByContributorTest + extends SingleFieldFilterTest("contributor") + with FreeTextFilteringTestCases + with AggregatingTestCases { + + val patriciaWork = "works.examples.contributor-filters-tests.0" + val karlMarxWork = "works.examples.contributor-filters-tests.1" + val jakePaulWork = "works.examples.contributor-filters-tests.2" + val darwinWork = "works.examples.contributor-filters-tests.3" + val patriciaDarwinWork = "works.examples.contributor-filters-tests.4" + val noContributorsWork = "works.examples.contributor-filters-tests.5" + + val testWorks = Seq( + patriciaWork, + karlMarxWork, + jakePaulWork, + darwinWork, + patriciaDarwinWork, + noContributorsWork + ) + + val filterName: String = "contributors.agent.label" + + protected val freeTextExamples: TableFor3[String, Seq[String], String] = + Table( + ("query", "expectedIds", "clue"), + ("Karl Marx", Seq(karlMarxWork), "single match"), + ( + """"Bath, Patricia"""", + Seq(patriciaWork, patriciaDarwinWork), + "multi match" + ), + ( + "Karl Marx,Jake Paul", + Seq(karlMarxWork, jakePaulWork), + "comma separated" + ), + ( + """"Bath, Patricia",Karl Marx""", + Seq(patriciaWork, patriciaDarwinWork, karlMarxWork), + "commas in quotes" + ), + ( + """"Bath, Patricia",Karl Marx,"Darwin \"Jones\", Charles"""", + Seq(patriciaWork, karlMarxWork, darwinWork, patriciaDarwinWork), + "quotes in quotes" + ) + ) + + val allValuesParams: String = + "contributors.agent.label=Karl%20Marx&aggregations=contributors.agent.label" + val allValuesResponse: String = worksListResponseWithAggs( + Seq(karlMarxWork), + Map( + "contributors.agent.label" -> Seq( + (2, "Bath, Patricia"), + (2, "Darwin \\\"Jones\\\", Charles"), + (1, "Jake Paul"), + (1, "Karl Marx") + ).map { + case (count, label) => + (count, s""" + |{ + | "label" : "$label", + | "type" : "Person" + | } + |""".stripMargin) + } + ) + ) + val redundantFilterParams: String = + s"contributors.agent.label=Karl%20Marx&genres.label=NotAGenre&aggregations=contributors.agent.label" + val redundantFilterBucket: String = + """ + |{ + | "label" : "Karl Marx", + | "type" : "Person" + | } + |""".stripMargin + + val aggregationName: String = "contributors.agent.label" + val unattestedValueParams: String = + "languages=sjn&contributors.agent.label=Joseph%20Pujol&aggregations=contributors.agent.label" + val bogusValueResponse: String = "" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByFormatTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByFormatTest.scala new file mode 100644 index 0000000000..656451b857 --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByFormatTest.scala @@ -0,0 +1,52 @@ +package weco.api.search.works.filtering + +class FilterAndAggregateByFormatTest + extends SingleFieldFilterTest("workType") + with FilteringTestCases + with AggregatingTestCases { + + val testWorks = worksFormatBooks ++ worksFormatJournals ++ worksFormatAudio ++ worksFormatPictures + + val listingParams: String = "workType=k" + val listingResponse: String = worksListResponse( + ids = Seq("works.formats.9.Pictures") + ) + val multipleParams: String = "workType=k,d" + val multipleResponse: String = worksListResponse( + ids = worksFormatJournals ++ worksFormatPictures + ) + val searchingParams: String = "query=Book&workType=k" + val searchingResponse: String = worksListResponse(ids = Nil) + + val aggregationName: String = "workType" + val allValuesParams: String = "workType=k&aggregations=workType" + val allValuesResponse: String = worksListResponseWithAggs( + worksFormatPictures, + Map( + "workType" -> Seq( + (4, "a", "Books"), + (3, "d", "Journals"), + (2, "i", "Audio"), + (1, "k", "Pictures") + ).map { + case (count, identifier, label) => + (count, s""" + |{ + | "id" : "$identifier", + | "label" : "$label", + | "type" : "Format" + | } + |""".stripMargin) + } + ) + ) + val redundantFilterParams: String = + "workType=k&genres.label=ThisIsNotAGenre&aggregations=workType" + val redundantFilterBucket: String = """{ + | "id" : "k", + | "label" : "Pictures", + | "type" : "Format" + | }""".stripMargin + val unattestedValueParams: String = + "workType=z&genres.label=ThisIsNotAGenre&aggregations=workType" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByGenreTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByGenreTest.scala new file mode 100644 index 0000000000..b95400aa89 --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByGenreTest.scala @@ -0,0 +1,93 @@ +package weco.api.search.works.filtering + +import io.circe.syntax.EncoderOps +import org.scalatest.prop.TableFor3 + +class FilterAndAggregateByGenreTest + extends SingleFieldFilterTest("genre") + with FreeTextFilteringTestCases + with AggregatingTestCases { + val annualReportsWork = s"works.examples.genre-filters-tests.0" + val pamphletsWork = "works.examples.genre-filters-tests.1" + val psychologyWork = "works.examples.genre-filters-tests.2" + val darwinWork = "works.examples.genre-filters-tests.3" + val mostThingsWork = "works.examples.genre-filters-tests.4" + val nothingWork = "works.examples.genre-filters-tests.5" + + val testWorks: Seq[String] = + Seq( + annualReportsWork, + pamphletsWork, + psychologyWork, + darwinWork, + mostThingsWork, + nothingWork + ) + + val freeTextExamples: TableFor3[String, Seq[String], String] = Table( + ("query", "expectedIds", "clue"), + ("Annual reports.", Seq(annualReportsWork), "single match single genre"), + ( + "Pamphlets.", + Seq(pamphletsWork, mostThingsWork), + "multi match single genre" + ), + ( + "Annual reports.,Pamphlets.", + Seq(annualReportsWork, pamphletsWork, mostThingsWork), + "comma separated" + ), + ( + """Annual reports.,"Psychology, Pathological"""", + Seq(annualReportsWork, psychologyWork, mostThingsWork), + "commas in quotes" + ), + ( + """"Darwin \"Jones\", Charles","Psychology, Pathological",Pamphlets.""", + Seq(darwinWork, psychologyWork, mostThingsWork, pamphletsWork), + "escaped quotes in quotes" + ) + ) + + val filterName: String = "genres.label" + + val allValuesParams: String = + "genres.label=Annual%20reports.&aggregations=genres.label" + val allValuesResponse: String = worksListResponseWithAggs( + Seq(annualReportsWork), + Map( + "genres.label" -> Seq( + (2, "Darwin \"Jones\", Charles"), + (2, "Pamphlets."), + (2, "Psychology, Pathological"), + (1, "Annual reports.") + ).map { + case (count, label) => + (count, s""" + |{ + | "concepts" : [ + | ], + | "label" : ${label.asJson}, + | "type" : "Genre" + | } + |""".stripMargin) + } + ) + ) + + val redundantFilterParams: String = + s"genres.label=Pamphlets.&languages=sjn&aggregations=genres.label" + val redundantFilterBucket: String = + """ + |{ + | "concepts" : [ + | ], + | "label" : "Pamphlets.", + | "type" : "Genre" + | } + |""".stripMargin + val aggregationName: String = "genres.label" + val unattestedValueParams: String = + "languages=sjn&genres.label=NotAGenre&aggregations=genres.label" + val bogusValueResponse: String = "" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByLanguageTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByLanguageTest.scala new file mode 100644 index 0000000000..459d622e63 --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateByLanguageTest.scala @@ -0,0 +1,78 @@ +package weco.api.search.works.filtering + +class FilterAndAggregateByLanguageTest + extends SingleFieldFilterTest("language") + with FilteringTestCases + with AggregatingTestCases { + val testWorks: Seq[String] = Seq( + "works.languages.0.eng", + "works.languages.1.eng", + "works.languages.2.eng", + "works.languages.3.eng+swe", + "works.languages.4.eng+swe+tur", + "works.languages.5.swe", + "works.languages.6.tur" + ) + val listingParams: String = "languages=eng" + val listingResponse: String = worksListResponse( + ids = Seq( + "works.languages.0.eng", + "works.languages.1.eng", + "works.languages.2.eng", + "works.languages.3.eng+swe", + "works.languages.4.eng+swe+tur" + ) + ) + + val multipleParams: String = "languages=swe,tur" + val multipleResponse: String = worksListResponse( + ids = Seq( + "works.languages.3.eng+swe", + "works.languages.4.eng+swe+tur", + "works.languages.5.swe", + "works.languages.6.tur" + ) + ) + + val searchingParams: String = "query=Swedish&languages=tur" + val searchingResponse: String = worksListResponse( + ids = Seq( + "works.languages.4.eng+swe+tur" + ) + ) + + val allValuesParams: String = "languages=tur&aggregations=languages" + val allValuesResponse: String = worksListResponseWithAggs( + Seq("works.languages.4.eng+swe+tur", "works.languages.6.tur"), + Map( + "languages" -> Seq( + (5, "eng", "English"), + (3, "swe", "Swedish"), + (2, "tur", "Turkish") + ).map { + case (count, identifier, label) => + (count, s""" + |{ + | "id" : "$identifier", + | "label" : "$label", + | "type" : "Language" + | } + |""".stripMargin) + } + ) + ) + val redundantFilterParams: String = + "languages=eng&genres.label=NotAGenre&aggregations=languages" + val redundantFilterBucket: String = + """ + |{ + | "id" : "eng", + | "label" : "English", + | "type" : "Language" + | } + |""".stripMargin + val aggregationName: String = "languages" + val unattestedValueParams: String = + "languages=sjn&genres.label=NotAGenre&aggregations=languages" + val bogusValueResponse: String = "" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateBySubjectTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateBySubjectTest.scala new file mode 100644 index 0000000000..a21ff308cc --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterAndAggregateBySubjectTest.scala @@ -0,0 +1,99 @@ +package weco.api.search.works.filtering + +import org.scalatest.prop.TableFor3 + +import java.net.URLEncoder + +class FilterAndAggregateBySubjectTest + extends SingleFieldFilterTest("subject") + with FreeTextFilteringTestCases + with AggregatingTestCases { + + val sanitationWork = "works.examples.subject-filters-tests.0" + val londonWork = "works.examples.subject-filters-tests.1" + val psychologyWork = "works.examples.subject-filters-tests.2" + val darwinWork = "works.examples.subject-filters-tests.3" + val mostThingsWork = "works.examples.subject-filters-tests.4" + val nothingWork = "works.examples.subject-filters-tests.5" + + val testWorks: Seq[String] = + Seq( + sanitationWork, + londonWork, + psychologyWork, + darwinWork, + mostThingsWork, + nothingWork + ) + + val filterName: String = "subjects.label" + + protected val freeTextExamples: TableFor3[String, Seq[String], String] = + Table( + ("query", "expectedIds", "clue"), + ( + "Sanitation.", + Seq(sanitationWork), + "single match single subject" + ), + ( + "London (England)", + Seq(londonWork, mostThingsWork), + "multi match single subject" + ), + ( + "Sanitation.,London (England)", + Seq(sanitationWork, londonWork, mostThingsWork), + "comma separated" + ), + ( + """Sanitation.,"Psychology, Pathological"""", + Seq(sanitationWork, psychologyWork, mostThingsWork), + "commas in quotes" + ), + ( + """"Darwin \"Jones\", Charles","Psychology, Pathological",London (England)""", + Seq(darwinWork, psychologyWork, londonWork, mostThingsWork), + "escaped quotes in quotes" + ) + ) + + val allValuesParams: String = + "subjects.label=Sanitation.&aggregations=subjects.label" + val allValuesResponse: String = worksListResponseWithAggs( + Seq(sanitationWork), + Map( + "subjects.label" -> Seq( + (2, "Darwin \\\"Jones\\\", Charles"), + (2, "London (England)"), + (2, "Psychology, Pathological"), + (1, "Sanitation.") + ).map { + case (count, label) => + (count, s""" + |{ + | "concepts" : [ + | ], + | "label" : "$label", + | "type" : "Subject" + | } + |""".stripMargin) + } + ) + ) + val redundantFilterParams: String = + s"subjects.label=${URLEncoder.encode(""""Psychology, Pathological"""", "UTF-8")}&genres.label=NotAGenre&aggregations=subjects.label" + val redundantFilterBucket: String = + """ + |{ + | "concepts" : [ + | ], + | "label" : "Psychology, Pathological", + | "type" : "Subject" + | } + |""".stripMargin + val aggregationName: String = "subjects.label" + val unattestedValueParams: String = + "languages=sjn&subjects.label=NotASubject&aggregations=subjects.label" + val bogusValueResponse: String = "" +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilterByTypeTest.scala b/search/src/test/scala/weco/api/search/works/filtering/FilterByTypeTest.scala new file mode 100644 index 0000000000..c52dfa112d --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilterByTypeTest.scala @@ -0,0 +1,29 @@ +package weco.api.search.works.filtering + +class FilterByTypeTest + extends SingleFieldFilterTest("type") + with FilteringTestCases { + val testWorks: Seq[String] = Seq( + "works.examples.different-work-types.Collection", + "works.examples.different-work-types.Series", + "works.examples.different-work-types.Section" + ) + val listingParams: String = "type=Collection" + val listingResponse: String = worksListResponse( + ids = Seq("works.examples.different-work-types.Collection") + ) + val multipleParams: String = "type=Collection,Series" + val multipleResponse: String = worksListResponse( + ids = Seq( + "works.examples.different-work-types.Collection", + "works.examples.different-work-types.Series" + ) + ) + val searchingParams: String = "query=rats&type=Series,Section" + val searchingResponse: String = worksListResponse( + ids = Seq( + "works.examples.different-work-types.Series", + "works.examples.different-work-types.Section" + ) + ) +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FilteringTestCases.scala b/search/src/test/scala/weco/api/search/works/filtering/FilteringTestCases.scala new file mode 100644 index 0000000000..c469cc0d2d --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FilteringTestCases.scala @@ -0,0 +1,64 @@ +package weco.api.search.works.filtering + +import org.scalatest.funspec.AnyFunSpec +import weco.api.search.works.ApiWorksTestBase + +/** + * Test cases that cover filtering by simple values such as ids. + */ +trait FilteringTestCases extends AnyFunSpec with ApiWorksTestBase { + this: SingleFieldFilterTest => + + val testWorks: Seq[String] + + val listingParams: String + val listingResponse: String + + val multipleParams: String + val multipleResponse: String + + val searchingParams: String + val searchingResponse: String + + describe(s"filtering $fieldName by simple values") { + it("filters by one value when listing works") { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + + assertJsonResponse(routes, path = s"$rootPath/works?$listingParams") { + Status.OK -> listingResponse + } + } + } + + it("filters by multiple values when listing works") { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + + assertJsonResponse( + routes, + path = s"$rootPath/works?$multipleParams" + ) { + Status.OK -> multipleResponse + } + } + } + + it("when searching for works") { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + + assertJsonResponse( + routes, + path = s"$rootPath/works?$searchingParams" + ) { + Status.OK -> searchingResponse + } + } + } + } + +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/FreeTextFilteringTestCases.scala b/search/src/test/scala/weco/api/search/works/filtering/FreeTextFilteringTestCases.scala new file mode 100644 index 0000000000..31346063db --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/FreeTextFilteringTestCases.scala @@ -0,0 +1,49 @@ +package weco.api.search.works.filtering + +import org.scalatest.funspec.AnyFunSpec +import org.scalatest.prop.{TableDrivenPropertyChecks, TableFor3} +import weco.api.search.works.ApiWorksTestBase + +import java.net.URLEncoder + +/** + * Test cases that cover filtering by complex values such as labels. + * Filter terms on labels are likely to have tricky characters such as commas and quotes that + * a naive processor might misinterpret as delimiters, rather than pass on to the database as + * part of the filter. + * + */ +trait FreeTextFilteringTestCases + extends AnyFunSpec + with ApiWorksTestBase + with TableDrivenPropertyChecks { + + this: SingleFieldFilterTest => + + val testWorks: Seq[String] + val filterName: String + + protected val freeTextExamples: TableFor3[String, Seq[String], String] + + describe(s"filtering $fieldName by various values") { + it(s"filters using a comma separated list") { + withWorksApi { + case (worksIndex, routes) => + indexTestDocuments(worksIndex, testWorks: _*) + + forAll(freeTextExamples) { + (query: String, expectedIds: Seq[String], clue: String) => + withClue(clue) { + assertJsonResponse( + routes, + path = s"$rootPath/works?$filterName=${URLEncoder + .encode(query, "UTF-8")}" + ) { + Status.OK -> worksListResponse(expectedIds) + } + } + } + } + } + } +} diff --git a/search/src/test/scala/weco/api/search/works/filtering/SingleFieldFilterTest.scala b/search/src/test/scala/weco/api/search/works/filtering/SingleFieldFilterTest.scala new file mode 100644 index 0000000000..13c35e6441 --- /dev/null +++ b/search/src/test/scala/weco/api/search/works/filtering/SingleFieldFilterTest.scala @@ -0,0 +1,5 @@ +package weco.api.search.works.filtering + +import org.scalatest.funspec.AnyFunSpec + +class SingleFieldFilterTest(val fieldName: String) extends AnyFunSpec