From c74f7f917ef768b6a312290c2cc37f295579b843 Mon Sep 17 00:00:00 2001 From: jwillp Date: Fri, 23 Aug 2024 01:41:30 -0400 Subject: [PATCH] Fix JSON artifact registry having a nil map --- artefactregistry.go | 6 ++++- mkdirartfproc.go | 39 +++++++++++++++++++++++---- mkdirartfproc_test.go | 24 ++++++++++++++--- writefileartfproc.go | 56 ++++++++++++++++++++++++++++++++++----- writefileartfproc_test.go | 16 +++++++++++ 5 files changed, 124 insertions(+), 17 deletions(-) diff --git a/artefactregistry.go b/artefactregistry.go index ac0639a..7b62d6d 100644 --- a/artefactregistry.go +++ b/artefactregistry.go @@ -82,7 +82,7 @@ type JSONArtifactRegistryProcessor struct { // NewJSONArtifactRegistry returns a new artifact file registry. func NewJSONArtifactRegistry(fileName string, fs FileSystem) *JSONArtifactRegistry { return &JSONArtifactRegistry{ - ArtifactMap: nil, + ArtifactMap: map[string]*JSONArtifactRegistryProcessor{}, FilePath: fileName, CurrentTimeProvider: func() time.Time { return time.Now() @@ -138,6 +138,10 @@ func (r *JSONArtifactRegistry) AddArtifact(processorName string, artifactName st r.mu.Lock() defer r.mu.Unlock() + if r.ArtifactMap == nil { + r.ArtifactMap = map[string]*JSONArtifactRegistryProcessor{} + } + if _, ok := r.ArtifactMap[processorName]; !ok { r.ArtifactMap[processorName] = &JSONArtifactRegistryProcessor{} } diff --git a/mkdirartfproc.go b/mkdirartfproc.go index ea7af0a..260bc3a 100644 --- a/mkdirartfproc.go +++ b/mkdirartfproc.go @@ -10,8 +10,9 @@ import ( // DirectoryArtifact is a data structure that can be used by a SpecificationProcessor to directory artifacts // that can be written by the MakeDirectoryArtifactsProcessor. type DirectoryArtifact struct { - Path string - Mode os.FileMode + Path string + FileMode os.FileMode + WriteMode WriteMode } type MakeDirectoryArtifactsProcessor struct { @@ -42,15 +43,43 @@ func (p MakeDirectoryArtifactsProcessor) Process(ctx ArtifactProcessingContext) if !ok { continue } + if dir.WriteMode == "" { + dir.WriteMode = WriteOnceMode + } + wg.Add(1) go func(ctx ArtifactProcessingContext, artifactName string, dir DirectoryArtifact) { defer wg.Done() - ctx.Logger.Info(fmt.Sprintf("Creating directory %q ...", dir.Path)) - - err := p.FileSystem.Mkdir(dir.Path, dir.Mode) + dirPath, err := p.FileSystem.Abs(dir.Path) if err != nil { ctx.Logger.Error(fmt.Sprintf("failed creating directory at %q", dir.Path)) errs = errs.Append(err) + return + } + + dirExists := true + if _, err := p.FileSystem.StatPath(dirPath); err != nil { + if !os.IsNotExist(err) { + ctx.Logger.Error(fmt.Sprintf("failed writing artifact file at %q", dirPath)) + errs = errs.Append(err) + return + } + dirExists = false + } + + if dir.WriteMode == WriteOnceMode && dirExists { + ctx.Logger.Info(fmt.Sprintf("Directory %q already exists ... skipping", dirPath)) + return + } + + ctx.Logger.Info(fmt.Sprintf("Creating directory %q ...", dirPath)) + if err = p.FileSystem.Mkdir(dirPath, dir.FileMode); err != nil { + ctx.Logger.Error(fmt.Sprintf("failed creating directory at %q", dirPath)) + errs = errs.Append(err) + return + } + + if dir.WriteMode != WriteOnceMode { ctx.AddToRegistry(artifactName) } }(ctx, artifact.Name, dir) diff --git a/mkdirartfproc_test.go b/mkdirartfproc_test.go index b0856fd..a66900c 100644 --- a/mkdirartfproc_test.go +++ b/mkdirartfproc_test.go @@ -23,11 +23,11 @@ func TestMakeDirectoryArtifactsProcessor_Process(t *testing.T) { artifacts: []Artifact{ { Name: "dir1", - Value: DirectoryArtifact{Path: "/path/to/dir1", Mode: 0755}, + Value: DirectoryArtifact{Path: "/path/to/dir1", FileMode: 0755}, }, { Name: "dir2", - Value: DirectoryArtifact{Path: "/path/to/dir2", Mode: 0755}, + Value: DirectoryArtifact{Path: "/path/to/dir2", FileMode: 0755}, }, }, expectedDirs: []string{"/path/to/dir1", "/path/to/dir2"}, @@ -41,7 +41,7 @@ func TestMakeDirectoryArtifactsProcessor_Process(t *testing.T) { artifacts: []Artifact{ { Name: "dir1", - Value: DirectoryArtifact{Path: "/path/to/dir1", Mode: 0755}, + Value: DirectoryArtifact{Path: "/path/to/dir1", FileMode: 0755}, }, { Name: "not_a_dir", @@ -60,12 +60,28 @@ func TestMakeDirectoryArtifactsProcessor_Process(t *testing.T) { artifacts: []Artifact{ { Name: "dir1", - Value: DirectoryArtifact{Path: "/path/to/dir1", Mode: 0755}, + Value: DirectoryArtifact{Path: "/path/to/dir1", FileMode: 0755}, }, }, expectedDirs: []string{}, expectError: assert.AnError, }, + { + name: "GIVEN file already exists WHEN write mode is Once THEN do not write file", + mockFS: &mockFileSystem{ + dirs: map[string]bool{ + "/dir": true, + }, + }, + artifacts: []Artifact{ + { + Name: "file1", + Value: DirectoryArtifact{Path: "/dir", WriteMode: WriteOnceMode}, + }, + }, + expectedDirs: []string{}, + expectError: nil, + }, } for _, tt := range tests { diff --git a/writefileartfproc.go b/writefileartfproc.go index 64bee66..1f8ce2c 100644 --- a/writefileartfproc.go +++ b/writefileartfproc.go @@ -9,12 +9,20 @@ import ( const WriteFileArtifactsProcessorErrorCode = "write_file_artifacts_processor_error" +type WriteMode string + +const ( + RecreateMode WriteMode = "recreate" + WriteOnceMode WriteMode = "once" +) + // FileArtifact is a data structure that can be used by a SpecificationProcessor to generate file artifacts // that can be written by the WriteFileArtifactProcessor. type FileArtifact struct { - Path string - Data []byte - Mode os.FileMode + Path string + Data []byte + Mode os.FileMode + WriteMode WriteMode } // WriteFileArtifactProcessor is a processor responsible for writing Artifact referring to files. @@ -52,16 +60,50 @@ func (p WriteFileArtifactProcessor) Process(ctx ArtifactProcessingContext) error if err := CheckContextDone(ctx); err != nil { return err } + if file.WriteMode == "" { + file.WriteMode = WriteOnceMode + } + wg.Add(1) go func(ctx ArtifactProcessingContext, file FileArtifact) { defer wg.Done() - ctx.Logger.Info(fmt.Sprintf("Writing file %q ...", file.Path)) - err := p.FileSystem.WriteFile(file.Path, file.Data, os.ModePerm) + + filePath, err := p.FileSystem.Abs(file.Path) if err != nil { - ctx.Logger.Error(fmt.Sprintf("failed writing artifact file at %q", file.Path)) + ctx.Logger.Error(fmt.Sprintf("failed writing artifact file at %q", filePath)) errs = errs.Append(err) + return + } + + fileExists := true + if _, err := p.FileSystem.StatPath(filePath); err != nil { + if !os.IsNotExist(err) { + ctx.Logger.Error(fmt.Sprintf("failed writing artifact file at %q", filePath)) + errs = errs.Append(err) + return + } + fileExists = false } - ctx.AddToRegistry(file.Path) + + if file.WriteMode == WriteOnceMode && fileExists { + ctx.Logger.Info(fmt.Sprintf("File %q already exists ... skipping", filePath)) + return + } + + // At this point if the file still already exists, this means that the clean step has not + // been executed properly. + + ctx.Logger.Info(fmt.Sprintf("Writing file %q ...", filePath)) + if err := p.FileSystem.WriteFile(filePath, file.Data, os.ModePerm); err != nil { + ctx.Logger.Error(fmt.Sprintf("failed writing artifact file at %q", filePath)) + errs = errs.Append(err) + return + } + + if file.WriteMode != WriteOnceMode { + ctx.AddToRegistry(file.Path) + } + }(ctx, file) } wg.Wait() diff --git a/writefileartfproc_test.go b/writefileartfproc_test.go index 5d2f551..6ba18c1 100644 --- a/writefileartfproc_test.go +++ b/writefileartfproc_test.go @@ -60,6 +60,22 @@ func TestWriteFileArtifactProcessor_Process(t *testing.T) { expectedFiles: []string{}, expectError: assert.AnError, }, + { + name: "GIVEN file already exists WHEN write mode is Once THEN do not write file", + mockFS: &mockFileSystem{ + files: map[string][]byte{ + "/path/to/file1": []byte("file content"), + }, + }, + artifacts: []Artifact{ + { + Name: "file1", + Value: FileArtifact{Path: "/path/to/file1", Mode: 0755, WriteMode: WriteOnceMode}, + }, + }, + expectedFiles: []string{}, + expectError: nil, + }, } for _, tt := range tests {