From 04a20841ab64a4f8f979a7e7c2c80714c6334f5a Mon Sep 17 00:00:00 2001 From: Pete F Date: Thu, 18 Jun 2026 15:46:19 +0100 Subject: [PATCH] Remove a/b test in favour of using PLAIN_NOT query variant --- .../app/controllers/QueryController.scala | 9 --- newswires/app/db/FingerpostWireEntry.scala | 79 ++++++------------- newswires/app/db/QueryVariant.scala | 34 -------- newswires/app/models/QueryResponse.scala | 5 +- .../client/src/context/SearchContext.tsx | 6 -- newswires/client/src/sharedTypes.ts | 10 --- .../test/db/FingerpostWireEntrySpec.scala | 45 ----------- newswires/test/models/QueryResponseSpec.scala | 10 +-- 8 files changed, 30 insertions(+), 168 deletions(-) delete mode 100644 newswires/app/db/QueryVariant.scala diff --git a/newswires/app/controllers/QueryController.scala b/newswires/app/controllers/QueryController.scala index 67367a11f..1e0b09551 100644 --- a/newswires/app/controllers/QueryController.scala +++ b/newswires/app/controllers/QueryController.scala @@ -16,7 +16,6 @@ import play.api.{Configuration, Logging} import service.FeatureSwitchProvider import java.time.Instant -import scala.util.Random class QueryController( val controllerComponents: ControllerComponents, @@ -95,16 +94,8 @@ class QueryController( timeStampColumn = timeStampColumn ) - val queryVariant = request.getQueryString("variant") match { - case Some("not_exists") => NotExists - case Some("plain_not") => PlainNot - case _ if Random.nextBoolean() => NotExists - case _ => PlainNot - } - val queryResponse = FingerpostWireEntry.query( queryParams, - queryVariant = queryVariant, countQueryCap = countQueryCap.getOrElse(COUNT_QUERY_CAP) ) diff --git a/newswires/app/db/FingerpostWireEntry.scala b/newswires/app/db/FingerpostWireEntry.scala index 82143694b..1b65b72f0 100644 --- a/newswires/app/db/FingerpostWireEntry.scala +++ b/newswires/app/db/FingerpostWireEntry.scala @@ -188,25 +188,9 @@ object FingerpostWireEntry object Filters { private def exclusionCondition( - alias: QuerySQLSyntaxProvider[SQLSyntaxSupport[ - FingerpostWireEntry - ], FingerpostWireEntry], - queryVariant: QueryVariant = PlainNot - )(innerClause: SQLSyntax) = { - queryVariant match { - case NotExists => - // unpleasant, but the sort of trick you need to pull - // because "NOT IN (...)" doesn't hit an index. - // https://stackoverflow.com/a/19364694 - - sqls"""|NOT EXISTS ( - | SELECT FROM ${FingerpostWireEntry as alias} - | WHERE ${syn.id} = ${alias.id} - | AND $innerClause - |)""".stripMargin - - case PlainNot => sqls"NOT ($innerClause)" - } + innerClause: SQLSyntax + ) = { + sqls"NOT ($innerClause)" } private def supplierCondition( @@ -263,10 +247,9 @@ object FingerpostWireEntry (suppliers: List[String]) => supplierCondition(syn, suppliers) def supplierExclSQL( - suppliersExcl: List[String], - queryVariant: QueryVariant = PlainNot + suppliersExcl: List[String] ) = { - exclusionCondition(syn, queryVariant)( + exclusionCondition( supplierCondition(syn, suppliersExcl) ) } @@ -279,10 +262,9 @@ object FingerpostWireEntry ) def guSourceFeedExclSQL( - guSourceFeedsExcl: List[String], - queryVariant: QueryVariant = PlainNot + guSourceFeedsExcl: List[String] ) = { - exclusionCondition(syn, queryVariant)( + exclusionCondition( sqls.in( sqls"upper(${syn.guSourceFeed})", guSourceFeedsExcl.map(feed => sqls"upper($feed)") @@ -340,10 +322,9 @@ object FingerpostWireEntry (keywords: List[String]) => keywordCondition(syn, keywords) def keywordsExclSQL( - keywords: List[String], - queryVariant: QueryVariant = PlainNot + keywords: List[String] ): SQLSyntax = { - exclusionCondition(syn, queryVariant)(keywordCondition(syn, keywords)) + exclusionCondition(keywordCondition(syn, keywords)) } lazy val categoryCodeInclSQL = @@ -356,10 +337,9 @@ object FingerpostWireEntry } def categoryCodeExclSQL( - categoryCodesExcl: List[String], - queryVariant: QueryVariant = PlainNot + categoryCodesExcl: List[String] ) = { - exclusionCondition(syn, queryVariant)( + exclusionCondition( categoryCodeSomeConditions(syn, categoryCodesExcl) ) } @@ -403,10 +383,9 @@ object FingerpostWireEntry } def preComputedCategoriesExclSQL( - preComputedCategories: List[String], - queryVariant: QueryVariant = PlainNot + preComputedCategories: List[String] ) = { - exclusionCondition(syn, queryVariant)( + exclusionCondition( preComputedCategoriesConditions(syn, preComputedCategories) ) } @@ -431,8 +410,7 @@ object FingerpostWireEntry } private[db] def filtersBuilder( - filters: FilterParams, - queryVariant: QueryVariant = PlainNot + filters: FilterParams ): Option[SQLSyntax] = { val suppliersQuery: Option[SQLSyntax] = filters.suppliersIncl match { case Nil => None @@ -442,7 +420,7 @@ object FingerpostWireEntry val suppliersExclQuery: Option[SQLSyntax] = filters.suppliersExcl match { case Nil => None case suppliersExcl => - Some(Filters.supplierExclSQL(suppliersExcl, queryVariant)) + Some(Filters.supplierExclSQL(suppliersExcl)) } val searchQuery: Option[SQLSyntax] = @@ -455,7 +433,7 @@ object FingerpostWireEntry val keywordsExclQuery = filters.keywordExcl match { case Nil => None - case keywords => Some(Filters.keywordsExclSQL(keywords, queryVariant)) + case keywords => Some(Filters.keywordsExclSQL(keywords)) } val categoryCodesInclQuery = filters.categoryCodesIncl match { @@ -467,7 +445,7 @@ object FingerpostWireEntry val categoryCodesExclQuery = filters.categoryCodesExcl match { case Nil => None case categoryCodesExcl => - Some(Filters.categoryCodeExclSQL(categoryCodesExcl, queryVariant)) + Some(Filters.categoryCodeExclSQL(categoryCodesExcl)) } val hasDataFormattingQuery = filters.hasDataFormatting match { @@ -488,8 +466,7 @@ object FingerpostWireEntry case presetCategoriesExcl => Some( Filters.preComputedCategoriesExclSQL( - presetCategoriesExcl, - queryVariant + presetCategoriesExcl ) ) } @@ -507,7 +484,7 @@ object FingerpostWireEntry val guSourceFeedExclQuery = filters.guSourceFeedsExcl match { case Nil => None case sourceFeedsExcl => - Some(Filters.guSourceFeedExclSQL(sourceFeedsExcl, queryVariant)) + Some(Filters.guSourceFeedExclSQL(sourceFeedsExcl)) } val eventCodeQuery = filters.eventCode match { @@ -535,10 +512,9 @@ object FingerpostWireEntry } private[db] def presetsBuilder( - presets: List[FilterParams], - queryVariant: QueryVariant = PlainNot + presets: List[FilterParams] ): Option[SQLSyntax] = { - val andClauses = presets.flatMap(p => filtersBuilder(p, queryVariant)) + val andClauses = presets.flatMap(p => filtersBuilder(p)) orAll(andClauses) } @@ -562,8 +538,7 @@ object FingerpostWireEntry queryCursor: QueryCursor, queryOrdering: TimeStampColumn, searchPresets: List[FilterParams] = Nil, - negatedSearchPresets: List[FilterParams] = Nil, - queryVariant: QueryVariant = PlainNot + negatedSearchPresets: List[FilterParams] = Nil ): SQLSyntax = { val dataOnlyWhereClauses = queryCursorQuery(queryCursor, queryOrdering) @@ -572,8 +547,8 @@ object FingerpostWireEntry searchParams.dateRange.end, queryOrdering ) - val customSearchClauses = filtersBuilder(searchParams.filters, queryVariant) - val presetSearchClauses = presetsBuilder(searchPresets, queryVariant) + val customSearchClauses = filtersBuilder(searchParams.filters) + val presetSearchClauses = presetsBuilder(searchPresets) val negatedPresetSearchClauses = presetsBuilder(negatedSearchPresets).map(clause => sqls"NOT $clause") @@ -653,8 +628,7 @@ object FingerpostWireEntry def query( queryParams: QueryParams, - countQueryCap: Long = COUNT_QUERY_CAP, - queryVariant: QueryVariant = PlainNot + countQueryCap: Long = COUNT_QUERY_CAP ): QueryResponse = DB readOnly { implicit session => val whereClause = buildWhereClause( queryParams.searchParams, @@ -721,8 +695,7 @@ object FingerpostWireEntry QueryResponse( results, totalCount, - countQueryCap, - queryVariant + countQueryCap ) } diff --git a/newswires/app/db/QueryVariant.scala b/newswires/app/db/QueryVariant.scala deleted file mode 100644 index 0be45a909..000000000 --- a/newswires/app/db/QueryVariant.scala +++ /dev/null @@ -1,34 +0,0 @@ -package db - -import io.circe.{Decoder, Encoder} - -sealed abstract class QueryVariant( - val name: String, - val description: String -) - -object QueryVariant { - implicit val encoder: Encoder[QueryVariant] = - Encoder.forProduct2("name", "description")(v => (v.name, v.description)) - - implicit val decoder: Decoder[QueryVariant] = - Decoder[String].emap { - case "not_exists" => Right(NotExists) - case "plain_not" => Right(PlainNot) - case other => Left(s"Unknown QueryVariant: $other") - } -} - -object NotExists - extends QueryVariant( - name = "not_exists", - description = - "Uses a 'NOT EXISTS (...)' clause to exclude results that match the search term" - ) - -object PlainNot - extends QueryVariant( - name = "plain_not", - description = - "Uses a plain NOT clause to exclude results that match the search term" - ) diff --git a/newswires/app/models/QueryResponse.scala b/newswires/app/models/QueryResponse.scala index 7cd0ae924..a0f9c2097 100644 --- a/newswires/app/models/QueryResponse.scala +++ b/newswires/app/models/QueryResponse.scala @@ -1,14 +1,13 @@ package models -import db.{FingerpostWireEntry, QueryVariant, TimeStampColumn, ToolLink} +import db.{FingerpostWireEntry, TimeStampColumn, ToolLink} import io.circe.generic.semiauto.{deriveDecoder, deriveEncoder} import io.circe.{Decoder, Encoder} case class QueryResponse( results: List[FingerpostWireEntry], totalCount: Long, - countQueryCap: Long, - queryVariant: QueryVariant + countQueryCap: Long // keywordCounts: Map[String, Int] ) diff --git a/newswires/client/src/context/SearchContext.tsx b/newswires/client/src/context/SearchContext.tsx index 647dd4108..61635cf7d 100644 --- a/newswires/client/src/context/SearchContext.tsx +++ b/newswires/client/src/context/SearchContext.tsx @@ -643,11 +643,5 @@ function createFetchedResultsTelemetryData({ totalCount: data.totalCount, isRefresh, requestId, - ...Object.fromEntries( - Object.entries(data.queryVariant ?? {}).map(([key, value]) => [ - `query-variant_${key}`, - JSON.stringify(value), - ]), - ), }; } diff --git a/newswires/client/src/sharedTypes.ts b/newswires/client/src/sharedTypes.ts index 4b50f1e6a..487375b9d 100644 --- a/newswires/client/src/sharedTypes.ts +++ b/newswires/client/src/sharedTypes.ts @@ -95,12 +95,6 @@ export const WiresQueryResponseSchema = z.object({ results: z.array(WireDataFromAPISchema), totalCount: z.number(), countQueryCap: z.number(), - queryVariant: z - .object({ - name: z.string(), - description: z.string(), - }) - .optional(), // keywordCounts: z.record(z.string(), z.number()), }); @@ -150,10 +144,6 @@ export type WiresQueryData = { results: WireData[]; totalCount: number; countQueryCap: number; - queryVariant?: { - name: string; - description: string; - }; }; export const isValidDateValue = (value: string): value is EuiDateString => diff --git a/newswires/test/db/FingerpostWireEntrySpec.scala b/newswires/test/db/FingerpostWireEntrySpec.scala index 707491bd5..a4b39046c 100644 --- a/newswires/test/db/FingerpostWireEntrySpec.scala +++ b/newswires/test/db/FingerpostWireEntrySpec.scala @@ -786,51 +786,6 @@ class FingerpostWireEntrySpec extends AnyFlatSpec with Matchers with models { ) } - behavior of "exclusion clauses when NotExists variant is specified" - it should "create the correct sql snippet for suppliersExcl when NotExists variant is specified" in { - val supplierExclClause = - "NOT EXISTS ( SELECT FROM fingerpost_wire_entry fm WHERE fm.id = fm.id AND upper(fm.supplier) in (upper(?)) )" - val suppliersExclSQL = - FingerpostWireEntry.Filters.supplierExclSQL(List("supplier"), NotExists) - suppliersExclSQL should matchSqlSnippet( - expectedClause = supplierExclClause, - expectedParams = List("supplier") - ) - } - it should "create the correct sql snippet for categoryCodesExcl when NotExists variant is specified" in { - val categoryExclClause = - "NOT EXISTS ( SELECT FROM fingerpost_wire_entry fm WHERE fm.id = fm.id AND fm.category_codes && ? )" - val categoryCodesExcl = - FingerpostWireEntry.Filters.categoryCodeExclSQL(List("code"), NotExists) - categoryCodesExcl should matchSqlSnippet( - expectedClause = categoryExclClause, - expectedParams = List(List("code")) - ) - } - it should "create the correct sql snippet for precomputedCategoriesExcl when NotExists variant is specified" in { - val precomputedCategoriesExclClause = - "NOT EXISTS ( SELECT FROM fingerpost_wire_entry fm WHERE fm.id = fm.id AND fm.precomputed_categories && ? )" - val precomputedCategoriesExcl = - FingerpostWireEntry.Filters.preComputedCategoriesExclSQL( - List("category"), - NotExists - ) - precomputedCategoriesExcl should matchSqlSnippet( - expectedClause = precomputedCategoriesExclClause, - expectedParams = List(List("category")) - ) - } - it should "create the correct sql snippet for keywordsExcl when NotExists variant is specified" in { - val keywordExclClause = - "NOT EXISTS ( SELECT FROM fingerpost_wire_entry fm WHERE fm.id = fm.id AND (fm.content -> 'keywords') ??| ? )" - val keywordExclSQL = - FingerpostWireEntry.Filters.keywordsExclSQL(List("keyword"), NotExists) - keywordExclSQL should matchSqlSnippet( - expectedClause = keywordExclClause, - expectedParams = List(List("keyword")) - ) - } - behavior of "dataformatting SQL helpers" it should "create the correct sql snippet for hasDataFormatting set to true" in { val hasDataFormattingSQL = diff --git a/newswires/test/models/QueryResponseSpec.scala b/newswires/test/models/QueryResponseSpec.scala index a1e04be5f..aa250bbeb 100644 --- a/newswires/test/models/QueryResponseSpec.scala +++ b/newswires/test/models/QueryResponseSpec.scala @@ -1,11 +1,6 @@ package models -import db.{ - AddedToCollectionAtTime, - FingerpostWireEntry, - IngestedAtTime, - PlainNot -} +import db.{AddedToCollectionAtTime, FingerpostWireEntry, IngestedAtTime} import helpers.models import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers @@ -20,8 +15,7 @@ class QueryResponseSpec extends AnyFlatSpec with Matchers with models { QueryResponse( results = results, totalCount = results.length, - countQueryCap = FingerpostWireEntry.COUNT_QUERY_CAP, - queryVariant = PlainNot + countQueryCap = FingerpostWireEntry.COUNT_QUERY_CAP ) behavior of "QueryResponse.display"