From b5fac920d9aa0d3e3fa899867785cbda0fc9f533 Mon Sep 17 00:00:00 2001 From: Raphaelle Cantin Date: Fri, 19 May 2023 11:04:50 +0100 Subject: [PATCH 1/2] Remove references to _queryType --- .../weco/api/search/models/SearchQuery.scala | 12 +-------- .../api/search/models/SearchQueryType.scala | 26 ------------------- .../weco/api/search/rest/WorksParams.scala | 12 +-------- .../search/services/WorksRequestBuilder.scala | 6 +++-- .../search/elasticsearch/WorksQueryTest.scala | 10 +++---- .../api/search/works/WorksErrorsTest.scala | 19 +------------- 6 files changed, 11 insertions(+), 74 deletions(-) delete mode 100644 search/src/main/scala/weco/api/search/models/SearchQueryType.scala diff --git a/search/src/main/scala/weco/api/search/models/SearchQuery.scala b/search/src/main/scala/weco/api/search/models/SearchQuery.scala index 14e58fedc5..c28651d892 100644 --- a/search/src/main/scala/weco/api/search/models/SearchQuery.scala +++ b/search/src/main/scala/weco/api/search/models/SearchQuery.scala @@ -1,13 +1,3 @@ package weco.api.search.models -case class SearchQuery(query: String, queryType: SearchQueryType) -object SearchQuery { - def apply( - query: String, - maybeQueryType: Option[SearchQueryType] - ): SearchQuery = - SearchQuery(query, maybeQueryType.getOrElse(SearchQueryType.default)) - - def apply(query: String): SearchQuery = - SearchQuery(query, None) -} +case class SearchQuery(query: String) \ No newline at end of file diff --git a/search/src/main/scala/weco/api/search/models/SearchQueryType.scala b/search/src/main/scala/weco/api/search/models/SearchQueryType.scala deleted file mode 100644 index f8eb51905f..0000000000 --- a/search/src/main/scala/weco/api/search/models/SearchQueryType.scala +++ /dev/null @@ -1,26 +0,0 @@ -package weco.api.search.models - -import com.sksamuel.elastic4s.requests.searches.queries.compound.BoolQuery -import weco.api.search.elasticsearch.WorksMultiMatcher - -sealed trait SearchQueryType { - import SearchQueryType._ - - // This is because the `this` is the actual simpleton so gets `$` - // appended to the `simpleName`. Not great, but contained, so meh. - // As this changes quite often it felt worth doing. - final val name = this.getClass.getSimpleName.split("\\$").last - - def toEsQuery(q: String): BoolQuery = this match { - case MultiMatcher => WorksMultiMatcher(q) - } -} -object SearchQueryType { - val default = MultiMatcher - // These are the queries that we are surfacing to the frontend to be able to select which one they want to run. - // You'll need to change the `allowableValues` in `weco.api.search.swagger.SwaggerDocs` - // when changing these as they can't be read there as they need to be constant. - val allowed = List(MultiMatcher) - - final case object MultiMatcher extends SearchQueryType -} diff --git a/search/src/main/scala/weco/api/search/rest/WorksParams.scala b/search/src/main/scala/weco/api/search/rest/WorksParams.scala index eca9951f9f..d562a3193e 100644 --- a/search/src/main/scala/weco/api/search/rest/WorksParams.scala +++ b/search/src/main/scala/weco/api/search/rest/WorksParams.scala @@ -92,7 +92,6 @@ case class MultipleWorksParams( include: Option[WorksIncludes], aggregations: Option[List[WorkAggregationRequest]], query: Option[String], - _queryType: Option[SearchQueryType] ) extends QueryParams with Paginated { @@ -102,7 +101,7 @@ case class MultipleWorksParams( def searchOptions(apiConfig: ApiConfig): WorkSearchOptions = WorkSearchOptions( searchQuery = query map { query => - SearchQuery(query, _queryType) + SearchQuery(query) }, filters = filters, pageSize = pageSize.getOrElse(apiConfig.defaultPageSize), @@ -163,7 +162,6 @@ object MultipleWorksParams extends QueryParamsUtils { "pageSize".as[Int].?, "sort".as[List[SortRequest]].?, "sortOrder".as[SortingOrder].?, - "_queryType".as[SearchQueryType].?, "query".as[String].?, "include".as[WorksIncludes].?, "aggregations".as[List[WorkAggregationRequest]].? @@ -178,7 +176,6 @@ object MultipleWorksParams extends QueryParamsUtils { pageSize, sort, sortOrder, - queryType, query, includes, aggregations @@ -246,7 +243,6 @@ object MultipleWorksParams extends QueryParamsUtils { include = includes, aggregations = aggregations, query = query, - _queryType = queryType ) validated(params.paginationErrors, params) } @@ -311,10 +307,4 @@ object MultipleWorksParams extends QueryParamsUtils { "asc" -> SortingOrder.Ascending, "desc" -> SortingOrder.Descending ) - - implicit val _queryTypeDecoder: Decoder[SearchQueryType] = - decodeOneWithDefaultOf( - SearchQueryType.default, - "MultiMatcher" -> SearchQueryType.MultiMatcher - ) } diff --git a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala index 3919ab5b85..1379e2c911 100644 --- a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala +++ b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala @@ -14,6 +14,8 @@ import weco.api.search.models.request.{ WorkAggregationRequest } import weco.api.search.rest.PaginationQuery +import weco.api.search.elasticsearch.WorksMultiMatcher + object WorksRequestBuilder extends ElasticsearchRequestBuilder[WorkSearchOptions] { @@ -128,8 +130,8 @@ object WorksRequestBuilder ): BoolQuery = searchOptions.searchQuery .map { - case SearchQuery(query, queryType) => - queryType.toEsQuery(query) + case SearchQuery(query) => + WorksMultiMatcher(query) } .getOrElse { boolQuery } diff --git a/search/src/test/scala/weco/api/search/elasticsearch/WorksQueryTest.scala b/search/src/test/scala/weco/api/search/elasticsearch/WorksQueryTest.scala index 72f973f4b3..dc46288b7b 100644 --- a/search/src/test/scala/weco/api/search/elasticsearch/WorksQueryTest.scala +++ b/search/src/test/scala/weco/api/search/elasticsearch/WorksQueryTest.scala @@ -7,7 +7,7 @@ import org.scalatest.EitherValues import weco.api.search.fixtures.{IndexFixtures, TestDocumentFixtures} import weco.api.search.generators.SearchOptionsGenerators import weco.api.search.models.index.IndexedWork -import weco.api.search.models.{SearchQuery, SearchQueryType} +import weco.api.search.models.{SearchQuery} import weco.api.search.services.WorksService import weco.fixtures.TestWith @@ -246,11 +246,11 @@ class WorksQueryTest private def assertForQueryResults[R](index: Index, query: String)( testWith: TestWith[Seq[IndexedWork.Visible], R] - ) = SearchQueryType.allowed map { queryType => + ) = { val future = worksService.listOrSearch( index, searchOptions = createWorksSearchOptionsWith( - searchQuery = Some(SearchQuery(query, queryType)) + searchQuery = Some(SearchQuery(query)) ) ) @@ -258,9 +258,7 @@ class WorksQueryTest _.right.value.results } - withClue(s"[Using query: ${queryType.name}]") { - testWith(results) - } + testWith(results) } private val worksService = new WorksService( diff --git a/search/src/test/scala/weco/api/search/works/WorksErrorsTest.scala b/search/src/test/scala/weco/api/search/works/WorksErrorsTest.scala index fae106ee5f..406c373e76 100644 --- a/search/src/test/scala/weco/api/search/works/WorksErrorsTest.scala +++ b/search/src/test/scala/weco/api/search/works/WorksErrorsTest.scala @@ -131,23 +131,6 @@ class WorksErrorsTest extends ApiWorksTestBase with TableDrivenPropertyChecks { } } - // This is expected as it's transient parameter that will have valid values changing over time - // And if there is a client with a deprecated value, we wouldn't want it to fail - describe("returns a 200 for invalid values in the ?_queryType parameter") { - it("200s despite being a unknown value") { - withWorksApi { - case (_, route) => - assertJsonResponse( - route, - path = - s"$rootPath/works?_queryType=athingwewouldneverusebutmightbecausewesaidwewouldnot" - ) { - Status.OK -> emptyJsonResult - } - } - } - } - describe("returns a 400 Bad Request for user errors") { describe("errors in the ?pageSize query") { it("not an integer") { @@ -296,7 +279,7 @@ class WorksErrorsTest extends ApiWorksTestBase with TableDrivenPropertyChecks { case (_, route) => assertBadRequest(route)( path = - s"$rootPath/works?_queryType=undefined&production.dates.from=%2B011860-01-01", + s"$rootPath/works?production.dates.from=%2B011860-01-01", description = "production.dates.from: year must be less than 9999" ) } From 5bdf32f4f2321c8e90320e20c652effc63ed26c4 Mon Sep 17 00:00:00 2001 From: Buildkite on behalf of Wellcome Collection Date: Fri, 19 May 2023 10:07:35 +0000 Subject: [PATCH 2/2] Apply auto-formatting rules --- search/src/main/scala/weco/api/search/models/SearchQuery.scala | 2 +- .../scala/weco/api/search/services/WorksRequestBuilder.scala | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/search/src/main/scala/weco/api/search/models/SearchQuery.scala b/search/src/main/scala/weco/api/search/models/SearchQuery.scala index c28651d892..72c9cdc965 100644 --- a/search/src/main/scala/weco/api/search/models/SearchQuery.scala +++ b/search/src/main/scala/weco/api/search/models/SearchQuery.scala @@ -1,3 +1,3 @@ package weco.api.search.models -case class SearchQuery(query: String) \ No newline at end of file +case class SearchQuery(query: String) diff --git a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala index 1379e2c911..1033a45639 100644 --- a/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala +++ b/search/src/main/scala/weco/api/search/services/WorksRequestBuilder.scala @@ -16,7 +16,6 @@ import weco.api.search.models.request.{ import weco.api.search.rest.PaginationQuery import weco.api.search.elasticsearch.WorksMultiMatcher - object WorksRequestBuilder extends ElasticsearchRequestBuilder[WorkSearchOptions] {