From d54fcc483677ceca6bea757a9d994fd328074717 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Raddaoui=20Mar=C3=ADn?= Date: Tue, 27 Aug 2024 21:12:51 +0200 Subject: [PATCH] Use temporal-activities/archivezip [skip-codecov] --- cmd/enduro-am-worker/main.go | 7 +- internal/workflow/activities/zip.go | 98 ------------------------ internal/workflow/activities/zip_test.go | 93 ---------------------- internal/workflow/processing.go | 7 +- internal/workflow/processing_test.go | 11 +-- 5 files changed, 13 insertions(+), 203 deletions(-) delete mode 100644 internal/workflow/activities/zip.go delete mode 100644 internal/workflow/activities/zip_test.go diff --git a/cmd/enduro-am-worker/main.go b/cmd/enduro-am-worker/main.go index ef67798eb..6524d0cd5 100644 --- a/cmd/enduro-am-worker/main.go +++ b/cmd/enduro-am-worker/main.go @@ -15,6 +15,7 @@ import ( "entgo.io/ent/dialect/sql" bagit_gython "github.com/artefactual-labs/bagit-gython" "github.com/artefactual-sdps/temporal-activities/archiveextract" + "github.com/artefactual-sdps/temporal-activities/archivezip" "github.com/artefactual-sdps/temporal-activities/bagcreate" "github.com/artefactual-sdps/temporal-activities/bagvalidate" "github.com/artefactual-sdps/temporal-activities/removepaths" @@ -251,10 +252,8 @@ func main() { temporalsdk_activity.RegisterOptions{Name: bagcreate.Name}, ) w.RegisterActivityWithOptions( - activities.NewZipActivity( - logger, - ).Execute, - temporalsdk_activity.RegisterOptions{Name: activities.ZipActivityName}, + archivezip.New().Execute, + temporalsdk_activity.RegisterOptions{Name: archivezip.Name}, ) w.RegisterActivityWithOptions( am.NewUploadTransferActivity(logger, sftpClient, cfg.AM.PollInterval).Execute, diff --git a/internal/workflow/activities/zip.go b/internal/workflow/activities/zip.go deleted file mode 100644 index 453e88817..000000000 --- a/internal/workflow/activities/zip.go +++ /dev/null @@ -1,98 +0,0 @@ -package activities - -import ( - "archive/zip" - "context" - "fmt" - "io" - "os" - "path/filepath" - - "github.com/go-logr/logr" -) - -const ZipActivityName = "ZipActivity" - -type ZipActivityParams struct { - SourceDir string - DestPath string -} - -type ZipActivityResult struct { - Path string -} - -type zipActivity struct { - logger logr.Logger -} - -func NewZipActivity(logger logr.Logger) *zipActivity { - return &zipActivity{logger: logger} -} - -// Execute creates a Zip archive at params.DestPath from the contents of -// params.SourceDir. If params.DestPath is not specified then params.SourceDir -// + ".zip" will be used. -func (a *zipActivity) Execute(ctx context.Context, params *ZipActivityParams) (*ZipActivityResult, error) { - a.logger.V(1).Info("Executing ZipActivity", - "SourceDir", params.SourceDir, - "DestPath", params.DestPath, - ) - - if params.SourceDir == "" { - return &ZipActivityResult{}, fmt.Errorf("%s: missing source dir", ZipActivityName) - } - - dest := params.DestPath - if params.DestPath == "" { - dest = params.SourceDir + ".zip" - a.logger.V(1).Info(ZipActivityName+": dest changed", "dest", dest) - } - - w, err := os.Create(dest) // #nosec G304 -- trusted path - if err != nil { - return &ZipActivityResult{}, fmt.Errorf("%s: create: %v", ZipActivityName, err) - } - defer w.Close() - - z := zip.NewWriter(w) - defer z.Close() - - err = filepath.WalkDir(params.SourceDir, func(path string, d os.DirEntry, err error) error { - if err != nil { - return err - } - - if d.IsDir() { - return nil - } - - // Include SourceDir in the zip paths, but not its parent dirs. - p, err := filepath.Rel(filepath.Dir(params.SourceDir), path) - if err != nil { - return err - } - - f, err := z.Create(p) - if err != nil { - return err - } - - r, err := os.Open(path) // #nosec G304 -- trusted path - if err != nil { - return err - } - defer r.Close() - - if _, err := io.Copy(f, r); err != nil { - return err - } - - return nil - }) - if err != nil { - return &ZipActivityResult{}, fmt.Errorf("%s: add files: %v", ZipActivityName, err) - } - - return &ZipActivityResult{Path: dest}, nil -} diff --git a/internal/workflow/activities/zip_test.go b/internal/workflow/activities/zip_test.go deleted file mode 100644 index 20466caa6..000000000 --- a/internal/workflow/activities/zip_test.go +++ /dev/null @@ -1,93 +0,0 @@ -package activities_test - -import ( - "archive/zip" - "fmt" - "testing" - - "github.com/go-logr/logr" - temporalsdk_activity "go.temporal.io/sdk/activity" - temporalsdk_testsuite "go.temporal.io/sdk/testsuite" - "gotest.tools/v3/assert" - tfs "gotest.tools/v3/fs" - - "github.com/artefactual-sdps/enduro/internal/workflow/activities" -) - -func TestZipActivity(t *testing.T) { - t.Parallel() - - transferName := "my_transfer" - contents := tfs.WithDir(transferName, - tfs.WithDir("subdir", - tfs.WithFile("abc.txt", "Testing A-B-C"), - ), - tfs.WithFile("123.txt", "Testing 1-2-3!"), - ) - td := tfs.NewDir(t, "enduro-zip-test", contents) - restrictedDir := tfs.NewDir(t, "enduro-zip-restricted", tfs.WithMode(0o555)) - - type test struct { - name string - params activities.ZipActivityParams - want map[string]int64 - wantErr string - } - for _, tc := range []test{ - { - name: "Zips a directory", - params: activities.ZipActivityParams{SourceDir: td.Join(transferName)}, - want: map[string]int64{ - "my_transfer/123.txt": 14, - "my_transfer/subdir/abc.txt": 13, - }, - }, - { - name: "Errors when SourceDir is missing", - wantErr: "ZipActivity: missing source dir", - }, - { - name: "Errors when dest is not writable", - params: activities.ZipActivityParams{ - SourceDir: td.Join(transferName), - DestPath: restrictedDir.Join(transferName + ".zip"), - }, - wantErr: fmt.Sprintf("ZipActivity: create: open %s: permission denied", restrictedDir.Join(transferName+".zip")), - }, - } { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - - ts := &temporalsdk_testsuite.WorkflowTestSuite{} - env := ts.NewTestActivityEnvironment() - env.RegisterActivityWithOptions( - activities.NewZipActivity(logr.Discard()).Execute, - temporalsdk_activity.RegisterOptions{ - Name: activities.ZipActivityName, - }, - ) - - fut, err := env.ExecuteActivity(activities.ZipActivityName, tc.params) - if tc.wantErr != "" { - assert.ErrorContains(t, err, tc.wantErr) - return - } - assert.NilError(t, err) - - var res activities.ZipActivityResult - _ = fut.Get(&res) - assert.DeepEqual(t, res, activities.ZipActivityResult{Path: td.Join(transferName + ".zip")}) - - // Confirm the zip has the expected contents. - rc, err := zip.OpenReader(td.Join(transferName + ".zip")) - assert.NilError(t, err) - t.Cleanup(func() { rc.Close() }) - - files := make(map[string]int64, len(rc.File)) - for _, f := range rc.File { - files[f.Name] = f.FileInfo().Size() - } - assert.DeepEqual(t, files, tc.want) - }) - } -} diff --git a/internal/workflow/processing.go b/internal/workflow/processing.go index 8ea3b065a..4bc86a4f2 100644 --- a/internal/workflow/processing.go +++ b/internal/workflow/processing.go @@ -15,6 +15,7 @@ import ( "time" "github.com/artefactual-sdps/temporal-activities/archiveextract" + "github.com/artefactual-sdps/temporal-activities/archivezip" "github.com/artefactual-sdps/temporal-activities/bagcreate" "github.com/artefactual-sdps/temporal-activities/bagvalidate" "github.com/artefactual-sdps/temporal-activities/removepaths" @@ -859,11 +860,11 @@ func (w *ProcessingWorkflow) transferAM(ctx temporalsdk_workflow.Context, tinfo // Zip PIP. activityOpts := withActivityOptsForLocalAction(ctx) - var zipResult activities.ZipActivityResult + var zipResult archivezip.Result err = temporalsdk_workflow.ExecuteActivity( activityOpts, - activities.ZipActivityName, - &activities.ZipActivityParams{SourceDir: tinfo.TempPath}, + archivezip.Name, + &archivezip.Params{SourceDir: tinfo.TempPath}, ).Get(activityOpts, &zipResult) if err != nil { return err diff --git a/internal/workflow/processing_test.go b/internal/workflow/processing_test.go index 7555f260f..53753ec88 100644 --- a/internal/workflow/processing_test.go +++ b/internal/workflow/processing_test.go @@ -8,6 +8,7 @@ import ( "time" "github.com/artefactual-sdps/temporal-activities/archiveextract" + "github.com/artefactual-sdps/temporal-activities/archivezip" "github.com/artefactual-sdps/temporal-activities/bagcreate" "github.com/artefactual-sdps/temporal-activities/bagvalidate" "github.com/artefactual-sdps/temporal-activities/removepaths" @@ -159,8 +160,8 @@ func (s *ProcessingWorkflowTestSuite) setupAMWorkflowTest( temporalsdk_activity.RegisterOptions{Name: bagcreate.Name}, ) s.env.RegisterActivityWithOptions( - activities.NewZipActivity(logger).Execute, - temporalsdk_activity.RegisterOptions{Name: activities.ZipActivityName}, + archivezip.New().Execute, + temporalsdk_activity.RegisterOptions{Name: archivezip.Name}, ) s.env.RegisterActivityWithOptions( am.NewUploadTransferActivity(logger, sftpc, 10*time.Second).Execute, @@ -614,10 +615,10 @@ func (s *ProcessingWorkflowTestSuite) TestAMWorkflow() { &bagcreate.Result{BagPath: extractPath}, nil, ) - s.env.OnActivity(activities.ZipActivityName, sessionCtx, - &activities.ZipActivityParams{SourceDir: extractPath}, + s.env.OnActivity(archivezip.Name, sessionCtx, + &archivezip.Params{SourceDir: extractPath}, ).Return( - &activities.ZipActivityResult{Path: extractPath + "/transfer.zip"}, nil, + &archivezip.Result{Path: extractPath + "/transfer.zip"}, nil, ) s.env.OnActivity(am.UploadTransferActivityName, sessionCtx,