From c26443d7fd45090ea9803365bffd735ce202508a Mon Sep 17 00:00:00 2001 From: kat <28567881+hk21702@users.noreply.github.com> Date: Thu, 30 Jan 2025 13:38:51 -0500 Subject: [PATCH] Implement Report processed marking --- internal/fileutils/fileutils_test.go | 30 +- internal/report/report.go | 65 +++- internal/report/report_test.go | 293 +++++++++++++++++- .../TestMarkAsProcessed/golden/basic_move | 7 + .../golden/basic_move_new_data | 7 + .../golden/basic_move_overwrite | 7 + .../TestMarkAsProcessed/golden/dstperm_none | 7 + .../TestMarkAsProcessed/golden/srcperm_none | 7 + .../testdata/TestReadJSON/golden/basic_read | 1 + .../TestReadJSON/golden/multiple_files | 1 + .../TestUndoProcessed/golden/basic_move | 7 + .../golden/basic_move_new_data | 7 + .../golden/basic_move_overwrite | 7 + internal/uploader/internal_test.go | 52 ---- internal/uploader/upload.go | 28 +- 15 files changed, 419 insertions(+), 107 deletions(-) create mode 100644 internal/report/testdata/TestMarkAsProcessed/golden/basic_move create mode 100644 internal/report/testdata/TestMarkAsProcessed/golden/basic_move_new_data create mode 100644 internal/report/testdata/TestMarkAsProcessed/golden/basic_move_overwrite create mode 100644 internal/report/testdata/TestMarkAsProcessed/golden/dstperm_none create mode 100644 internal/report/testdata/TestMarkAsProcessed/golden/srcperm_none create mode 100644 internal/report/testdata/TestReadJSON/golden/basic_read create mode 100644 internal/report/testdata/TestReadJSON/golden/multiple_files create mode 100644 internal/report/testdata/TestUndoProcessed/golden/basic_move create mode 100644 internal/report/testdata/TestUndoProcessed/golden/basic_move_new_data create mode 100644 internal/report/testdata/TestUndoProcessed/golden/basic_move_overwrite diff --git a/internal/fileutils/fileutils_test.go b/internal/fileutils/fileutils_test.go index 15c2b96..f635843 100644 --- a/internal/fileutils/fileutils_test.go +++ b/internal/fileutils/fileutils_test.go @@ -3,6 +3,7 @@ package fileutils_test import ( "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/require" @@ -13,21 +14,25 @@ func TestAtomicWrite(t *testing.T) { t.Parallel() tests := map[string]struct { - data []byte - fileExists bool - invalidDir bool + data []byte + fileExists bool + fileExistsPerms os.FileMode + invalidDir bool - wantError bool + wantErrWin bool + wantError bool }{ "Empty file": {data: []byte{}}, "Non-empty file": {data: []byte("data")}, - "Override file": {data: []byte("data"), fileExists: true}, - "Override empty file": {data: []byte{}, fileExists: true}, + "Override file": {data: []byte("data"), fileExistsPerms: 0600, fileExists: true}, + "Override empty file": {data: []byte{}, fileExistsPerms: 0600, fileExists: true}, - "Existing empty file": {data: []byte{}, fileExists: true}, - "Existing non-empty file": {data: []byte("data"), fileExists: true}, + "Existing empty file": {data: []byte{}, fileExistsPerms: 0600, fileExists: true}, + "Existing non-empty file": {data: []byte("data"), fileExistsPerms: 0600, fileExists: true}, - "Invalid Dir": {data: []byte("data"), invalidDir: true, wantError: true}, + "Override read-only file": {data: []byte("data"), fileExistsPerms: 0400, fileExists: true, wantErrWin: true}, + "Override No Perms file": {data: []byte("data"), fileExistsPerms: 0000, fileExists: true, wantErrWin: true}, + "Invalid Dir": {data: []byte("data"), invalidDir: true, wantError: true}, } for name, tc := range tests { @@ -42,12 +47,13 @@ func TestAtomicWrite(t *testing.T) { } if tc.fileExists { - err := fileutils.AtomicWrite(path, oldFile) - require.NoError(t, err, "Setup: AtomicWrite should not return an error") + err := os.WriteFile(path, oldFile, tc.fileExistsPerms) + require.NoError(t, err, "Setup: WriteFile should not return an error") + t.Cleanup(func() { _ = os.Chmod(path, 0600) }) } err := fileutils.AtomicWrite(path, tc.data) - if tc.wantError { + if tc.wantError || (tc.wantErrWin && runtime.GOOS == "windows") { require.Error(t, err, "AtomicWrite should return an error") // Check that the file was not overwritten diff --git a/internal/report/report.go b/internal/report/report.go index eb22687..d866fad 100644 --- a/internal/report/report.go +++ b/internal/report/report.go @@ -15,6 +15,7 @@ import ( "time" "github.com/ubuntu/ubuntu-insights/internal/constants" + "github.com/ubuntu/ubuntu-insights/internal/fileutils" ) var ( @@ -30,7 +31,7 @@ var ( // Report represents a report file. type Report struct { - Path string // Path is the path to the report. + Path string // Path is the path to the report file. Name string // Name is the name of the report file, including extension. TimeStamp int64 // TimeStamp is the timestamp of the report. @@ -73,6 +74,50 @@ func (r Report) ReadJSON() ([]byte, error) { return data, nil } +// MarkAsProcessed moves the report to a destination directory, and writes the data to the report. +// The original report is removed. +// +// The new report is returned, and the original data is stashed for use with UndoProcessed. +// Note that calling MarkAsProcessed multiple times on the same report will overwrite the stashed data. +func (r Report) MarkAsProcessed(dest string, data []byte) (Report, error) { + origData, err := r.ReadJSON() + if err != nil { + return Report{}, fmt.Errorf("failed to read original report: %v", err) + } + + newReport := Report{Path: filepath.Join(dest, r.Name), Name: r.Name, TimeStamp: r.TimeStamp, + reportStash: reportStash{Path: r.Path, Data: origData}} + + if err := fileutils.AtomicWrite(newReport.Path, data); err != nil { + return Report{}, fmt.Errorf("failed to write report: %v", err) + } + + if err := os.Remove(r.Path); err != nil { + return Report{}, fmt.Errorf("failed to remove report: %v", err) + } + + return newReport, nil +} + +// UndoProcessed moves the report back to the original directory, and writes the original data to the report. +// The new report is returned, and the original data is removed. +func (r Report) UndoProcessed() (Report, error) { + if r.reportStash.Path == "" { + return Report{}, errors.New("no stashed data to restore") + } + + if err := fileutils.AtomicWrite(r.reportStash.Path, r.reportStash.Data); err != nil { + return Report{}, fmt.Errorf("failed to write report: %v", err) + } + + if err := os.Remove(r.Path); err != nil { + return Report{}, fmt.Errorf("failed to remove report: %v", err) + } + + newReport := Report{Path: r.reportStash.Path, Name: r.Name, TimeStamp: r.TimeStamp} + return newReport, nil +} + // getReportTime returns a int64 representation of the report time from the report path. func getReportTime(path string) (int64, error) { fileName := filepath.Base(path) @@ -108,8 +153,7 @@ func GetForPeriod(dir string, time time.Time, period int) (Report, error) { var report Report err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { if err != nil { - slog.Error("Failed to access path", "path", path, "error", err) - return err + return fmt.Errorf("failed to access path: %v", err) } // Skip subdirectories. @@ -122,8 +166,7 @@ func GetForPeriod(dir string, time time.Time, period int) (Report, error) { slog.Info("Skipping non-report file", "file", d.Name(), "error", err) return nil } else if err != nil { - slog.Error("Failed to create report object", "error", err) - return err + return fmt.Errorf("failed to create report object: %v", err) } if r.TimeStamp < periodStart { @@ -156,8 +199,7 @@ func GetPerPeriod(dir string, period int) (map[int64]Report, error) { reports := make(map[int64]Report) err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { if err != nil { - slog.Error("Failed to access path", "path", path, "error", err) - return err + return fmt.Errorf("failed to access path: %v", err) } if d.IsDir() && path != dir { @@ -169,8 +211,7 @@ func GetPerPeriod(dir string, period int) (map[int64]Report, error) { slog.Info("Skipping non-report file", "file", d.Name(), "error", err) return nil } else if err != nil { - slog.Error("Failed to create report object", "error", err) - return err + return fmt.Errorf("failed to create report object: %v", err) } periodStart := r.TimeStamp - (r.TimeStamp % int64(period)) @@ -195,8 +236,7 @@ func GetAll(dir string) ([]Report, error) { reports := make([]Report, 0) err := filepath.WalkDir(dir, func(path string, d os.DirEntry, err error) error { if err != nil { - slog.Error("Failed to access path", "path", path, "error", err) - return err + return fmt.Errorf("failed to access path: %v", err) } if d.IsDir() && path != dir { @@ -208,8 +248,7 @@ func GetAll(dir string) ([]Report, error) { slog.Info("Skipping non-report file", "file", d.Name(), "error", err) return nil } else if err != nil { - slog.Error("Failed to create report object", "error", err) - return err + return fmt.Errorf("failed to create report object: %v", err) } reports = append(reports, r) diff --git a/internal/report/report_test.go b/internal/report/report_test.go index 96b41be..984e0d1 100644 --- a/internal/report/report_test.go +++ b/internal/report/report_test.go @@ -3,11 +3,12 @@ package report_test import ( "os" "path/filepath" + "runtime" "testing" "time" "github.com/stretchr/testify/require" - report "github.com/ubuntu/ubuntu-insights/internal/report" + "github.com/ubuntu/ubuntu-insights/internal/report" "github.com/ubuntu/ubuntu-insights/internal/testutils" ) @@ -109,7 +110,7 @@ func TestGetForPeriod(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles) + dir, err := setupNoDataDir(t, tc.files, tc.subDir, tc.subDirFiles) require.NoError(t, err, "Setup: failed to setup temporary directory") if tc.invalidDir { dir = filepath.Join(dir, "invalid dir") @@ -169,7 +170,7 @@ func TestGetPerPeriod(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles) + dir, err := setupNoDataDir(t, tc.files, tc.subDir, tc.subDirFiles) require.NoError(t, err, "Setup: failed to setup temporary directory") if tc.invalidDir { dir = filepath.Join(dir, "invalid dir") @@ -219,7 +220,7 @@ func TestGetAll(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { t.Parallel() - dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles) + dir, err := setupNoDataDir(t, tc.files, tc.subDir, tc.subDirFiles) require.NoError(t, err, "Setup: failed to setup temporary directory") if tc.invalidDir { dir = filepath.Join(dir, "invalid dir") @@ -242,11 +243,289 @@ func TestGetAll(t *testing.T) { } } -func setupTmpDir(t *testing.T, files []string, subDir string, subDirFiles []string) (string, error) { +func TestMarkAsProcessed(t *testing.T) { + t.Parallel() + + type got struct { + Report report.Report + SrcFiles map[string]string + DstFiles map[string]string + } + + tests := map[string]struct { + srcFile map[string]string + dstFile map[string]string + + fileName string + data []byte + srcFilePerms os.FileMode + dstFilePerms os.FileMode + + wantErrUnix bool + wantErrWin bool + wantErr bool + }{ + "Basic Move": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{}, + fileName: "1.json", + data: []byte(`{"test": true}`), + srcFilePerms: os.FileMode(0o600), + dstFilePerms: os.FileMode(0o600), + wantErr: false, + }, + "Basic Move New Data": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{}, + fileName: "1.json", + data: []byte("new data"), + srcFilePerms: os.FileMode(0o600), + dstFilePerms: os.FileMode(0o600), + wantErr: false, + }, + "Basic Move Overwrite": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{"1.json": "old data"}, + fileName: "1.json", + data: []byte("new data"), + srcFilePerms: os.FileMode(0o600), + dstFilePerms: os.FileMode(0o600), + wantErr: false, + }, "SrcPerm None": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{}, + fileName: "1.json", + data: []byte(`{"test": true}`), + srcFilePerms: os.FileMode(000), + dstFilePerms: os.FileMode(0o600), + wantErrUnix: true, + }, "DstPerm None": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{"1.json": "old data"}, + fileName: "1.json", + data: []byte("new data"), + srcFilePerms: os.FileMode(0o600), + dstFilePerms: os.FileMode(000), + wantErrWin: true, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + rootDir, srcDir, dstDir := setupProcessingDirs(t) + + setupBasicDir(t, tc.srcFile, tc.srcFilePerms, srcDir) + setupBasicDir(t, tc.dstFile, tc.dstFilePerms, dstDir) + + r, err := report.New(filepath.Join(srcDir, tc.fileName)) + require.NoError(t, err, "Setup: failed to create report object") + + r, err = r.MarkAsProcessed(dstDir, tc.data) + if (tc.wantErr) || (tc.wantErrUnix && runtime.GOOS != "windows") || (tc.wantErrWin && runtime.GOOS == "windows") { + require.Error(t, err, "expected an error but got none") + return + } + require.NoError(t, err, "got an unexpected error") + + dstDirContents, err := testutils.GetDirContents(t, dstDir, 2) + require.NoError(t, err, "failed to get directory contents") + + srcDirContents, err := testutils.GetDirContents(t, srcDir, 2) + require.NoError(t, err, "failed to get directory contents") + + r = sanitizeReportPath(t, r, rootDir) + got := got{Report: r, SrcFiles: srcDirContents, DstFiles: dstDirContents} + want := testutils.LoadWithUpdateFromGoldenYAML(t, got) + require.EqualExportedValues(t, want, got, "MarkAsProcessed should move the report to the processed directory") + }) + } +} + +func TestUndoProcessed(t *testing.T) { + t.Parallel() + + type got struct { + Report report.Report + SrcFiles map[string]string + DstFiles map[string]string + } + + tests := map[string]struct { + srcFile map[string]string + dstFile map[string]string + + fileName string + data []byte + + wantErr bool + }{ + "Basic Move": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{}, + fileName: "1.json", + data: []byte(`"new data"`), + wantErr: false, + }, + "Basic Move New Data": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{}, + fileName: "1.json", + data: []byte("new data"), + wantErr: false, + }, + "Basic Move Overwrite": { + srcFile: map[string]string{"1.json": `{"test": true}`}, + dstFile: map[string]string{"1.json": "old data"}, + fileName: "1.json", + data: []byte("new data"), + wantErr: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + rootDir, srcDir, dstDir := setupProcessingDirs(t) + + setupBasicDir(t, tc.srcFile, 0600, srcDir) + setupBasicDir(t, tc.dstFile, 0600, dstDir) + + r, err := report.New(filepath.Join(srcDir, tc.fileName)) + require.NoError(t, err, "Setup: failed to create report object") + + r, err = r.MarkAsProcessed(dstDir, tc.data) + require.NoError(t, err, "Setup: failed to mark report as processed") + + r, err = r.UndoProcessed() + if tc.wantErr { + require.Error(t, err, "expected an error but got none") + return + } + require.NoError(t, err, "got an unexpected error") + + dstDirContents, err := testutils.GetDirContents(t, dstDir, 2) + require.NoError(t, err, "failed to get directory contents") + + srcDirContents, err := testutils.GetDirContents(t, srcDir, 2) + require.NoError(t, err, "failed to get directory contents") + + r = sanitizeReportPath(t, r, rootDir) + got := got{Report: r, SrcFiles: srcDirContents, DstFiles: dstDirContents} + want := testutils.LoadWithUpdateFromGoldenYAML(t, got) + require.EqualExportedValues(t, want, got, "UndoProcessed should move the report to the processed directory") + }) + } +} + +func TestUndoProcessedNoStash(t *testing.T) { + t.Parallel() + + r, err := report.New("1.json") + require.NoError(t, err, "Setup: failed to create report object") + + _, err = r.UndoProcessed() + require.Error(t, err, "UndoProcessed should return an error if the report has not been marked as processed") +} + +func TestMarkAsProcessedNoFile(t *testing.T) { + t.Parallel() + + _, srcDir, dstDir := setupProcessingDirs(t) + r, err := report.New(filepath.Join(srcDir, "1.json")) + require.NoError(t, err, "Setup: failed to create report object") + + _, err = r.MarkAsProcessed(dstDir, []byte(`"new data"`)) + require.Error(t, err, "MarkAsProcessed should return an error if the report file does not exist") +} + +func TestUndoProcessedNoFile(t *testing.T) { + t.Parallel() + + _, srcDir, dstDir := setupProcessingDirs(t) + reportPath := filepath.Join(srcDir, "1.json") + require.NoError(t, os.WriteFile(reportPath, []byte(`{"test": true}`), 0600), "Setup: failed to write report file") + r, err := report.New(reportPath) + require.NoError(t, err, "Setup: failed to create report object") + + r, err = r.MarkAsProcessed(dstDir, []byte(`"new data"`)) + require.NoError(t, err, "Setup: failed to mark report as processed") + + require.NoError(t, os.Remove(r.Path), "Setup: failed to remove report file") + + _, err = r.UndoProcessed() + require.Error(t, err, "UndoProcessed should return an error if the report file does not exist") +} + +func TestReadJSON(t *testing.T) { + t.Parallel() + + tests := map[string]struct { + files map[string]string + file string + + wantErr bool + }{ + "Basic Read": {files: map[string]string{"1.json": `{"test": true}`}, file: "1.json"}, + "Multiple Files": {files: map[string]string{"1.json": `{"test": true}`, "2.json": `{"test": false}`}, file: "1.json"}, + + "Empty File": {files: map[string]string{"1.json": ""}, file: "1.json", wantErr: true}, + "Invalid JSON": {files: map[string]string{"1.json": `{"test":::`}, file: "1.json", wantErr: true}, + "No File": {files: map[string]string{}, file: "1.json", wantErr: true}, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + t.Parallel() + _, srcDir, _ := setupProcessingDirs(t) + + setupBasicDir(t, tc.files, 0600, srcDir) + + r, err := report.New(filepath.Join(srcDir, tc.file)) + require.NoError(t, err, "Setup: failed to create report object") + + data, err := r.ReadJSON() + if tc.wantErr { + require.Error(t, err, "expected an error but got none") + return + } + require.NoError(t, err, "got an unexpected error") + + got := string(data) + want := testutils.LoadWithUpdateFromGolden(t, got) + require.Equal(t, want, got, "ReadJSON should return the data from the report file") + }) + } +} + +func setupProcessingDirs(t *testing.T) (rootDir, srcDir, dstDir string) { t.Helper() + rootDir = t.TempDir() + srcDir = filepath.Join(rootDir, "src") + dstDir = filepath.Join(rootDir, "dst") + err := os.MkdirAll(srcDir, 0700) + require.NoError(t, err, "Setup: failed to create source directory") + err = os.MkdirAll(dstDir, 0700) + require.NoError(t, err, "Setup: failed to create destination directory") + return rootDir, srcDir, dstDir +} - dir := t.TempDir() +func setupBasicDir(t *testing.T, files map[string]string, perms os.FileMode, dir string) { + t.Helper() + for file, data := range files { + path := filepath.Join(dir, file) + err := os.WriteFile(path, []byte(data), perms) + require.NoError(t, err, "Setup: failed to write file") + t.Cleanup(func() { + _ = os.Chmod(path, os.FileMode(0600)) + }) + } +} +func setupNoDataDir(t *testing.T, files []string, subDir string, subDirFiles []string) (string, error) { + t.Helper() + + dir := t.TempDir() for _, file := range files { path := filepath.Join(dir, file) if err := os.WriteFile(path, []byte{}, 0600); err != nil { @@ -282,5 +561,5 @@ func sanitizeReportPath(t *testing.T, r report.Report, dir string) report.Report require.NoError(t, err, "failed to get relative path") return report.Report{} } - return report.Report{Path: fp, Name: r.Name, TimeStamp: r.TimeStamp} + return report.Report{Path: filepath.ToSlash(fp), Name: r.Name, TimeStamp: r.TimeStamp} } diff --git a/internal/report/testdata/TestMarkAsProcessed/golden/basic_move b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move new file mode 100644 index 0000000..fa8bd51 --- /dev/null +++ b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move @@ -0,0 +1,7 @@ +report: + path: dst/1.json + name: 1.json + timestamp: 1 +srcfiles: {} +dstfiles: + 1.json: '{"test": true}' diff --git a/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_new_data b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_new_data new file mode 100644 index 0000000..0948d76 --- /dev/null +++ b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_new_data @@ -0,0 +1,7 @@ +report: + path: dst/1.json + name: 1.json + timestamp: 1 +srcfiles: {} +dstfiles: + 1.json: new data diff --git a/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_overwrite b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_overwrite new file mode 100644 index 0000000..0948d76 --- /dev/null +++ b/internal/report/testdata/TestMarkAsProcessed/golden/basic_move_overwrite @@ -0,0 +1,7 @@ +report: + path: dst/1.json + name: 1.json + timestamp: 1 +srcfiles: {} +dstfiles: + 1.json: new data diff --git a/internal/report/testdata/TestMarkAsProcessed/golden/dstperm_none b/internal/report/testdata/TestMarkAsProcessed/golden/dstperm_none new file mode 100644 index 0000000..0948d76 --- /dev/null +++ b/internal/report/testdata/TestMarkAsProcessed/golden/dstperm_none @@ -0,0 +1,7 @@ +report: + path: dst/1.json + name: 1.json + timestamp: 1 +srcfiles: {} +dstfiles: + 1.json: new data diff --git a/internal/report/testdata/TestMarkAsProcessed/golden/srcperm_none b/internal/report/testdata/TestMarkAsProcessed/golden/srcperm_none new file mode 100644 index 0000000..fa8bd51 --- /dev/null +++ b/internal/report/testdata/TestMarkAsProcessed/golden/srcperm_none @@ -0,0 +1,7 @@ +report: + path: dst/1.json + name: 1.json + timestamp: 1 +srcfiles: {} +dstfiles: + 1.json: '{"test": true}' diff --git a/internal/report/testdata/TestReadJSON/golden/basic_read b/internal/report/testdata/TestReadJSON/golden/basic_read new file mode 100644 index 0000000..fb25aa1 --- /dev/null +++ b/internal/report/testdata/TestReadJSON/golden/basic_read @@ -0,0 +1 @@ +{"test": true} \ No newline at end of file diff --git a/internal/report/testdata/TestReadJSON/golden/multiple_files b/internal/report/testdata/TestReadJSON/golden/multiple_files new file mode 100644 index 0000000..fb25aa1 --- /dev/null +++ b/internal/report/testdata/TestReadJSON/golden/multiple_files @@ -0,0 +1 @@ +{"test": true} \ No newline at end of file diff --git a/internal/report/testdata/TestUndoProcessed/golden/basic_move b/internal/report/testdata/TestUndoProcessed/golden/basic_move new file mode 100644 index 0000000..3330bd0 --- /dev/null +++ b/internal/report/testdata/TestUndoProcessed/golden/basic_move @@ -0,0 +1,7 @@ +report: + path: src/1.json + name: 1.json + timestamp: 1 +srcfiles: + 1.json: '{"test": true}' +dstfiles: {} diff --git a/internal/report/testdata/TestUndoProcessed/golden/basic_move_new_data b/internal/report/testdata/TestUndoProcessed/golden/basic_move_new_data new file mode 100644 index 0000000..3330bd0 --- /dev/null +++ b/internal/report/testdata/TestUndoProcessed/golden/basic_move_new_data @@ -0,0 +1,7 @@ +report: + path: src/1.json + name: 1.json + timestamp: 1 +srcfiles: + 1.json: '{"test": true}' +dstfiles: {} diff --git a/internal/report/testdata/TestUndoProcessed/golden/basic_move_overwrite b/internal/report/testdata/TestUndoProcessed/golden/basic_move_overwrite new file mode 100644 index 0000000..3330bd0 --- /dev/null +++ b/internal/report/testdata/TestUndoProcessed/golden/basic_move_overwrite @@ -0,0 +1,7 @@ +report: + path: src/1.json + name: 1.json + timestamp: 1 +srcfiles: + 1.json: '{"test": true}' +dstfiles: {} diff --git a/internal/uploader/internal_test.go b/internal/uploader/internal_test.go index 27be531..6ececef 100644 --- a/internal/uploader/internal_test.go +++ b/internal/uploader/internal_test.go @@ -95,58 +95,6 @@ func TestUploadBadFile(t *testing.T) { } } -func TestMoveReport(t *testing.T) { - t.Parallel() - - tests := map[string]struct { - fileExists bool - - wantErr bool - }{ - "File Exists": {fileExists: true}, - - "File Not Found": {fileExists: false, wantErr: true}, - } - - for name, tc := range tests { - t.Run(name, func(t *testing.T) { - t.Parallel() - dir := t.TempDir() - - um := &Uploader{ - collectedDir: filepath.Join(dir, constants.LocalFolder), - uploadedDir: filepath.Join(dir, constants.UploadedFolder), - } - - require.NoError(t, os.MkdirAll(um.collectedDir, 0750), "Setup: failed to create uploaded folder") - require.NoError(t, os.MkdirAll(um.uploadedDir, 0750), "Setup: failed to create collected folder") - - cDir := filepath.Join(um.collectedDir, "report.json") - uDir := filepath.Join(um.uploadedDir, "report.json") - if tc.fileExists { - f, err := os.Create(cDir) - require.NoError(t, err) - f.Close() - } - - err := um.moveReport(uDir, cDir, []byte("payload")) - if tc.wantErr { - require.Error(t, err) - return - } - require.NoError(t, err) - - _, err = os.Stat(uDir) - if !tc.fileExists { - require.Error(t, err, "File should not exist in the uploaded directory") - return - } - - require.NoError(t, err, "File should exist in the uploaded directory") - }) - } -} - func TestSend(t *testing.T) { t.Parallel() diff --git a/internal/uploader/upload.go b/internal/uploader/upload.go index cb04d46..04095e1 100644 --- a/internal/uploader/upload.go +++ b/internal/uploader/upload.go @@ -15,7 +15,6 @@ import ( "time" "github.com/ubuntu/ubuntu-insights/internal/constants" - "github.com/ubuntu/ubuntu-insights/internal/fileutils" "github.com/ubuntu/ubuntu-insights/internal/report" ) @@ -101,13 +100,13 @@ func (um Uploader) upload(r report.Report, url string, consent, force bool) erro } // Move report first to avoid the situation where the report is sent, but not marked as sent. - // TODO: maybe a method on Reports ? - if err := um.moveReport(filepath.Join(um.uploadedDir, r.Name), filepath.Join(um.collectedDir, r.Name), data); err != nil { - return fmt.Errorf("failed to move report after uploading: %v", err) + r, err = r.MarkAsProcessed(um.uploadedDir, data) + if err != nil { + return fmt.Errorf("failed to mark report as processed: %v", err) } if err := send(url, data); err != nil { - if moveErr := um.moveReport(filepath.Join(um.collectedDir, r.Name), filepath.Join(um.uploadedDir, r.Name), origData); moveErr != nil { - return fmt.Errorf("failed to send data: %v, and failed to restore the original report: %v", err, moveErr) + if _, err := r.UndoProcessed(); err != nil { + return fmt.Errorf("failed to send data: %v, and failed to restore the original report: %v", err, err) } return fmt.Errorf("failed to send data: %v", err) } @@ -124,23 +123,6 @@ func (um Uploader) getURL() (string, error) { return u.String(), nil } -func (um Uploader) moveReport(writePath, removePath string, data []byte) error { - // (Report).MarkAsProcessed(data) - // (Report).UndoProcessed - // moveReport writes the data to the writePath, and removes the matching file from the removePath. - // dest, src, data - - if err := fileutils.AtomicWrite(writePath, data); err != nil { - return fmt.Errorf("failed to write report: %v", err) - } - - if err := os.Remove(removePath); err != nil { - return fmt.Errorf("failed to remove report: %v", err) - } - - return nil -} - func send(url string, data []byte) error { slog.Debug("Sending data to server", "url", url, "data", data) req, err := http.NewRequest(http.MethodPost, url, bytes.NewBuffer(data))