Copilot commented on code in PR #7851: URL: https://github.com/apache/texera/pull/7851#discussion_r3847065260
########## sql/updates/41.sql: ########## @@ -0,0 +1,93 @@ +/* + * 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; + +-- 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: unpublishing clears the pin, and no other +-- path writes these columns. +DO +$$ + BEGIN + ALTER TABLE workflow DROP CONSTRAINT IF EXISTS workflow_published_consistent; + IF NOT EXISTS (SELECT 1 FROM pg_constraint WHERE conname = 'workflow_pin_requires_public') THEN + ALTER TABLE workflow + ADD CONSTRAINT workflow_pin_requires_public + CHECK (published_content IS NULL OR is_public); + END IF; Review Comment: `pg_constraint.conname` is only unique per relation, so an unrelated table or schema can already have this name. In that case this migration silently skips the workflow constraint and permits private pinned rows. Drop `workflow_pin_requires_public` from `workflow` and re-add it, as `texera_ddl.sql` already does. ########## amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/PublishedCopySchemaSpec.scala: ########## @@ -0,0 +1,93 @@ +/* + * 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.dao.MockTexeraDB +import org.apache.texera.dao.jooq.generated.tables.daos.WorkflowDao +import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow +import org.jooq.exception.DataAccessException +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +/** + * The columns a pinned public copy lives in, and the constraint that keeps them honest. + * + * Nothing writes them yet: this covers what the migration alone guarantees -- that a workflow + * public today keeps behaving as it does, and that a private workflow can never carry a pin. + */ +class PublishedCopySchemaSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with MockTexeraDB { + + private var workflowDao: WorkflowDao = _ + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + workflowDao = new WorkflowDao(getDSLContext.configuration()) + } Review Comment: This suite leaves its per-suite Hikari data source open. Other `MockTexeraDB` suites close it in `afterAll` (for example, `WorkflowVersionResourceSpec.scala:153-155` and `WorkflowResourceSpec.scala:223-225`); without that teardown, this new suite retains pool connections for the rest of the test JVM. ########## sql/texera_ddl.sql: ########## @@ -164,9 +164,28 @@ CREATE TABLE IF NOT EXISTS workflow content TEXT NOT NULL, creation_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, last_modified_time TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, - is_public BOOLEAN NOT NULL DEFAULT false + is_public BOOLEAN NOT NULL DEFAULT false, + -- 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 Review Comment: Adding these fields to the generated `Workflow` POJO also makes them writable through `POST /workflow/create`: that endpoint accepts a `Workflow` directly and passes it unchanged to `workflowDao.insert` (`WorkflowResource.scala:95-97,592-598`). A client can therefore create a public pinned copy now and bypass the future pin/anchor service, contradicting the stated schema-only rollout. Reject/clear these fields on creation or introduce a create-request DTO that does not expose them. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
