From 35c685a8b74f6366d435b61f229e2b7819550333 Mon Sep 17 00:00:00 2001
From: Cedric Cordenier <cedric.cordenier@smartcontract.com>
Date: Fri, 13 Dec 2024 11:14:55 +0000
Subject: [PATCH] [CAPPL-394] Some database changes

- Allow multiple workflows to reference the same secrets file
- Change workflow specs uniqueness constraint so it includes the
  workflow id
---
 core/services/workflows/syncer/orm.go         |  4 +-
 core/services/workflows/syncer/orm_test.go    | 42 +++++++++++++++++++
 .../workflows/syncer/workflow_registry.go     |  2 +-
 .../migrations/0259_add_workflow_secrets.sql  |  3 +-
 ..._alter_unique_constraint_workflow_name.sql | 14 +++++++
 5 files changed, 61 insertions(+), 4 deletions(-)
 create mode 100644 core/store/migrate/migrations/0261_alter_unique_constraint_workflow_name.sql

diff --git a/core/services/workflows/syncer/orm.go b/core/services/workflows/syncer/orm.go
index bd0501795e6..6e0c69f6d76 100644
--- a/core/services/workflows/syncer/orm.go
+++ b/core/services/workflows/syncer/orm.go
@@ -238,7 +238,7 @@ func (orm *orm) UpsertWorkflowSpec(ctx context.Context, spec *job.WorkflowSpec)
 			:created_at,
 			:updated_at,
 			:spec_type
-		) ON CONFLICT (workflow_owner, workflow_name) DO UPDATE
+		) ON CONFLICT (workflow_owner, workflow_name, workflow_id) DO UPDATE
 		SET
 			workflow = EXCLUDED.workflow,
 			config = EXCLUDED.config,
@@ -322,7 +322,7 @@ func (orm *orm) UpsertWorkflowSpecWithSecrets(
 				:created_at,
 				:updated_at,
 				:spec_type
-			) ON CONFLICT (workflow_owner, workflow_name) DO UPDATE
+			) ON CONFLICT (workflow_owner, workflow_name, workflow_id) DO UPDATE
 			SET
 				workflow = EXCLUDED.workflow,
 				config = EXCLUDED.config,
diff --git a/core/services/workflows/syncer/orm_test.go b/core/services/workflows/syncer/orm_test.go
index a94233e78a1..b6a705752bc 100644
--- a/core/services/workflows/syncer/orm_test.go
+++ b/core/services/workflows/syncer/orm_test.go
@@ -6,6 +6,8 @@ import (
 	"testing"
 	"time"
 
+	"github.com/google/uuid"
+
 	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils"
 	"github.com/smartcontractkit/chainlink/v2/core/internal/testutils/pgtest"
 	"github.com/smartcontractkit/chainlink/v2/core/logger"
@@ -372,4 +374,44 @@ func Test_UpsertWorkflowSpecWithSecrets(t *testing.T) {
 		require.NoError(t, err)
 		require.Equal(t, "new contents", contents)
 	})
+
+	t.Run("updates existing spec and secrets if spec has executions", func(t *testing.T) {
+		giveURL := "https://example.com"
+		giveBytes, err := crypto.Keccak256([]byte(giveURL))
+		require.NoError(t, err)
+		giveHash := hex.EncodeToString(giveBytes)
+		giveContent := "some contents"
+
+		spec := &job.WorkflowSpec{
+			Workflow:      "test_workflow",
+			Config:        "test_config",
+			WorkflowID:    "cid-123",
+			WorkflowOwner: "owner-123",
+			WorkflowName:  "Test Workflow",
+			Status:        job.WorkflowSpecStatusActive,
+			BinaryURL:     "http://example.com/binary",
+			ConfigURL:     "http://example.com/config",
+			CreatedAt:     time.Now(),
+			SpecType:      job.WASMFile,
+		}
+
+		_, err = orm.UpsertWorkflowSpecWithSecrets(ctx, spec, giveURL, giveHash, giveContent)
+		require.NoError(t, err)
+
+		_, err = db.ExecContext(
+			ctx,
+			`INSERT INTO workflow_executions (id, workflow_id, status, created_at) VALUES ($1, $2, $3, $4)`,
+			uuid.New().String(),
+			"cid-123",
+			"started",
+			time.Now(),
+		)
+		require.NoError(t, err)
+
+		// Update the status
+		spec.WorkflowID = "cid-456"
+
+		_, err = orm.UpsertWorkflowSpecWithSecrets(ctx, spec, giveURL, giveHash, "new contents")
+		require.NoError(t, err)
+	})
 }
diff --git a/core/services/workflows/syncer/workflow_registry.go b/core/services/workflows/syncer/workflow_registry.go
index 223fbe8e758..b80eab03b4a 100644
--- a/core/services/workflows/syncer/workflow_registry.go
+++ b/core/services/workflows/syncer/workflow_registry.go
@@ -333,7 +333,7 @@ func (w *workflowRegistry) readRegistryEvents(ctx context.Context, reader Contra
 			for _, event := range events {
 				err := w.handler.Handle(ctx, event.Event)
 				if err != nil {
-					w.lggr.Errorw("failed to handle event", "err", err)
+					w.lggr.Errorw("failed to handle event", "err", err, "type", event.Event.EventType)
 				}
 			}
 		}
diff --git a/core/store/migrate/migrations/0259_add_workflow_secrets.sql b/core/store/migrate/migrations/0259_add_workflow_secrets.sql
index fb76d945571..420f7ed6e49 100644
--- a/core/store/migrate/migrations/0259_add_workflow_secrets.sql
+++ b/core/store/migrate/migrations/0259_add_workflow_secrets.sql
@@ -38,4 +38,5 @@ DROP INDEX IF EXISTS idx_secrets_url_hash;
 
 -- Drop the workflow_artifacts table
 DROP TABLE IF EXISTS workflow_secrets;
--- +goose StatementEnd
\ No newline at end of file
+-- +goose StatementEnd
+
diff --git a/core/store/migrate/migrations/0261_alter_unique_constraint_workflow_name.sql b/core/store/migrate/migrations/0261_alter_unique_constraint_workflow_name.sql
new file mode 100644
index 00000000000..0b297bbe56e
--- /dev/null
+++ b/core/store/migrate/migrations/0261_alter_unique_constraint_workflow_name.sql
@@ -0,0 +1,14 @@
+-- +goose Up
+-- +goose StatementBegin
+-- unique constraint on workflow_owner and workflow_name
+ALTER TABLE workflow_specs DROP CONSTRAINT unique_workflow_owner_name;
+ALTER TABLE workflow_specs ADD CONSTRAINT unique_workflow_owner_name_id unique (workflow_owner, workflow_name, workflow_id);
+ALTER TABLE workflow_specs DROP CONSTRAINT workflow_specs_secrets_id_key;
+-- +goose StatementEnd
+
+-- +goose Down
+-- +goose StatementBegin
+ALTER TABLE workflow_specs DROP CONSTRAINT unique_workflow_owner_name_id;
+ALTER TABLE workflow_specs ADD CONSTRAINT unique_workflow_owner_name unique (workflow_owner, workflow_name);
+ALTER TABLE workflow_specs ADD CONSTRAINT workflow_specs_secrets_id_key unique (secrets_id);
+-- +goose StatementEnd