Do not return unwanted empty buckets (#674)

* remove AnyFunSpec from base class * remove AnyFunSpec from base class * commit missed file * Do not return unwanted empty buckets * Do not return unwanted empty buckets * extend no-empties rule to images * avoid naming collision * better naming, as per review
wellcomecollection · Jul 20, 2023 · d4f62ab · d4f62ab
1 parent 4fbbbb0
commit d4f62ab
Show file tree

Hide file tree

Showing 19 changed files with 905 additions and 65 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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"
                     }
                   ]
                 }