From a9996d3c0d9641696e62551e7f736d7fb8980eda Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Fri, 4 Sep 2026 04:09:49 -0700 Subject: [PATCH 1/2] test(amber): drop the unused pgroonga override from DatasetResourceSpec `DatasetResourceSpec.beforeAll` set the JVM-global `FulltextSearchQueryUtils.usePgroonga` to `false` and never put it back. The write had no effect on this suite: both of its `constructQuery` calls pass no keywords, so `getFullTextSearchFilter` returns at its empty-keyword guard before it ever reads the flag. Its only effect was to leave `false` behind for whichever suites sbt schedules after it in the same JVM (amber has no `Test / fork`), silently moving their full-text rendering onto the `to_tsvector`/`to_tsquery` arm. Delete the write rather than capture-and-restore it: with nothing here reading the flag, leaving it at its production default removes the leak outright, with no dependence on when the suite object is constructed or on how sbt orders the run. The `getKeywordsArray` helper that came along with the write is deleted too -- it has no caller (`WorkflowResourceSpec` has its own private copy) -- and the two imports that only those two members needed go with it. `DatasetSearchQueryBuilderSpec`'s header paragraph asserted this leak as fact while explaining why its keyword assertions must stay branch-independent. The instruction stands, so the paragraph now justifies it by the flag being JVM-global mutable state rather than by any particular suite's behaviour, and states precisely what survives onto both arms -- the concatenated `coalesce` expression and each individual keyword token, but not their joining, which the two arms render differently. --- .../DatasetSearchQueryBuilderSpec.scala | 25 +++++++++++-------- .../dashboard/file/DatasetResourceSpec.scala | 19 ++++++-------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala index 6987540fac1..839c36dde7a 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala @@ -111,16 +111,21 @@ import scala.jdk.CollectionConverters._ * `WorkflowExecutionService` a test->test dependency on `DAO` and `Auth` only, so workflow-core's * test tree is not on this module's test classpath. * - * The keyword tests RENDER a full-text predicate (they do not fetch one), which does read the - * JVM-global `FulltextSearchQueryUtils.usePgroonga` and emits whichever arm it currently selects: - * `pgroonga_condition(...)` when this suite runs alone, the `to_tsvector`/`to_tsquery` arm if - * `DatasetResourceSpec` or `WorkflowResourceSpec` ran earlier in this JVM and left the global - * `false` (both set it and neither restores it; amber has no `Test / fork`). This suite therefore - * neither touches nor restores that global, and every keyword assertion here is deliberately - * branch-independent: the tokens themselves and the `coalesce(...) || ' ' || coalesce(...)` - * expression are built at `FulltextSearchQueryUtils:49-51`, *before* the `if (usePgroonga)`. - * Anything added here must keep that property — an assertion on `pgroonga_condition` would pass - * solo and fail in a full-module run. + * The keyword tests RENDER a full-text predicate (they do not fetch one), and rendering reads the + * JVM-global `FulltextSearchQueryUtils.usePgroonga`: the `pgroonga_condition(...)` arm while it + * holds `true`, the `to_tsvector`/`to_tsquery` arm while it holds `false`. That flag is a plain + * mutable `var` and amber has no `Test / fork`, so its value here is whatever the suites sharing + * this JVM have left it at — not something this spec controls or should assume. This suite + * therefore neither touches nor restores it, and every keyword assertion here is deliberately + * branch-independent. Two things reach both arms: the `coalesce(...) || ' ' || coalesce(...)` + * expression, built at `FulltextSearchQueryUtils:49-51` *before* the `if (usePgroonga)` and then + * embedded verbatim by either arm, and each INDIVIDUAL keyword token. Their JOINING does not — + * the `true` arm space-joins the whole list into one literal (`:56`, rendering + * `pgroonga_condition('alpha beta', ...)`), while the `false` arm emits one predicate per keyword + * and joins the words *inside* a keyword with ` & ` (`:62-65`, rendering + * `to_tsquery('english', 'alpha & beta')`). So assert on individual tokens — never on a joined + * multi-token string, and never on one arm's own output; either would tie this spec's result to + * whichever other suites wrote the flag first. * * The `record.into(USER).into(classOf[User]).getEmail` in `VersionedResourceTables.hydrate` used to * be executed but unobservable: the dataset schema left `UnifiedResourceSchema`'s `userEmail` at its diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala index 70b8252c096..7e58489556b 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala @@ -25,7 +25,6 @@ import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum import org.apache.texera.dao.jooq.generated.tables.pojos.User import org.apache.texera.web.resource.dashboard.DashboardResource.SearchQueryParams import org.apache.texera.web.resource.dashboard.user.dataset.DatasetResource.DashboardDataset -import org.apache.texera.web.resource.dashboard.{FulltextSearchQueryUtils} import org.apache.texera.web.resource.dashboard.DatasetSearchQueryBuilder import org.scalatest.flatspec.AnyFlatSpec import org.apache.texera.dao.jooq.generated.tables.daos.{UserDao, DatasetDao, DatasetUserAccessDao} @@ -33,7 +32,6 @@ import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum import org.apache.texera.dao.jooq.generated.tables.pojos.{Dataset, DatasetUserAccess} import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} import java.time.OffsetDateTime -import java.util import org.apache.texera.web.resource.dashboard.SearchQueryBuilder.DATASET_RESOURCE_TYPE class DatasetResourceSpec @@ -96,7 +94,14 @@ class DatasetResourceSpec override protected def beforeAll(): Unit = { initializeDBAndReplaceDSLContext() - FulltextSearchQueryUtils.usePgroonga = false // disable pgroonga + // `FulltextSearchQueryUtils.usePgroonga` is deliberately left at its production default here: + // no test in this suite reaches the read, because both `constructQuery` calls below pass no + // keywords and `getFullTextSearchFilter` returns before that read on an empty keyword list. + // A keyword test added here would need the `to_tsvector` arm instead — `MockTexeraDB` strips + // the full-text index block out of the DDL, so the embedded Postgres has no pgroonga extension + // — and would have to set the flag and put it back: amber has no `Test / fork`, so a value + // left behind here follows every suite scheduled after this one in the same JVM. + // add test user directly val userDao = new UserDao(getDSLContext.configuration()) userDao.insert(ownerUser) @@ -124,14 +129,6 @@ class DatasetResourceSpec shutdownDB() } - private def getKeywordsArray(keywords: String*): util.ArrayList[String] = { - val keywordsList = new util.ArrayList[String]() - for (keyword <- keywords) { - keywordsList.add(keyword) - } - keywordsList - } - private def assertSameDataset(a: Dataset, b: DashboardDataset): Unit = { assert(a.getName == b.dataset.getName) } From d5e08b348ae8b55eb303465d47987461c2ddfca0 Mon Sep 17 00:00:00 2001 From: Xinyuan Lin Date: Fri, 4 Sep 2026 22:52:04 -0700 Subject: [PATCH 2/2] Refactor comments for clarity in DatasetSearchQueryBuilderSpec Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Xinyuan Lin --- .../dashboard/DatasetSearchQueryBuilderSpec.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala index 839c36dde7a..6be27bc25ce 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/DatasetSearchQueryBuilderSpec.scala @@ -118,11 +118,11 @@ import scala.jdk.CollectionConverters._ * this JVM have left it at — not something this spec controls or should assume. This suite * therefore neither touches nor restores it, and every keyword assertion here is deliberately * branch-independent. Two things reach both arms: the `coalesce(...) || ' ' || coalesce(...)` - * expression, built at `FulltextSearchQueryUtils:49-51` *before* the `if (usePgroonga)` and then - * embedded verbatim by either arm, and each INDIVIDUAL keyword token. Their JOINING does not — - * the `true` arm space-joins the whole list into one literal (`:56`, rendering + * expression, built in `FulltextSearchQueryUtils` as `combinedFields` before the `if (usePgroonga)` + * branch and embedded verbatim by either arm, and each INDIVIDUAL keyword token. Their JOINING does + * not — the `true` arm space-joins the full keyword list into one literal (rendering * `pgroonga_condition('alpha beta', ...)`), while the `false` arm emits one predicate per keyword - * and joins the words *inside* a keyword with ` & ` (`:62-65`, rendering + * and joins the words *inside* a keyword with ` & ` (rendering `to_tsquery('english', 'alpha & beta')`). * `to_tsquery('english', 'alpha & beta')`). So assert on individual tokens — never on a joined * multi-token string, and never on one arm's own output; either would tie this spec's result to * whichever other suites wrote the flag first.