This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7851-16da22bab8d508e8aeebb436e2259f82d0ce7148 in repository https://gitbox.apache.org/repos/asf/texera.git
commit 7d78268e95ab13c5dbb753106b47c37c9b287c70 Author: yangzhang75 <[email protected]> AuthorDate: Thu Sep 3 10:17:29 2026 +0000 feat(workflow): add the published-copy columns (#7851) ### What changes were proposed in this PR? A public workflow follows the author's live content today: every save reaches the Hub immediately. Pinning a version as the public copy needs somewhere to keep that copy, which is what these columns are. Nothing writes them yet — this is the schema alone. - **Four columns on `workflow`** — `published_version_id`, `published_content`, `published_name`, `published_description`. `is_public` stays the on/off switch; `published_content` is the pin, and NULL means the workflow follows the author's latest, which is what every workflow does today. **No backfill**, so deploying this changes nothing anyone can see. - **A CHECK constraint** — `published_content IS NULL OR is_public`. A pin only means something while the workflow is public, so the other case is unrepresentable rather than guarded against: unpublishing will have to clear the copy. - **A PGroonga index** over the pinned name/description/content, mirroring the latest-content index, so public search can match a pinned workflow against its frozen copy. The expression matches the one the query builder will produce. - **Three call sites updated** — adding columns widens the jOOQ-generated positional constructor from 7 parameters to 11, so clone, duplicate and restore-a-version build their POJO with setters instead. ### Any related issues, documentation, discussions? Closes #7865 Part of #7828. Design discussion: #7128. ### How was this PR tested? `PublishedCopySchemaSpec` covers what the migration alone guarantees: the columns land NULL so every workflow keeps following the author's latest, a public workflow may carry a frozen copy, and a private one is refused by the database. `WorkflowResourceSpec` and `WorkflowVersionResourceSpec` cover the three rewritten call sites — clone, duplicate and restore-a-version still produce a private copy with new operator ids. The migration was replayed on a scratch database to confirm it applies cleanly on top of the existing schema. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (claude-opus-5) --- .../dashboard/user/workflow/WorkflowResource.scala | 40 +++- .../user/workflow/WorkflowVersionResource.scala | 9 +- .../user/workflow/PublishedCopySchemaSpec.scala | 202 +++++++++++++++++++++ sql/changelog.xml | 5 + sql/texera_ddl.sql | 38 +++- sql/updates/47.sql | 104 +++++++++++ 6 files changed, 381 insertions(+), 17 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index f29b016ac1..b8bead4b0e 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala @@ -93,6 +93,13 @@ object WorkflowResource { } private def insertWorkflow(workflow: Workflow, user: User): Unit = { + // A workflow is born with nothing pinned. The endpoint takes a whole Workflow, so without this + // a request body could seed a published copy of its own choosing. + workflow.setPublishedVersionId(null) + workflow.setPublishedContent(null) + workflow.setPublishedName(null) + workflow.setPublishedDescription(null) + workflow.setPublishedDefaultView(null) workflowDao.insert(workflow) workflowOfUserDao.insert(new WorkflowOfUser(user.getUid, workflow.getWid)) workflowUserAccessDao.insert( @@ -136,6 +143,27 @@ object WorkflowResource { case class WorkflowIDs(wids: List[Integer]) + /** + * A workflow POJO for the copy-producing paths (clone, duplicate, restore-a-version). + * + * Built with setters rather than the positional constructor, so that adding a column cannot + * silently shift a null into the wrong field -- as adding the published-copy columns would. + */ + def newUnpublishedWorkflow( + name: String, + description: String, + content: String, + defaultView: DefaultViewEnum + ): Workflow = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setDescription(description) + workflow.setContent(content) + workflow.setIsPublic(false) + workflow.setDefaultView(defaultView) + workflow + } + private def updateWorkflowField( workflow: Workflow, sessionUser: SessionUser, @@ -507,14 +535,10 @@ class WorkflowResource extends LazyLogging { for (wid <- workflowIDs.wids) { val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) val newWorkflow = createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( oldWorkflow.getName + "_copy", oldWorkflow.getDescription, assignNewOperatorIds(oldWorkflow.getContent), - null, - null, - false, // the default view is part of the workflow, so a copy keeps it oldWorkflow.getDefaultView ), @@ -546,14 +570,10 @@ class WorkflowResource extends LazyLogging { } val oldWorkflow: Workflow = workflowDao.fetchOneByWid(wid) val newWorkflow: DashboardWorkflow = createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( oldWorkflow.getName + "_clone", oldWorkflow.getDescription, assignNewOperatorIds(oldWorkflow.getContent), - null, - null, - false, // a biologist's path is hub -> clone -> use, so the clone must stay usable oldWorkflow.getDefaultView ), diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala index bae03efbd7..3a0a5b61f5 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResource.scala @@ -30,7 +30,8 @@ import org.apache.texera.dao.jooq.generated.tables.daos.{WorkflowDao, WorkflowVe import org.apache.texera.dao.jooq.generated.tables.pojos.{Workflow, WorkflowVersion} import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource.{ DashboardWorkflow, - assignNewOperatorIds + assignNewOperatorIds, + newUnpublishedWorkflow } import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowVersionResource._ import org.jooq.DSLContext @@ -428,14 +429,10 @@ class WorkflowVersionResource { val newWorkflow: DashboardWorkflow = try { workflowResource.createWorkflow( - new Workflow( - null, + newUnpublishedWorkflow( newWorkflowName, workflowVersion.getDescription, assignNewOperatorIds(workflowVersion.getContent), - null, - null, - false, // carry the workflow's current default-view preference onto the clone workflowVersion.getDefaultView ), diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala new file mode 100644 index 0000000000..aebbd2aaf9 --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala @@ -0,0 +1,202 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.web.resource.dashboard.user.workflow + +import org.apache.texera.auth.SessionUser +import org.apache.texera.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.tables.daos.{UserDao, WorkflowDao} +import org.apache.texera.dao.jooq.generated.tables.pojos.{User, Workflow} +import org.jooq.exception.DataAccessException +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +import java.time.OffsetDateTime + +/** + * The columns a pinned public copy lives in, and the constraint that keeps them honest. + * + * Nothing pins anything yet, so this covers what the schema alone guarantees: a workflow public + * today keeps behaving as it does, a private one can never carry part of a frozen copy, and the two + * endpoints that take a whole Workflow from the request body cannot seed one. + */ +class PublishedCopySchemaSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private var workflowDao: WorkflowDao = _ + private var userDao: UserDao = _ + + /** A create needs an owner row to attach the workflow to. */ + private val owner = { + val user = new User + user.setUid(Integer.valueOf(1)) + user.setName("schema_owner") + user.setEmail("[email protected]") + user.setRole(UserRoleEnum.ADMIN) + user.setComment("test") + user.setAccountCreationTime(OffsetDateTime.parse("2025-01-01T00:00:00Z")) + user + } + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + workflowDao = new WorkflowDao(getDSLContext.configuration()) + userDao = new UserDao(getDSLContext.configuration()) + userDao.insert(owner) + } + + override protected def afterAll(): Unit = shutdownDB() + + /** A workflow POJO the resource layer can take, with nothing frozen on it. */ + private def newWorkflow(name: String, isPublic: Boolean): Workflow = { + val workflow = new Workflow() + workflow.setName(name) + workflow.setDescription("a workflow") + workflow.setContent("""{"operators":[]}""") + workflow.setIsPublic(isPublic) + workflow + } + + /** The same workflow, stored: what a row looks like before anything pins it. */ + private def insertWorkflow(name: String, isPublic: Boolean): Workflow = { + val workflow = newWorkflow(name, isPublic) + workflowDao.insert(workflow) + workflowDao.fetchOneByWid(workflow.getWid) + } + + behavior of "the published-copy columns" + + it should "leave every workflow following the author's latest" in { + // The migration adds columns and no backfill, so a workflow that was public before it ran shows + // exactly what it showed: nothing is frozen, which is the state the rest of the feature calls + // "following". + val stored = insertWorkflow("migration_changes_nothing", isPublic = true) + + stored.getPublishedContent shouldBe null + stored.getPublishedName shouldBe null + stored.getPublishedDescription shouldBe null + stored.getPublishedVersionId shouldBe null + stored.getPublishedDefaultView shouldBe null + } + + it should "let a public workflow carry a frozen copy" in { + val stored = insertWorkflow("public_may_be_pinned", isPublic = true) + stored.setPublishedContent("""{"operators":[]}""") + stored.setPublishedName("frozen name") + stored.setPublishedDescription("frozen description") + stored.setPublishedDefaultView(DefaultViewEnum.CANVAS) + + workflowDao.update(stored) + + workflowDao.fetchOneByWid(stored.getWid).getPublishedName shouldBe "frozen name" + } + + it should "ignore publish columns supplied by the client on create" in { + // The columns are part of the generated POJO, and `POST /workflow/create` takes one whole. A + // client must not be able to seed a published copy of its own choosing before anything can pin. + val workflow = newWorkflow("create_cannot_inject", isPublic = false) + workflow.setPublishedContent("""{"operators":[],"note":"injected"}""") + workflow.setPublishedName("injected_name") + workflow.setPublishedDescription("injected_description") + workflow.setPublishedVersionId(1) + workflow.setPublishedDefaultView(DefaultViewEnum.FORM) + + val wid = new WorkflowResource() + .createWorkflow(workflow, new SessionUser(owner)) + .workflow + .getWid + + val stored = workflowDao.fetchOneByWid(wid) + stored.getPublishedContent shouldBe null + stored.getPublishedName shouldBe null + stored.getPublishedDescription shouldBe null + stored.getPublishedVersionId shouldBe null + stored.getPublishedDefaultView shouldBe null + } + + it should "refuse a pinned copy with no default view" in { + // The form's definition rides inside the frozen content, so a pin that did not carry the view + // would leave the public opening a frozen graph under the author's live preference. + val stored = insertWorkflow("pinned_needs_a_view", isPublic = true) + stored.setPublishedContent("""{"operators":[]}""") + stored.setPublishedName("frozen name") + + a[DataAccessException] should be thrownBy workflowDao.update(stored) + } + + it should "refuse a pinned copy with no name" in { + // A pinned copy is what the public sees, and a workflow always has a name, so a frozen copy + // without one is a half-written pin rather than a legitimate state. + val stored = insertWorkflow("pinned_needs_a_name", isPublic = true) + stored.setPublishedContent("""{"operators":[]}""") + + a[DataAccessException] should be thrownBy workflowDao.update(stored) + } + + it should "refuse a private workflow that carries any part of a frozen copy" in { + // The five columns describe one copy, so they travel together: a path that cleared four of them + // must not be able to leave the fifth behind on a private row. + val leftovers = Seq[(String, Workflow => Unit)]( + "content" -> (_.setPublishedContent("""{"operators":[]}""")), + "name" -> (_.setPublishedName("frozen name")), + "description" -> (_.setPublishedDescription("frozen description")), + "version" -> (_.setPublishedVersionId(1)), + "view" -> (_.setPublishedDefaultView(DefaultViewEnum.FORM)) + ) + for ((column, leaveBehind) <- leftovers) { + val stored = insertWorkflow(s"private_cannot_keep_$column", isPublic = false) + leaveBehind(stored) + withClue(s"a private workflow kept published_$column: ") { + a[DataAccessException] should be thrownBy workflowDao.update(stored) + } + } + } + + it should "ignore publish columns supplied by the client on save" in { + // `/workflow/persist` is the autosave path and takes a whole Workflow from the request body. If + // a save could write these columns, a client could pin a copy of its own choosing on a public + // workflow -- and unpublishing it afterwards would then fail the constraint, leaving a workflow + // that can never be made private again. + val wid = new WorkflowResource() + .createWorkflow(newWorkflow("save_cannot_inject", isPublic = true), new SessionUser(owner)) + .workflow + .getWid + + val tampered = workflowDao.fetchOneByWid(wid) + tampered.setPublishedContent("""{"operators":[],"note":"injected"}""") + tampered.setPublishedName("injected_name") + tampered.setPublishedDescription("injected_description") + tampered.setPublishedVersionId(1) + tampered.setPublishedDefaultView(DefaultViewEnum.FORM) + new WorkflowResource().persistWorkflow(tampered, new SessionUser(owner)) + + val stored = workflowDao.fetchOneByWid(wid) + stored.getPublishedContent shouldBe null + stored.getPublishedName shouldBe null + stored.getPublishedDescription shouldBe null + stored.getPublishedVersionId shouldBe null + stored.getPublishedDefaultView shouldBe null + } + +} diff --git a/sql/changelog.xml b/sql/changelog.xml index 4b59671aa5..b0c0f1354d 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -144,6 +144,11 @@ <sqlFile path="sql/updates/46.sql"/> </changeSet> + <!-- Add version-pinning columns to workflow --> + <changeSet id="47" author="yangz75"> + <sqlFile path="sql/updates/47.sql"/> + </changeSet> + <!-- example changeSet <changeSet id="1" author="author"> <sqlFile path="sql/updates/1.sql"/> diff --git a/sql/texera_ddl.sql b/sql/texera_ddl.sql index d0e5ad8050..132080c18a 100644 --- a/sql/texera_ddl.sql +++ b/sql/texera_ddl.sql @@ -169,9 +169,35 @@ CREATE TABLE IF NOT EXISTS workflow is_public BOOLEAN NOT NULL DEFAULT false, -- Which view the workflow opens in by default (CANVAS or FORM); the form's definition -- lives in workflow.content (`formBinding`). - default_view default_view_enum NOT NULL DEFAULT 'CANVAS' + default_view default_view_enum NOT NULL DEFAULT 'CANVAS', + -- is_public is the on/off switch; published_content is the pin. NULL means the public follows the + -- author's latest content, non-NULL is the frozen copy the public sees instead. Materialized + -- rather than reconstructed from workflow_version, whose rows are reverse deltas. + -- published_version_id names the version row holding that copy, which is what the revision panel + -- marks so the author can restore it. + published_version_id INT, + published_content TEXT, + published_name VARCHAR(128), + published_description TEXT, + -- Which view the pinned copy opens in: the form's definition rides inside published_content, so + -- the switch has to freeze with it or the public gets a form view over a copy with no form. + published_default_view default_view_enum ); +-- A pin only means something while the workflow is public. The four columns describe one copy, so +-- the database keeps them together: either all absent, or a pinned copy on a public workflow. +ALTER TABLE workflow + DROP CONSTRAINT IF EXISTS workflow_pin_requires_public; +ALTER TABLE workflow + ADD CONSTRAINT workflow_pin_requires_public + CHECK ( + (published_content IS NULL AND published_name IS NULL + AND published_description IS NULL AND published_version_id IS NULL + AND published_default_view IS NULL) + OR (is_public AND published_content IS NOT NULL AND published_name IS NOT NULL + AND published_default_view IS NOT NULL) + ); + -- workflow_of_user CREATE TABLE IF NOT EXISTS workflow_of_user ( @@ -705,6 +731,16 @@ BEGIN r.tablename, r.tablename, r.index_column, stem_filter ); END LOOP; + + -- Public search matches the pinned copy -- name, description and content together -- rather than + -- the author's live values, so it needs its own index alongside idx_workflow_pgroonga. The + -- expression must match the one the query builds against these columns. + EXECUTE format( + 'CREATE INDEX idx_workflow_published_pgroonga ON workflow USING pgroonga ' || + '((COALESCE(published_name, '''') || '' '' || COALESCE(published_description, '''') || '' '' || COALESCE(published_content, ''''))) ' || + 'WITH (tokenizer = ''TokenMecab''%s);', + stem_filter + ); END $$; -- END Fulltext search index creation (DO NOT EDIT THIS LINE) diff --git a/sql/updates/47.sql b/sql/updates/47.sql new file mode 100644 index 0000000000..b24dfaf43c --- /dev/null +++ b/sql/updates/47.sql @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +\c texera_db + +SET search_path TO texera_db; + +BEGIN; + +-- Version pinning: a public workflow follows the author's latest until they pin the version they +-- have now, after which the public keeps seeing that frozen copy. is_public stays the on/off switch; +-- published_content is the pin, NULL while following. Materialized rather than replayed from +-- workflow_version, whose rows are reverse deltas that no fulltext index can cover. +ALTER TABLE workflow + -- The version row holding the pinned copy. Its delta is the identity patch, so replaying it + -- returns exactly what is on public show; the revision panel marks that row, which is how the + -- author restores the public version into their editor. + ADD COLUMN IF NOT EXISTS published_version_id INT, + ADD COLUMN IF NOT EXISTS published_content TEXT, + ADD COLUMN IF NOT EXISTS published_name VARCHAR(128), + ADD COLUMN IF NOT EXISTS published_description TEXT, + -- Which view the pinned copy opens in. The form's definition rides inside published_content, so + -- without freezing this too the public would get the live preference over the frozen graph -- + -- a form view on a copy that has no form in it. The type comes from 44.sql, which always runs + -- first: a fresh database applies the changelog in order, and an existing one has it already. + ADD COLUMN IF NOT EXISTS published_default_view default_view_enum; + +-- No backfill. Every workflow that is public today has no pin, which is the following state, which is +-- exactly what it does today: deploying this migration changes nothing anyone can see. Pinning is +-- something an author opts into afterwards. + +-- A pin only means something while the workflow is public, so a private workflow must not carry one. +-- Making that unrepresentable is cheaper than catching it in every path that could break it. +-- Dropped and re-added rather than guarded on a name lookup: constraint names are unique per table, +-- not per database, so a same-named constraint on any other table would make the guard skip this one +-- and leave the workflow table without it. Mirrors texera_ddl.sql. +-- +-- All four columns are covered, not just the content: they describe one copy, so they have to be +-- either wholly absent or on a public workflow. Guarding the content alone would let a path that +-- clears three of them leave the fourth behind on a private row. +ALTER TABLE workflow + DROP CONSTRAINT IF EXISTS workflow_pin_requires_public; +ALTER TABLE workflow + ADD CONSTRAINT workflow_pin_requires_public + CHECK ( + (published_content IS NULL AND published_name IS NULL + AND published_description IS NULL AND published_version_id IS NULL + AND published_default_view IS NULL) + OR (is_public AND published_content IS NOT NULL AND published_name IS NOT NULL + AND published_default_view IS NOT NULL) + ); + +COMMIT; + +-- Fulltext index over the pinned copy, mirroring the latest-content index built in texera_ddl.sql. +-- Public search matches a pinned workflow against its pinned name, description and content rather +-- than the live ones, so those three need an index of their own; unpinned rows keep using the +-- latest-content index. The expression has to match the one public search will build against these +-- columns, or the planner cannot use it. +-- Runs outside the transaction above because the plugin probe issues its own commands. +DO +$$ + DECLARE + stem_filter TEXT := ''; + plugin_status TEXT; + BEGIN + DROP INDEX IF EXISTS idx_workflow_published_pgroonga; + + WITH plugin_registration AS (SELECT pgroonga_command('plugin_register token_filters/stem') AS result) + SELECT CASE + WHEN result::jsonb @> '[true]' THEN 'Plugin registered successfully' + ELSE 'Plugin registration failed' + END + INTO plugin_status + FROM plugin_registration; + + IF plugin_status = 'Plugin registered successfully' THEN + stem_filter := ', plugins=''token_filters/stem'', token_filters=''TokenFilterStem'''; + END IF; + + EXECUTE format( + 'CREATE INDEX idx_workflow_published_pgroonga ON workflow USING pgroonga ' || + '((COALESCE(published_name, '''') || '' '' || COALESCE(published_description, '''') || '' '' || COALESCE(published_content, ''''))) ' || + 'WITH (tokenizer = ''TokenMecab''%s);', + stem_filter + ); + END +$$;
