refactor: add workflowcheck linter and conform to it

satisfactorymodding · Aug 23, 2024 · d574774 · d574774
1 parent 0aa96ab
commit d574774
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 25 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -33,6 +33,19 @@ jobs:
           version: v1.60.1
           args: --timeout 5m
 
+  workflowcheck:
+    name: Workflow Check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: jetify-com/devbox-install-action@v0.11.0
+        with:
+          enable-cache: true
+
+      - name: Workflow Check
+        run: devbox run -- go run go.temporal.io/sdk/contrib/tools/workflowcheck@latest ./...
+
   test:
     name: Test
     runs-on: ubuntu-latest

diff --git a/gql/resolver_versions.go b/gql/resolver_versions.go
@@ -14,6 +14,7 @@ import (
 	"github.com/Vilsol/slox"
 	"github.com/dgraph-io/ristretto"
 	"github.com/pkg/errors"
+	"github.com/spf13/viper"
 	"go.temporal.io/sdk/client"
 
 	"github.com/satisfactorymodding/smr-api/dataloader"
@@ -113,7 +114,7 @@ func (r *mutationResolver) FinalizeCreateVersion(ctx context.Context, modID stri
 	if _, err := workflows.Client(ctx).ExecuteWorkflow(ctx, client.StartWorkflowOptions{
 		ID:        fmt.Sprintf("finalize-version-upload-%s-%s-%s", modID, uploadID, mod.ModReference),
 		TaskQueue: workflows.RepoTaskQueue,
-	}, workflows.FinalizeVersionUploadWorkflow, mod.ID, uploadID, version); err != nil {
+	}, workflows.FinalizeVersionUploadWorkflow, mod.ID, uploadID, version, viper.GetString("skip-virus-check")); err != nil {
 		return false, fmt.Errorf("failed to start finalization workflow: %w", err)
 	}
 

diff --git a/tests/version_test.go b/tests/version_test.go
@@ -119,8 +119,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 		})
 	}
 
-	var versionID string
-
+	var uploadID string
 	t.Run("Create Version", func(t *testing.T) {
 		createRequest := authRequest(`mutation CreateVersion($mod_id: ModID!) {
 			createVersion(modId: $mod_id)
@@ -133,7 +132,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 		testza.AssertNoError(t, client.Run(ctx, createRequest, &createResponse))
 		testza.AssertNotEqual(t, "", createResponse.CreateVersion)
 
-		versionID = createResponse.CreateVersion
+		uploadID = createResponse.CreateVersion
 	})
 
 	t.Run("Upload Parts", func(t *testing.T) {
@@ -167,7 +166,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 					}`,
 					"variables": map[string]interface{}{
 						"mod_id":     modID,
-						"version_id": versionID,
+						"version_id": uploadID,
 						"part":       i + 1,
 						"file":       nil,
 					},
@@ -230,7 +229,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 			})
 		}`, token)
 		finalizeRequest.Var("mod_id", modID)
-		finalizeRequest.Var("version_id", versionID)
+		finalizeRequest.Var("version_id", uploadID)
 
 		var finalizeResponse struct {
 			FinalizeCreateVersion bool
@@ -239,6 +238,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 		testza.AssertTrue(t, finalizeResponse.FinalizeCreateVersion)
 	})
 
+	var versionID string
 	t.Run("Wait For Version", func(t *testing.T) {
 		request := authRequest(`query CheckVersionUploadState($mod_id: ModID!, $version_id: VersionID!) {
 			checkVersionUploadState(modId: $mod_id, versionId: $version_id) {
@@ -249,7 +249,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 			}
 		}`, token)
 		request.Var("mod_id", modID)
-		request.Var("version_id", versionID)
+		request.Var("version_id", uploadID)
 
 		end := time.Now().Add(time.Minute * 30)
 		for time.Now().Before(end) {
@@ -288,6 +288,7 @@ func RunVersionTest(ctx context.Context, t *testing.T, client *graphql.Client, m
 			return
 		}
 
+		// TODO wait on workflow instead of poll
 		if executeVirusCheck {
 			for time.Now().Before(end) {
 				getModVersion := authRequest(`query GetModVersion($version_id: VersionID!) {

diff --git a/workflows/versions.go b/workflows/versions.go
@@ -10,7 +10,6 @@ import (
 
 	"entgo.io/ent/dialect/sql"
 	"github.com/Vilsol/slox"
-	"github.com/spf13/viper"
 	"go.temporal.io/sdk/temporal"
 	"go.temporal.io/sdk/workflow"
 
@@ -27,7 +26,7 @@ import (
 	"github.com/satisfactorymodding/smr-api/validation"
 )
 
-func FinalizeVersionUploadWorkflow(ctx workflow.Context, modID string, uploadID string, version generated.NewVersion) error {
+func FinalizeVersionUploadWorkflow(ctx workflow.Context, modID string, uploadID string, version generated.NewVersion, skipVirusCheck bool) error {
 	ctx = workflow.WithActivityOptions(ctx, workflow.ActivityOptions{
 		StartToCloseTimeout: time.Minute * 30,
 		RetryPolicy: &temporal.RetryPolicy{
@@ -37,10 +36,7 @@ func FinalizeVersionUploadWorkflow(ctx workflow.Context, modID string, uploadID
 
 	fatalError := func(ctx workflow.Context, err error, modInfo *validation.ModInfo) error {
 		if err != nil {
-			err2 := workflow.ExecuteActivity(ctx, removeModActivity, modID, modInfo, uploadID).Get(ctx, nil)
-			if err2 != nil {
-				slog.Error("failed to remove mod", slog.Any("err", err2))
-			}
+			_ = workflow.ExecuteActivity(ctx, removeModActivity, modID, modInfo, uploadID).Get(ctx, nil)
 			return workflow.ExecuteActivity(ctx, storeRedisStateActivity, uploadID, nil, err.Error()).Get(ctx, nil)
 		}
 		return err
@@ -58,11 +54,7 @@ func FinalizeVersionUploadWorkflow(ctx workflow.Context, modID string, uploadID
 	}
 
 	var metadata *string
-	err = workflow.ExecuteActivity(ctx, extractMetadataActivity, modID, uploadID, modInfo).Get(ctx, &metadata)
-	if err != nil {
-		// Do not retry extracting metadata again
-		slog.Error("failed to extract metadata", slog.Any("err", err), slog.String("mod_id", modID), slog.String("upload_id", uploadID))
-	}
+	_ = workflow.ExecuteActivity(ctx, extractMetadataActivity, modID, uploadID, modInfo).Get(ctx, &metadata)
 
 	var fileKey *string
 	err = workflow.ExecuteActivity(ctx, renameVersionActivity, modID, uploadID, modInfo.Version).Get(ctx, &fileKey)
@@ -83,7 +75,7 @@ func FinalizeVersionUploadWorkflow(ctx workflow.Context, modID string, uploadID
 	}
 
 	data := &generated.CreateVersionResponse{
-		AutoApproved: shouldAutoApprove(modInfo),
+		AutoApproved: shouldAutoApprove(modInfo, skipVirusCheck),
 		Version:      (*conv.VersionImpl)(nil).Convert(dbVersion),
 	}
 
@@ -190,7 +182,16 @@ func extractModInfoActivity(ctx context.Context, modID string, uploadID string)
 	return modInfo, nil
 }
 
-func extractMetadataActivity(ctx context.Context, modID string, uploadID string, modInfo *validation.ModInfo) (*string, error) {
+func extractMetadataActivity(ctx context.Context, modID string, uploadID string, modInfo *validation.ModInfo) *string {
+	metadata, err := extractMetadata(ctx, modID, uploadID, modInfo)
+	if err != nil {
+		slog.Error("failed to extract metadata", slog.Any("err", err), slog.String("mod_id", modID), slog.String("upload_id", uploadID))
+		return nil
+	}
+	return metadata
+}
+
+func extractMetadata(ctx context.Context, modID string, uploadID string, modInfo *validation.ModInfo) (*string, error) {
 	mod, err := db.From(ctx).Mod.Get(ctx, modID)
 	if err != nil {
 		return nil, err
@@ -396,10 +397,11 @@ func storeRedisStateActivity(ctx context.Context, uploadID string, data *generat
 	return nil
 }
 
-func removeModActivity(ctx context.Context, modID string, modInfo *validation.ModInfo, uploadID string) error {
+func removeModActivity(ctx context.Context, modID string, modInfo *validation.ModInfo, uploadID string) {
 	mod, err := db.From(ctx).Mod.Get(ctx, modID)
 	if err != nil {
-		return err
+		slog.Error("failed to retrieve mod", slog.Any("err", err))
+		return
 	}
 
 	// TODO: cleanup file parts if failure happened before completing multipart upload
@@ -411,7 +413,6 @@ func removeModActivity(ctx context.Context, modID string, modInfo *validation.Mo
 			_ = storage.DeleteModTarget(ctx, mod.ID, mod.Name, modInfo.Version, target)
 		}
 	}
-	return nil
 }
 
 func downloadMod(ctx context.Context, mod *ent.Mod, uploadID string) ([]byte, error) {
@@ -429,8 +430,8 @@ func downloadMod(ctx context.Context, mod *ent.Mod, uploadID string) ([]byte, er
 	return fileData, nil
 }
 
-func shouldAutoApprove(modInfo *validation.ModInfo) bool {
-	if viper.GetBool("skip-virus-check") {
+func shouldAutoApprove(modInfo *validation.ModInfo, skipVirusCheck bool) bool {
+	if skipVirusCheck {
 		return true
 	}