fix(controlplane): bump revision only during workflow contract update (#468)

Signed-off-by: Fabián Ezequiel Gallina <[email protected]>

Author: fgallina

app/controlplane/internal/biz/workflowrun_integration_test.go

@@ -19,6 +19,7 @@ import (
 	"context"
 	"testing"
 
+	schemav1 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1"
 	"github.com/chainloop-dev/chainloop/app/controlplane/internal/biz"
 	"github.com/chainloop-dev/chainloop/app/controlplane/internal/biz/testhelpers"
 	"github.com/chainloop-dev/chainloop/internal/credentials"
@@ -198,6 +199,64 @@ func (s *workflowRunIntegrationTestSuite) TestCreate() {
 	})
 }
 
+func (s *workflowRunIntegrationTestSuite) TestUpdate() {
+	testCases := []struct {
+		name         string
+		contractID   string
+		inputName    string
+		inputSchema  *schemav1.CraftingSchema
+		wantError    bool
+		wantRevision int
+	}{
+		{
+			name:       "invalid contract",
+			contractID: uuid.NewString(),
+			wantError:  true,
+		},
+		{
+			name:         "update only name does not bump revision",
+			contractID:   s.contractVersion.Contract.ID.String(),
+			inputName:    "new name",
+			wantRevision: 1,
+		},
+		{
+			name:         "updating schema bumps revision",
+			contractID:   s.contractVersion.Contract.ID.String(),
+			inputSchema:  &schemav1.CraftingSchema{SchemaVersion: "v123"},
+			wantRevision: 2,
+		},
+		{
+			name:         "updating with same schema DOES NOT bump revision",
+			contractID:   s.contractVersion.Contract.ID.String(),
+			inputSchema:  &schemav1.CraftingSchema{SchemaVersion: "v123"},
+			wantRevision: 2,
+		},
+		{
+			name:         "updating with same schema but different name DOES NOT bump revision either",
+			contractID:   s.contractVersion.Contract.ID.String(),
+			inputSchema:  &schemav1.CraftingSchema{SchemaVersion: "v123"},
+			inputName:    "new new name",
+			wantRevision: 2,
+		},
+	}
+
+	for _, tc := range testCases {
+		s.T().Run(tc.name, func(t *testing.T) {
+			contract, err := s.WorkflowContract.Update(context.Background(), s.org.ID, tc.contractID, tc.inputName, tc.inputSchema)
+			if tc.wantError {
+				s.Error(err)
+				return
+			}
+
+			s.NoError(err)
+			s.Equal(tc.wantRevision, contract.Version.Revision)
+			if tc.inputName != "" {
+				s.Equal(tc.inputName, contract.Contract.Name)
+			}
+		})
+	}
+}
+
 // Run the tests
 func TestWorkflowRunUseCase(t *testing.T) {
 	suite.Run(t, new(workflowRunIntegrationTestSuite))

app/controlplane/internal/data/workflowcontract.go

@@ -16,6 +16,7 @@
 package data
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"time"
@@ -180,18 +181,18 @@ func (r *WorkflowContractRepo) Update(ctx context.Context, opts *biz.ContractUpd
 		}
 	}
 
-	version, err := latestVersion(ctx, contract)
+	latestVersion, err := latestVersion(ctx, contract)
 	if err != nil {
 		return nil, err
 	}
 
-	// Create a revision only if we are providing a new contract
-	if opts.ContractBody != nil {
-		// TODO: Add pesimist locking to make sure we are incrementing the latest revision
-		version, err = tx.WorkflowContractVersion.Create().
+	// Create a revision only if we are providing a new contract and it has changed
+	if opts.ContractBody != nil && !bytes.Equal(latestVersion.Body, opts.ContractBody) {
+		// TODO: Add pessimist locking to make sure we are incrementing the latest revision
+		latestVersion, err = tx.WorkflowContractVersion.Create().
 			SetBody(opts.ContractBody).
 			SetContract(contract).
-			SetRevision(version.Revision + 1).
+			SetRevision(latestVersion.Revision + 1).
 			Save(ctx)
 		if err != nil {
 			return nil, rollback(tx, err)
@@ -207,7 +208,7 @@ func (r *WorkflowContractRepo) Update(ctx context.Context, opts *biz.ContractUpd
 		return nil, err
 	}
 
-	v, err := entContractVersionToBizContractVersion(version)
+	v, err := entContractVersionToBizContractVersion(latestVersion)
 	if err != nil {
 		return nil, err
 	}