From 0302a2352b4efaf283513d7bfae1cedcaea8d1f3 Mon Sep 17 00:00:00 2001
From: kat <28567881+hk21702@users.noreply.github.com>
Date: Wed, 29 Jan 2025 11:30:20 -0500
Subject: [PATCH 1/4] bump Go version to 1.23.5 in go.mod and tools/go.mod

---
 go.mod       | 2 +-
 tools/go.mod | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/go.mod b/go.mod
index fba82ff..3f2828c 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module github.com/ubuntu/ubuntu-insights
 
-go 1.23.4
+go 1.23.5
 
 require (
 	github.com/BurntSushi/toml v1.4.0
diff --git a/tools/go.mod b/tools/go.mod
index aff027b..54450c4 100644
--- a/tools/go.mod
+++ b/tools/go.mod
@@ -1,6 +1,6 @@
 module github.com/ubuntu/ubuntu-insights/tools
 
-go 1.23.4
+go 1.23.5
 
 require github.com/golangci/golangci-lint v1.63.4
 

From 288f605b60eb16e104f8e433eb7387468e0e5bd4 Mon Sep 17 00:00:00 2001
From: kat <28567881+hk21702@users.noreply.github.com>
Date: Wed, 29 Jan 2025 13:32:58 -0500
Subject: [PATCH 2/4] Refactor uploader and consent management: rename methods,
 remove unused code, and improve time provider abstraction

---
 internal/consent/consent.go                   |  33 +++--
 internal/consent/consent_test.go              |  57 +++++++-
 internal/consent/export_test.go               |   4 +-
 ...alid_value_global_file,_valid_false_source |   0
 ...valid_value_global_file,_valid_true_source |   0
 .../golden/no_global_file,_valid_false_source |   0
 .../golden/no_global_file,_valid_true_source  |   0
 .../golden/valid_false_global_file            |   0
 .../golden/valid_true_global_file             |   0
 ...valid_true_global_file,_valid_false_source |   0
 .../valid_true_global_file,_valid_true_source |   0
 .../golden/new_file,_write_global_false       |   0
 .../golden/new_file,_write_global_true        |   0
 .../golden/new_file,_write_source_false       |   0
 .../golden/new_file,_write_source_true        |   0
 .../overwrite_file,_write_diff_global_false   |   0
 .../overwrite_file,_write_diff_global_true    |   0
 .../overwrite_file,_write_diff_source_false   |   0
 .../overwrite_file,_write_diff_source_true    |   0
 .../golden/overwrite_file,_write_global_false |   0
 .../golden/overwrite_file,_write_global_true  |   0
 .../golden/overwrite_file,_write_source_false |   0
 .../golden/overwrite_file,_write_source_true  |   0
 internal/fileutils/fileutils.go               |  10 --
 internal/fileutils/fileutils_test.go          |  36 -----
 internal/reportutils/reportutils.go           |  10 ++
 internal/uploader/export_test.go              |  13 ++
 internal/uploader/internal_test.go            |  14 +-
 internal/uploader/upload.go                   |  87 +++++++------
 internal/uploader/uploader.go                 |  20 +--
 internal/uploader/uploader_test.go            | 123 ++++++++----------
 31 files changed, 210 insertions(+), 197 deletions(-)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/invalid_value_global_file,_valid_false_source (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/invalid_value_global_file,_valid_true_source (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/no_global_file,_valid_false_source (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/no_global_file,_valid_true_source (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/valid_false_global_file (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/valid_true_global_file (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/valid_true_global_file,_valid_false_source (100%)
 rename internal/consent/testdata/{TestGetConsentState => TestGetState}/golden/valid_true_global_file,_valid_true_source (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/new_file,_write_global_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/new_file,_write_global_true (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/new_file,_write_source_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/new_file,_write_source_true (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_diff_global_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_diff_global_true (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_diff_source_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_diff_source_true (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_global_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_global_true (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_source_false (100%)
 rename internal/consent/testdata/{TestSetConsentStates => TestSetState}/golden/overwrite_file,_write_source_true (100%)

diff --git a/internal/consent/consent.go b/internal/consent/consent.go
index d13a145..b1c0749 100644
--- a/internal/consent/consent.go
+++ b/internal/consent/consent.go
@@ -30,12 +30,12 @@ func New(path string) *Manager {
 	return &Manager{path: path}
 }
 
-// GetConsentState gets the consent state for the given source.
+// GetState gets the consent state for the given source.
 // If the source do not have a consent file, it will be considered as a false state.
 // If the source is an empty string, then the global consent state will be returned.
 // If the target consent file does not exist, it will not be created.
-func (cm Manager) GetConsentState(source string) (bool, error) {
-	sourceConsent, err := readConsentFile(cm.getConsentFile(source))
+func (cm Manager) GetState(source string) (bool, error) {
+	sourceConsent, err := readFile(cm.getFile(source))
 	if err != nil {
 		slog.Error("Error reading source consent file", "source", source, "error", err)
 		return false, err
@@ -46,20 +46,33 @@ func (cm Manager) GetConsentState(source string) (bool, error) {
 
 var consentSourceFilePattern = `%s` + constants.ConsentSourceBaseSeparator + constants.GlobalFileName
 
-// SetConsentState updates the consent state for the given source.
+// SetState updates the consent state for the given source.
 // If the source is an empty string, then the global consent state will be set.
 // If the target consent file does not exist, it will be created.
-func (cm Manager) SetConsentState(source string, state bool) (err error) {
+func (cm Manager) SetState(source string, state bool) (err error) {
 	defer decorate.OnError(&err, "could not set consent state")
 
 	consent := consentFile{ConsentState: state}
-	return consent.write(cm.getConsentFile(source))
+	return consent.write(cm.getFile(source))
 }
 
-// getConsentFile returns the expected path to the consent file for the given source.
+// HasConsent returns true if there is consent for the given source, based on the hierarchy rules.
+// If the source has a consent file, its value is returned.
+// Otherwise, the global consent state is returned.
+func (cm Manager) HasConsent(source string) (bool, error) {
+	consent, err := cm.GetState(source)
+	if err != nil {
+		slog.Warn("Could not get source specific consent state, falling back to global consent state", "source", source, "error", err)
+		return cm.GetState("")
+	}
+
+	return consent, nil
+}
+
+// getFile returns the expected path to the consent file for the given source.
 // If source is blank, it returns the path to the global consent file.
 // It does not check if the file exists, or if it is valid.
-func (cm Manager) getConsentFile(source string) string {
+func (cm Manager) getFile(source string) string {
 	p := filepath.Join(cm.path, constants.GlobalFileName)
 	if source != "" {
 		p = filepath.Join(cm.path, fmt.Sprintf(consentSourceFilePattern, source))
@@ -69,7 +82,7 @@ func (cm Manager) getConsentFile(source string) string {
 }
 
 // getSourceConsentFiles returns a map of all paths to validly named consent files in the folder, other than the global file.
-func (cm Manager) getConsentFiles() (map[string]string, error) {
+func (cm Manager) getFiles() (map[string]string, error) {
 	sourceFiles := make(map[string]string)
 
 	entries, err := os.ReadDir(cm.path)
@@ -94,7 +107,7 @@ func (cm Manager) getConsentFiles() (map[string]string, error) {
 	return sourceFiles, nil
 }
 
-func readConsentFile(path string) (consentFile, error) {
+func readFile(path string) (consentFile, error) {
 	var consent consentFile
 	_, err := toml.DecodeFile(path, &consent)
 	slog.Debug("Read consent file", "file", path, "consent", consent.ConsentState)
diff --git a/internal/consent/consent_test.go b/internal/consent/consent_test.go
index 4393831..612fdb5 100644
--- a/internal/consent/consent_test.go
+++ b/internal/consent/consent_test.go
@@ -11,7 +11,7 @@ import (
 	"github.com/ubuntu/ubuntu-insights/internal/testutils"
 )
 
-func TestGetConsentState(t *testing.T) {
+func TestGetState(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
@@ -57,7 +57,7 @@ func TestGetConsentState(t *testing.T) {
 			require.NoError(t, err, "Setup: failed to setup temporary consent files")
 			cm := consent.New(dir)
 
-			got, err := cm.GetConsentState(tc.source)
+			got, err := cm.GetState(tc.source)
 			if tc.wantErr {
 				require.Error(t, err, "expected an error but got none")
 				return
@@ -70,7 +70,7 @@ func TestGetConsentState(t *testing.T) {
 	}
 }
 
-func TestSetConsentStates(t *testing.T) {
+func TestSetState(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
@@ -113,7 +113,7 @@ func TestSetConsentStates(t *testing.T) {
 			require.NoError(t, err, "Setup: failed to setup temporary consent files")
 			cm := consent.New(dir)
 
-			err = cm.SetConsentState(tc.writeSource, tc.writeState)
+			err = cm.SetState(tc.writeSource, tc.writeState)
 			if tc.wantErr {
 				require.Error(t, err, "expected an error but got none")
 				return
@@ -133,6 +133,55 @@ func TestSetConsentStates(t *testing.T) {
 	}
 }
 
+func TestHasConsent(t *testing.T) {
+	t.Parallel()
+
+	tests := map[string]struct {
+		source string
+
+		globalFile string
+
+		want    bool
+		wantErr bool
+	}{
+		"True Global-True Source":          {source: "valid_true", globalFile: "valid_true-consent.toml", want: true},
+		"True Global-False Source":         {source: "valid_false", globalFile: "valid_true-consent.toml", want: false},
+		"True Global-Invalid Value Source": {source: "invalid_value", globalFile: "valid_true-consent.toml", want: true},
+		"True Global-Invalid File Source":  {source: "invalid_file", globalFile: "valid_true-consent.toml", want: true},
+		"True Global-Not A File Source":    {source: "not_a_file", globalFile: "valid_true-consent.toml", want: true},
+
+		"False Global-True Source":          {source: "valid_true", globalFile: "valid_false-consent.toml", want: true},
+		"False Global-False Source":         {source: "valid_false", globalFile: "valid_false-consent.toml", want: false},
+		"False Global-Invalid Value Source": {source: "invalid_value", globalFile: "valid_false-consent.toml", want: false},
+		"False Global-Invalid File Source":  {source: "invalid_file", globalFile: "valid_false-consent.toml", want: false},
+		"False Global-Not A File Source":    {source: "not_a_file", globalFile: "valid_false-consent.toml", want: false},
+
+		"No Global-True Source":          {source: "valid_true", want: true},
+		"No Global-False Source":         {source: "valid_false", want: false},
+		"No Global-Invalid Value Source": {source: "invalid_value", wantErr: true},
+		"No Global-Invalid File Source":  {source: "invalid_file", wantErr: true},
+		"No Global-Not A File Source":    {source: "not_a_file", wantErr: true},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			dir, err := setupTmpConsentFiles(t, tc.globalFile)
+			require.NoError(t, err, "Setup: failed to setup temporary consent files")
+			cm := consent.New(dir)
+
+			got, err := cm.HasConsent(tc.source)
+			if tc.wantErr {
+				require.Error(t, err, "expected an error but got none")
+				return
+			}
+			require.NoError(t, err, "got an unexpected error")
+
+			require.Equal(t, tc.want, got, "HasConsent should return expected consent state")
+		})
+	}
+}
+
 func setupTmpConsentFiles(t *testing.T, globalFile string) (string, error) {
 	t.Helper()
 
diff --git a/internal/consent/export_test.go b/internal/consent/export_test.go
index f887256..8f88838 100644
--- a/internal/consent/export_test.go
+++ b/internal/consent/export_test.go
@@ -4,14 +4,14 @@ package consent
 // It does not get the global consent state.
 // If continueOnErr is true, it will continue to the next source if an error occurs.
 func (cm Manager) GetAllSourceConsentStates(continueOnErr bool) (map[string]bool, error) {
-	p, err := cm.getConsentFiles()
+	p, err := cm.getFiles()
 	if err != nil {
 		return nil, err
 	}
 
 	consentStates := make(map[string]bool)
 	for source, path := range p {
-		consent, err := readConsentFile(path)
+		consent, err := readFile(path)
 		if err != nil && !continueOnErr {
 			return nil, err
 		}
diff --git a/internal/consent/testdata/TestGetConsentState/golden/invalid_value_global_file,_valid_false_source b/internal/consent/testdata/TestGetState/golden/invalid_value_global_file,_valid_false_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/invalid_value_global_file,_valid_false_source
rename to internal/consent/testdata/TestGetState/golden/invalid_value_global_file,_valid_false_source
diff --git a/internal/consent/testdata/TestGetConsentState/golden/invalid_value_global_file,_valid_true_source b/internal/consent/testdata/TestGetState/golden/invalid_value_global_file,_valid_true_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/invalid_value_global_file,_valid_true_source
rename to internal/consent/testdata/TestGetState/golden/invalid_value_global_file,_valid_true_source
diff --git a/internal/consent/testdata/TestGetConsentState/golden/no_global_file,_valid_false_source b/internal/consent/testdata/TestGetState/golden/no_global_file,_valid_false_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/no_global_file,_valid_false_source
rename to internal/consent/testdata/TestGetState/golden/no_global_file,_valid_false_source
diff --git a/internal/consent/testdata/TestGetConsentState/golden/no_global_file,_valid_true_source b/internal/consent/testdata/TestGetState/golden/no_global_file,_valid_true_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/no_global_file,_valid_true_source
rename to internal/consent/testdata/TestGetState/golden/no_global_file,_valid_true_source
diff --git a/internal/consent/testdata/TestGetConsentState/golden/valid_false_global_file b/internal/consent/testdata/TestGetState/golden/valid_false_global_file
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/valid_false_global_file
rename to internal/consent/testdata/TestGetState/golden/valid_false_global_file
diff --git a/internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file b/internal/consent/testdata/TestGetState/golden/valid_true_global_file
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file
rename to internal/consent/testdata/TestGetState/golden/valid_true_global_file
diff --git a/internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file,_valid_false_source b/internal/consent/testdata/TestGetState/golden/valid_true_global_file,_valid_false_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file,_valid_false_source
rename to internal/consent/testdata/TestGetState/golden/valid_true_global_file,_valid_false_source
diff --git a/internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file,_valid_true_source b/internal/consent/testdata/TestGetState/golden/valid_true_global_file,_valid_true_source
similarity index 100%
rename from internal/consent/testdata/TestGetConsentState/golden/valid_true_global_file,_valid_true_source
rename to internal/consent/testdata/TestGetState/golden/valid_true_global_file,_valid_true_source
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_global_false b/internal/consent/testdata/TestSetState/golden/new_file,_write_global_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_global_false
rename to internal/consent/testdata/TestSetState/golden/new_file,_write_global_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_global_true b/internal/consent/testdata/TestSetState/golden/new_file,_write_global_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_global_true
rename to internal/consent/testdata/TestSetState/golden/new_file,_write_global_true
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_source_false b/internal/consent/testdata/TestSetState/golden/new_file,_write_source_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_source_false
rename to internal/consent/testdata/TestSetState/golden/new_file,_write_source_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_source_true b/internal/consent/testdata/TestSetState/golden/new_file,_write_source_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/new_file,_write_source_true
rename to internal/consent/testdata/TestSetState/golden/new_file,_write_source_true
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_global_false b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_global_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_global_false
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_global_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_global_true b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_global_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_global_true
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_global_true
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_source_false b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_source_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_source_false
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_source_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_source_true b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_source_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_diff_source_true
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_diff_source_true
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_global_false b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_global_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_global_false
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_global_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_global_true b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_global_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_global_true
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_global_true
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_source_false b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_source_false
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_source_false
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_source_false
diff --git a/internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_source_true b/internal/consent/testdata/TestSetState/golden/overwrite_file,_write_source_true
similarity index 100%
rename from internal/consent/testdata/TestSetConsentStates/golden/overwrite_file,_write_source_true
rename to internal/consent/testdata/TestSetState/golden/overwrite_file,_write_source_true
diff --git a/internal/fileutils/fileutils.go b/internal/fileutils/fileutils.go
index b07bfde..b113a2f 100644
--- a/internal/fileutils/fileutils.go
+++ b/internal/fileutils/fileutils.go
@@ -2,7 +2,6 @@
 package fileutils
 
 import (
-	"errors"
 	"fmt"
 	"log/slog"
 	"os"
@@ -37,12 +36,3 @@ func AtomicWrite(path string, data []byte) error {
 	}
 	return nil
 }
-
-// FileExists checks if a file exists.
-func FileExists(path string) (bool, error) {
-	_, err := os.Stat(path)
-	if err != nil && !errors.Is(err, os.ErrNotExist) {
-		return false, err
-	}
-	return !errors.Is(err, os.ErrNotExist), nil
-}
diff --git a/internal/fileutils/fileutils_test.go b/internal/fileutils/fileutils_test.go
index ec6f776..15c2b96 100644
--- a/internal/fileutils/fileutils_test.go
+++ b/internal/fileutils/fileutils_test.go
@@ -74,39 +74,3 @@ func TestAtomicWrite(t *testing.T) {
 		})
 	}
 }
-
-func TestFileExists(t *testing.T) {
-	t.Parallel()
-
-	tests := map[string]struct {
-		fileExists bool
-
-		wantExists bool
-		wantError  bool
-	}{
-		"Returns_true_when_file_exists":                      {fileExists: true, wantExists: true},
-		"Returns_false_when_file_does_not_exist":             {fileExists: false, wantExists: false},
-		"Returns_false_when_parent_directory_does_not_exist": {fileExists: false, wantExists: false},
-	}
-
-	for name, tc := range tests {
-		t.Run(name, func(t *testing.T) {
-			t.Parallel()
-
-			tempDir := t.TempDir()
-			path := filepath.Join(tempDir, "file")
-			if tc.fileExists {
-				err := fileutils.AtomicWrite(path, []byte(""))
-				require.NoError(t, err, "Setup: AtomicWrite should not return an error")
-			}
-
-			exists, err := fileutils.FileExists(path)
-			if tc.wantError {
-				require.Error(t, err, "FileExists should return an error")
-			} else {
-				require.NoError(t, err, "FileExists should not return an error")
-			}
-			require.Equal(t, tc.wantExists, exists, "FileExists should return the expected result")
-		})
-	}
-}
diff --git a/internal/reportutils/reportutils.go b/internal/reportutils/reportutils.go
index f08a695..932d99c 100644
--- a/internal/reportutils/reportutils.go
+++ b/internal/reportutils/reportutils.go
@@ -1,6 +1,8 @@
 // Package reportutils provides utility functions for handling reports.
 package reportutils
 
+// package report
+
 import (
 	"errors"
 	"log/slog"
@@ -16,6 +18,8 @@ import (
 // ErrInvalidPeriod is returned when a function requiring a period, received an invalid, period that isn't a non-negative integer.
 var ErrInvalidPeriod = errors.New("invalid period, period should be a positive integer")
 
+// type Report struct{}
+
 // GetPeriodStart returns the start of the period window for a given period in seconds.
 func GetPeriodStart(period int) (int64, error) {
 	if period <= 0 {
@@ -31,6 +35,8 @@ func GetReportTime(path string) (int64, error) {
 	return strconv.ParseInt(strings.TrimSuffix(fileName, filepath.Ext(fileName)), 10, 64)
 }
 
+// Path is a method on Report
+
 // GetReportPath returns the path for the most recent report within a period window, returning an empty string if no report is found.
 // Not inclusive of the period end (periodStart + period).
 //
@@ -85,6 +91,8 @@ func GetReportPath(dir string, time int64, period int) (string, error) {
 	return mostRecentReportPath, nil
 }
 
+// -> map[int64]Report
+
 // GetReports returns the paths for the latest report within each period window for a given directory.
 // The key of the map is the start of the period window, and the report timestamp.
 //
@@ -131,6 +139,8 @@ func GetReports(dir string, period int) (map[int64]int64, error) {
 	return reports, nil
 }
 
+// reports.GetAll(dir string) ([]Report, error)
+
 // GetAllReports returns the filename for all reports within a given directory, which match the expected pattern.
 // Does not traverse subdirectories.
 func GetAllReports(dir string) ([]string, error) {
diff --git a/internal/uploader/export_test.go b/internal/uploader/export_test.go
index d34d9f5..a9a4c3f 100644
--- a/internal/uploader/export_test.go
+++ b/internal/uploader/export_test.go
@@ -1,17 +1,30 @@
 package uploader
 
+import "time"
+
+type MockTimeProvider struct {
+	CurrentTime int64
+}
+
+func (m MockTimeProvider) Now() time.Time {
+	return time.Unix(m.CurrentTime, 0)
+}
+
+// WithCachePath sets the cache path for the uploader.
 func WithCachePath(path string) Options {
 	return func(o *options) {
 		o.cachePath = path
 	}
 }
 
+// WithBaseServerURL sets the base server URL for the uploader.
 func WithBaseServerURL(url string) Options {
 	return func(o *options) {
 		o.baseServerURL = url
 	}
 }
 
+// WithTimeProvider sets the time provider for the uploader.
 func WithTimeProvider(tp timeProvider) Options {
 	return func(o *options) {
 		o.timeProvider = tp
diff --git a/internal/uploader/internal_test.go b/internal/uploader/internal_test.go
index d7a0147..f9882d3 100644
--- a/internal/uploader/internal_test.go
+++ b/internal/uploader/internal_test.go
@@ -12,14 +12,6 @@ import (
 	"github.com/ubuntu/ubuntu-insights/internal/fileutils"
 )
 
-type mockTimeProvider struct {
-	currentTime int64
-}
-
-func (m mockTimeProvider) NowUnix() int64 {
-	return m.currentTime
-}
-
 func TestUploadBadFile(t *testing.T) {
 	t.Parallel()
 	basicContent := `{"Content":true, "string": "string"}`
@@ -59,11 +51,11 @@ func TestUploadBadFile(t *testing.T) {
 
 			dir := t.TempDir()
 
-			um := &Manager{
+			um := &Uploader{
 				collectedDir: filepath.Join(dir, constants.LocalFolder),
 				uploadedDir:  filepath.Join(dir, constants.UploadedFolder),
 				minAge:       0,
-				timeProvider: mockTimeProvider{currentTime: 0},
+				timeProvider: MockTimeProvider{CurrentTime: 0},
 			}
 
 			require.NoError(t, os.Mkdir(um.collectedDir, 0750), "Setup: failed to create uploaded folder")
@@ -114,7 +106,7 @@ func TestMoveReport(t *testing.T) {
 			t.Parallel()
 			dir := t.TempDir()
 
-			um := &Manager{
+			um := &Uploader{
 				collectedDir: filepath.Join(dir, constants.LocalFolder),
 				uploadedDir:  filepath.Join(dir, constants.UploadedFolder),
 			}
diff --git a/internal/uploader/upload.go b/internal/uploader/upload.go
index cc15bdd..2f181e3 100644
--- a/internal/uploader/upload.go
+++ b/internal/uploader/upload.go
@@ -3,6 +3,7 @@ package uploader
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"log/slog"
 	"net/http"
@@ -18,19 +19,19 @@ import (
 	"github.com/ubuntu/ubuntu-insights/internal/reportutils"
 )
 
+var (
+	// ErrReportNotMature is returned when a report is not mature enough to be uploaded.
+	ErrReportNotMature = errors.New("report is not mature enough to be uploaded")
+)
+
 // Upload uploads the reports corresponding to the source to the configured server.
 // Does not do duplicate checks.
-func (um Manager) Upload(force bool) error {
+func (um Uploader) Upload(force bool) error {
 	slog.Debug("Uploading reports")
 
-	gConsent, err := um.consentM.GetConsentState("")
+	consent, err := um.consentM.HasConsent(um.source)
 	if err != nil {
-		return fmt.Errorf("upload failed to get global consent state: %v", err)
-	}
-
-	sConsent, err := um.consentM.GetConsentState(um.source)
-	if err != nil {
-		return fmt.Errorf("upload failed to get source consent state: %v", err)
+		return fmt.Errorf("upload failed to get consent state: %v", err)
 	}
 
 	reports, err := reportutils.GetAllReports(um.collectedDir)
@@ -44,14 +45,17 @@ func (um Manager) Upload(force bool) error {
 	}
 
 	var wg sync.WaitGroup
-	for _, file := range reports {
+	for _, name := range reports {
 		wg.Add(1)
-		go func(file string) {
+		go func(name string) {
 			defer wg.Done()
-			if err := um.upload(file, url, gConsent && sConsent, force); err != nil {
-				slog.Warn("Failed to upload report", "file", file, "source", um.source, "error", err)
+			err := um.upload(name, url, consent, force)
+			if errors.Is(err, ErrReportNotMature) {
+				slog.Debug("Skipped report upload, not mature enough", "file", name, "source", um.source)
+			} else if err != nil {
+				slog.Warn("Failed to upload report", "file", name, "source", um.source, "error", err)
 			}
-		}(file)
+		}(name)
 	}
 	wg.Wait()
 
@@ -60,28 +64,31 @@ func (um Manager) Upload(force bool) error {
 
 // upload uploads an individual report to the server. It returns an error if the report is not mature enough to be uploaded, or if the upload fails.
 // It also moves the report to the uploaded directory after a successful upload.
-func (um Manager) upload(fName, url string, consent, force bool) error {
-	slog.Debug("Uploading report", "file", fName, "consent", consent, "force", force)
+func (um Uploader) upload(name, url string, consent, force bool) error {
+	slog.Debug("Uploading report", "file", name, "consent", consent, "force", force)
 
-	ts, err := reportutils.GetReportTime(fName)
+	// TODO… pass the Report object directly.
+	ts, err := reportutils.GetReportTime(name)
 	if err != nil {
 		return fmt.Errorf("failed to parse report time from filename: %v", err)
 	}
 
-	if ts > um.timeProvider.NowUnix()-um.minAge && !force {
-		return fmt.Errorf("report is not mature enough to be uploaded")
+	if um.timeProvider.Now().Add(time.Duration(-um.minAge)*time.Second).Before(time.Unix(ts, 0)) && !force {
+		return ErrReportNotMature
 	}
 
 	// Check for duplicate reports.
-	fileExists, err := fileutils.FileExists(filepath.Join(um.uploadedDir, fName))
-	if err != nil {
+	_, err = os.Stat(filepath.Join(um.uploadedDir, name))
+	if err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("failed to check if report has already been uploaded: %v", err)
 	}
-	if fileExists && !force {
+	if err == nil && !force {
+		// TODO: What to do with the original file? Should we clean it up?
+		// Should we move it elsewhere for investigation in a "tmp" and clean it afterwards?
 		return fmt.Errorf("report has already been uploaded")
 	}
 
-	origData, err := um.readPayload(fName)
+	origData, err := um.readJSON(name)
 	if err != nil {
 		return fmt.Errorf("failed to get payload: %v", err)
 	}
@@ -100,11 +107,12 @@ func (um Manager) upload(fName, url string, consent, force bool) error {
 	}
 
 	// Move report first to avoid the situation where the report is sent, but not marked as sent.
-	if err := um.moveReport(filepath.Join(um.uploadedDir, fName), filepath.Join(um.collectedDir, fName), data); err != nil {
+	// TODO: maybe a method on Reports ?
+	if err := um.moveReport(filepath.Join(um.uploadedDir, name), filepath.Join(um.collectedDir, name), data); err != nil {
 		return fmt.Errorf("failed to move report after uploading: %v", err)
 	}
 	if err := send(url, data); err != nil {
-		if moveErr := um.moveReport(filepath.Join(um.collectedDir, fName), filepath.Join(um.uploadedDir, fName), origData); moveErr != nil {
+		if moveErr := um.moveReport(filepath.Join(um.collectedDir, name), filepath.Join(um.uploadedDir, name), origData); moveErr != nil {
 			return fmt.Errorf("failed to send data: %v, and failed to restore the original report: %v", err, moveErr)
 		}
 		return fmt.Errorf("failed to send data: %v", err)
@@ -113,7 +121,7 @@ func (um Manager) upload(fName, url string, consent, force bool) error {
 	return nil
 }
 
-func (um Manager) getURL() (string, error) {
+func (um Uploader) getURL() (string, error) {
 	u, err := url.Parse(um.baseServerURL)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse base server URL %s: %v", um.baseServerURL, err)
@@ -122,37 +130,32 @@ func (um Manager) getURL() (string, error) {
 	return u.String(), nil
 }
 
-func (um Manager) readPayload(file string) ([]byte, error) {
-	var jsonData map[string]interface{}
-
+// readJSON reads the JSON data from the report file.
+func (um Uploader) readJSON(file string) ([]byte, error) {
 	// Read the report file
 	data, err := os.ReadFile(path.Join(um.collectedDir, file))
 	if err != nil {
 		return nil, fmt.Errorf("failed to read report file: %v", err)
 	}
 
-	// Remarshal the JSON data to ensure it is valid
-	if err := json.Unmarshal(data, &jsonData); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal JSON data: %v", err)
-	}
-
-	data, err = json.Marshal(jsonData)
-	if err != nil {
-		return nil, fmt.Errorf("failed to marshal JSON data: %v", err)
+	if !json.Valid(data) {
+		return nil, fmt.Errorf("invalid JSON data in report file")
 	}
 
 	return data, nil
 }
 
-// moveReport writes the data to the writePath, and removes the matching file from the removePath.
-func (um Manager) moveReport(writePath, removePath string, data []byte) error {
-	err := fileutils.AtomicWrite(writePath, data)
-	if err != 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)
 	}
 
-	err = os.Remove(removePath)
-	if err != nil {
+	if err := os.Remove(removePath); err != nil {
 		return fmt.Errorf("failed to remove report: %v", err)
 	}
 
diff --git a/internal/uploader/uploader.go b/internal/uploader/uploader.go
index c304583..2469319 100644
--- a/internal/uploader/uploader.go
+++ b/internal/uploader/uploader.go
@@ -13,17 +13,17 @@ import (
 )
 
 type timeProvider interface {
-	NowUnix() int64
+	Now() time.Time
 }
 
 type realTimeProvider struct{}
 
-func (realTimeProvider) NowUnix() int64 {
-	return time.Now().Unix()
+func (realTimeProvider) Now() time.Time {
+	return time.Now()
 }
 
-// Manager is an abstraction of the uploader component.
-type Manager struct {
+// Uploader is an abstraction of the uploader component.
+type Uploader struct {
 	source   string
 	consentM consentManager
 	minAge   int64
@@ -46,19 +46,19 @@ type options struct {
 type Options func(*options)
 
 type consentManager interface {
-	GetConsentState(source string) (bool, error)
+	HasConsent(source string) (bool, error)
 }
 
 // New returns a new UploaderManager.
-func New(cm consentManager, source string, minAge uint, dryRun bool, args ...Options) (Manager, error) {
+func New(cm consentManager, source string, minAge uint, dryRun bool, args ...Options) (Uploader, error) {
 	slog.Debug("Creating new uploader manager", "source", source, "minAge", minAge, "dryRun", dryRun)
 
 	if source == "" {
-		return Manager{}, fmt.Errorf("source cannot be an empty string")
+		return Uploader{}, fmt.Errorf("source cannot be an empty string")
 	}
 
 	if minAge > math.MaxInt64 {
-		return Manager{}, fmt.Errorf("min age %d is too large, would overflow", minAge)
+		return Uploader{}, fmt.Errorf("min age %d is too large, would overflow", minAge)
 	}
 
 	opts := options{
@@ -70,7 +70,7 @@ func New(cm consentManager, source string, minAge uint, dryRun bool, args ...Opt
 		opt(&opts)
 	}
 
-	return Manager{
+	return Uploader{
 		source:       source,
 		consentM:     cm,
 		minAge:       int64(minAge),
diff --git a/internal/uploader/uploader_test.go b/internal/uploader/uploader_test.go
index 7dc4bd5..2a4d3d1 100644
--- a/internal/uploader/uploader_test.go
+++ b/internal/uploader/uploader_test.go
@@ -25,48 +25,36 @@ var (
 	badContent            = `bad content`
 )
 
-type mockTimeProvider struct {
-	currentTime int64
-}
-
-func (m mockTimeProvider) NowUnix() int64 {
-	return m.currentTime
-}
-
 var (
-	cmSErr     = testConsentManager{sErr: fmt.Errorf("consent error")}
-	cmTrueSErr = testConsentManager{sState: true, gState: true, sErr: fmt.Errorf("consent error")}
-	cmGErr     = testConsentManager{gErr: fmt.Errorf("consent error")}
-	cmTrueGErr = testConsentManager{gState: true, gErr: fmt.Errorf("consent error")}
-	cmTrue     = testConsentManager{sState: true, gState: true}
-	cmFalse    = testConsentManager{sState: false, gState: false}
-	cmSTrue    = testConsentManager{sState: true, gState: false}
-	cmGTrue    = testConsentManager{sState: false, gState: true}
+	cTrue    = testConsentChecker{consent: true}
+	cFalse   = testConsentChecker{consent: false}
+	cErr     = testConsentChecker{err: fmt.Errorf("consent error")}
+	cErrTrue = testConsentChecker{consent: true, err: fmt.Errorf("consent error")}
 )
 
 func TestNew(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
-		cm     testConsentManager
-		source string
-		minAge uint
-		dryRun bool
+		consent testConsentChecker
+		source  string
+		minAge  uint
+		dryRun  bool
 
 		wantErr bool
 	}{
-		"Valid":        {cm: cmTrue, source: "source", minAge: 5, dryRun: true},
-		"Zero Min Age": {cm: cmTrue, source: "source", minAge: 0},
+		"Valid":        {consent: cTrue, source: "source", minAge: 5, dryRun: true},
+		"Zero Min Age": {consent: cTrue, source: "source", minAge: 0},
 
-		"Empty Source":    {cm: cmTrue, source: "", wantErr: true},
-		"Minage Overflow": {cm: cmTrue, source: "source", minAge: math.MaxUint64, wantErr: true},
+		"Empty Source":    {consent: cTrue, source: "", wantErr: true},
+		"Minage Overflow": {consent: cTrue, source: "source", minAge: math.MaxUint64, wantErr: true},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			_, err := uploader.New(tc.cm, tc.source, tc.minAge, tc.dryRun)
+			_, err := uploader.New(tc.consent, tc.source, tc.minAge, tc.dryRun)
 			if tc.wantErr {
 				require.Error(t, err)
 				return
@@ -89,44 +77,40 @@ func TestUpload(t *testing.T) {
 		url                       string
 		invalidDir                bool
 
-		cm     testConsentManager
-		minAge uint
-		dryRun bool
-		force  bool
+		consent testConsentChecker
+		minAge  uint
+		dryRun  bool
+		force   bool
 
 		wantErr bool
 	}{
-		"No Reports":            {cm: cmTrue, serverResponse: http.StatusOK},
-		"No Reports with Dummy": {dummy: true, cm: cmTrue, serverResponse: http.StatusOK},
-		"Single Upload":         {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverResponse: http.StatusOK},
-		"Multi Upload":          {localFiles: map[string]reportType{"1.json": normal, "5.json": normal}, cm: cmTrue, serverResponse: http.StatusOK},
-		"Min Age":               {localFiles: map[string]reportType{"1.json": normal, "9.json": normal}, cm: cmTrue, minAge: 5, serverResponse: http.StatusOK},
-		"Future Timestamp":      {localFiles: map[string]reportType{"1.json": normal, "11.json": normal}, cm: cmTrue, serverResponse: http.StatusOK},
-		"Duplicate Upload":      {localFiles: map[string]reportType{"1.json": normal}, uploadedFiles: map[string]reportType{"1.json": badContent}, cm: cmTrue, serverResponse: http.StatusAccepted},
-		"Bad Content":           {localFiles: map[string]reportType{"1.json": badContent}, cm: cmTrue, serverResponse: http.StatusOK},
-
-		"Consent Manager Source Error":              {localFiles: map[string]reportType{"1.json": normal}, cm: cmSErr, serverResponse: http.StatusOK, wantErr: true},
-		"Consent Manager Source Error with True":    {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrueSErr, serverResponse: http.StatusOK, wantErr: true},
-		"Consent Manager Global Error":              {localFiles: map[string]reportType{"1.json": normal}, cm: cmGErr, serverResponse: http.StatusOK, wantErr: true},
-		"Consent Manager Global Error with True":    {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrueGErr, serverResponse: http.StatusOK, wantErr: true},
-		"Consent Manager False":                     {localFiles: map[string]reportType{"1.json": normal}, cm: cmFalse, serverResponse: http.StatusOK},
-		"Consent Manager Global True, Source False": {localFiles: map[string]reportType{"1.json": normal}, cm: cmGTrue, serverResponse: http.StatusOK},
-		"Consent Manager Global False, Source True": {localFiles: map[string]reportType{"1.json": normal}, cm: cmSTrue, serverResponse: http.StatusOK},
-
-		"Force CM False":  {localFiles: map[string]reportType{"1.json": normal}, cm: cmFalse, force: true, serverResponse: http.StatusOK},
-		"Force Min Age":   {localFiles: map[string]reportType{"1.json": normal, "9.json": normal}, cm: cmTrue, minAge: 5, force: true, serverResponse: http.StatusOK},
-		"Force Duplicate": {localFiles: map[string]reportType{"1.json": normal}, uploadedFiles: map[string]reportType{"1.json": badContent}, cm: cmTrue, force: true, serverResponse: http.StatusOK},
-
-		"OptOut Payload CM True":  {localFiles: map[string]reportType{"1.json": optOut}, cm: cmTrue, serverResponse: http.StatusOK},
-		"OptOut Payload CM False": {localFiles: map[string]reportType{"1.json": optOut}, cm: cmFalse, serverResponse: http.StatusOK},
-
-		"Dry run": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, dryRun: true},
-
-		"Bad URL":        {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, url: "http://a b.com/", wantErr: true},
-		"Bad Response":   {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverResponse: http.StatusForbidden},
-		"Offline Server": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, serverOffline: true},
-
-		"Invalid Directory": {localFiles: map[string]reportType{"1.json": normal}, cm: cmTrue, invalidDir: true, wantErr: true},
+		"No Reports":            {consent: cTrue, serverResponse: http.StatusOK},
+		"No Reports with Dummy": {dummy: true, consent: cTrue, serverResponse: http.StatusOK},
+		"Single Upload":         {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, serverResponse: http.StatusOK},
+		"Multi Upload":          {localFiles: map[string]reportType{"1.json": normal, "5.json": normal}, consent: cTrue, serverResponse: http.StatusOK},
+		"Min Age":               {localFiles: map[string]reportType{"1.json": normal, "9.json": normal}, consent: cTrue, minAge: 5, serverResponse: http.StatusOK},
+		"Future Timestamp":      {localFiles: map[string]reportType{"1.json": normal, "11.json": normal}, consent: cTrue, serverResponse: http.StatusOK},
+		"Duplicate Upload":      {localFiles: map[string]reportType{"1.json": normal}, uploadedFiles: map[string]reportType{"1.json": badContent}, consent: cTrue, serverResponse: http.StatusAccepted},
+		"Bad Content":           {localFiles: map[string]reportType{"1.json": badContent}, consent: cTrue, serverResponse: http.StatusOK},
+
+		"Consent Manager Source Error":           {localFiles: map[string]reportType{"1.json": normal}, consent: cErr, serverResponse: http.StatusOK, wantErr: true},
+		"Consent Manager Source Error with True": {localFiles: map[string]reportType{"1.json": normal}, consent: cErrTrue, serverResponse: http.StatusOK, wantErr: true},
+		"Consent Manager False":                  {localFiles: map[string]reportType{"1.json": normal}, consent: cFalse, serverResponse: http.StatusOK},
+
+		"Force CM False":  {localFiles: map[string]reportType{"1.json": normal}, consent: cFalse, force: true, serverResponse: http.StatusOK},
+		"Force Min Age":   {localFiles: map[string]reportType{"1.json": normal, "9.json": normal}, consent: cTrue, minAge: 5, force: true, serverResponse: http.StatusOK},
+		"Force Duplicate": {localFiles: map[string]reportType{"1.json": normal}, uploadedFiles: map[string]reportType{"1.json": badContent}, consent: cTrue, force: true, serverResponse: http.StatusOK},
+
+		"OptOut Payload CM True":  {localFiles: map[string]reportType{"1.json": optOut}, consent: cTrue, serverResponse: http.StatusOK},
+		"OptOut Payload CM False": {localFiles: map[string]reportType{"1.json": optOut}, consent: cFalse, serverResponse: http.StatusOK},
+
+		"Dry run": {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, dryRun: true},
+
+		"Bad URL":        {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, url: "http://a b.com/", wantErr: true},
+		"Bad Response":   {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, serverResponse: http.StatusForbidden},
+		"Offline Server": {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, serverOffline: true},
+
+		"Invalid Directory": {localFiles: map[string]reportType{"1.json": normal}, consent: cTrue, invalidDir: true, wantErr: true},
 	}
 
 	for name, tc := range tests {
@@ -149,8 +133,8 @@ func TestUpload(t *testing.T) {
 				require.NoError(t, os.RemoveAll(filepath.Join(dir, "local")), "Setup: failed to remove local directory")
 			}
 
-			mgr, err := uploader.New(tc.cm, "source", tc.minAge, tc.dryRun,
-				uploader.WithBaseServerURL(tc.url), uploader.WithCachePath(dir), uploader.WithTimeProvider(mockTimeProvider{currentTime: mockTime}))
+			mgr, err := uploader.New(tc.consent, "source", tc.minAge, tc.dryRun,
+				uploader.WithBaseServerURL(tc.url), uploader.WithCachePath(dir), uploader.WithTimeProvider(uploader.MockTimeProvider{CurrentTime: mockTime}))
 			require.NoError(t, err, "Setup: failed to create new uploader manager")
 
 			err = mgr.Upload(tc.force)
@@ -211,16 +195,11 @@ func writeFiles(t *testing.T, targetDir string, files map[string]reportType) {
 	}
 }
 
-type testConsentManager struct {
-	sState bool
-	gState bool
-	sErr   error
-	gErr   error
+type testConsentChecker struct {
+	consent bool
+	err     error
 }
 
-func (m testConsentManager) GetConsentState(source string) (bool, error) {
-	if source != "" {
-		return m.sState, m.sErr
-	}
-	return m.gState, m.gErr
+func (m testConsentChecker) HasConsent(source string) (bool, error) {
+	return m.consent, m.err
 }

From 24e17725bbb1b8807861e278606b3c86f088d25d Mon Sep 17 00:00:00 2001
From: kat <28567881+hk21702@users.noreply.github.com>
Date: Wed, 29 Jan 2025 18:04:33 -0500
Subject: [PATCH 3/4] Report package for handling report files

---
 internal/report/export_test.go                |  10 +
 internal/report/report.go                     | 223 ++++++++++++++++++
 .../report_test.go}                           |  84 ++++---
 .../TestGetAll}/golden/empty_directory        |   0
 .../TestGetAll/golden/files_in_subdir         |  12 +
 .../TestGetAll}/golden/invalid_file_extension |   0
 .../TestGetAll}/golden/invalid_file_names     |   0
 .../golden/mix_of_valid_and_invalid           |  18 ++
 .../TestGetForPeriod/golden/empty_directory   |   3 +
 .../TestGetForPeriod/golden/empty_subdir      |   3 +
 .../TestGetForPeriod/golden/files_in_subdir   |   3 +
 .../golden/invalid_file_extension             |   3 +
 .../golden/invalid_file_names                 |   3 +
 .../golden/negative_timestamp                 |   3 +
 .../golden/not_inclusive_period               |   3 +
 .../golden/specific_time_single_valid_report  |   3 +
 .../TestGetPerPeriod}/golden/empty_directory  |   0
 .../TestGetPerPeriod}/golden/files_in_subdir  |   0
 .../TestGetPerPeriod/golden/get_all_reports   |  28 +++
 .../golden/get_newest_of_period               |   4 +
 .../golden/invalid_file_extension             |   0
 .../golden/invalid_file_names                 |   0
 .../golden/mix_of_valid_and_invalid           |   4 +
 .../golden/multiple_consecutive_windows       |  12 +
 .../golden/multiple_non-consecutive_windows   |  12 +
 .../testdata/TestNew/golden/valid_report      |   3 +
 .../TestNew/golden/valid_report_with_path     |   3 +
 internal/reportutils/reportutils.go           | 177 --------------
 .../TestGetAllReports/golden/files_in_subdir  |   2 -
 .../golden/mix_of_valid_and_invalid           |   3 -
 .../TestGetReportPath/golden/empty_directory  |   0
 .../TestGetReportPath/golden/empty_subdir     |   0
 .../TestGetReportPath/golden/files_in_subdir  |   0
 .../golden/invalid_file_extension             |   0
 .../golden/invalid_file_names                 |   0
 .../golden/negative_timestamp                 |   1 -
 .../golden/not_inclusive_period               |   1 -
 .../golden/specific_time_single_valid_report  |   1 -
 .../TestGetReportTime/golden/alt_extension    |   1 -
 .../golden/valid_report_time                  |   1 -
 .../golden/valid_report_time_with_path        |   1 -
 .../TestGetReports/golden/get_all_reports     |   7 -
 .../golden/get_newest_of_period               |   1 -
 .../golden/mix_of_valid_and_invalid           |   1 -
 .../golden/multiple_consecutive_windows       |   3 -
 .../golden/multiple_consequtive_windows       |   3 -
 .../golden/multiple_non-consecutive_windows   |   3 -
 .../golden/multiple_non-consequtive_windows   |   3 -
 internal/uploader/internal_test.go            |  19 +-
 internal/uploader/upload.go                   |  53 ++---
 50 files changed, 432 insertions(+), 286 deletions(-)
 create mode 100644 internal/report/export_test.go
 create mode 100644 internal/report/report.go
 rename internal/{reportutils/reportutils_test.go => report/report_test.go} (79%)
 rename internal/{reportutils/testdata/TestGetAllReports => report/testdata/TestGetAll}/golden/empty_directory (100%)
 create mode 100644 internal/report/testdata/TestGetAll/golden/files_in_subdir
 rename internal/{reportutils/testdata/TestGetAllReports => report/testdata/TestGetAll}/golden/invalid_file_extension (100%)
 rename internal/{reportutils/testdata/TestGetAllReports => report/testdata/TestGetAll}/golden/invalid_file_names (100%)
 create mode 100644 internal/report/testdata/TestGetAll/golden/mix_of_valid_and_invalid
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/empty_directory
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/empty_subdir
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/files_in_subdir
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/invalid_file_extension
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/invalid_file_names
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/negative_timestamp
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/not_inclusive_period
 create mode 100644 internal/report/testdata/TestGetForPeriod/golden/specific_time_single_valid_report
 rename internal/{reportutils/testdata/TestGetReports => report/testdata/TestGetPerPeriod}/golden/empty_directory (100%)
 rename internal/{reportutils/testdata/TestGetReports => report/testdata/TestGetPerPeriod}/golden/files_in_subdir (100%)
 create mode 100644 internal/report/testdata/TestGetPerPeriod/golden/get_all_reports
 create mode 100644 internal/report/testdata/TestGetPerPeriod/golden/get_newest_of_period
 rename internal/{reportutils/testdata/TestGetReports => report/testdata/TestGetPerPeriod}/golden/invalid_file_extension (100%)
 rename internal/{reportutils/testdata/TestGetReports => report/testdata/TestGetPerPeriod}/golden/invalid_file_names (100%)
 create mode 100644 internal/report/testdata/TestGetPerPeriod/golden/mix_of_valid_and_invalid
 create mode 100644 internal/report/testdata/TestGetPerPeriod/golden/multiple_consecutive_windows
 create mode 100644 internal/report/testdata/TestGetPerPeriod/golden/multiple_non-consecutive_windows
 create mode 100644 internal/report/testdata/TestNew/golden/valid_report
 create mode 100644 internal/report/testdata/TestNew/golden/valid_report_with_path
 delete mode 100644 internal/reportutils/reportutils.go
 delete mode 100644 internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir
 delete mode 100644 internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/empty_directory
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/empty_subdir
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/files_in_subdir
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_extension
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_names
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/negative_timestamp
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/not_inclusive_period
 delete mode 100644 internal/reportutils/testdata/TestGetReportPath/golden/specific_time_single_valid_report
 delete mode 100644 internal/reportutils/testdata/TestGetReportTime/golden/alt_extension
 delete mode 100644 internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time
 delete mode 100644 internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time_with_path
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/get_all_reports
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/get_newest_of_period
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/mix_of_valid_and_invalid
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/multiple_consequtive_windows
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows
 delete mode 100644 internal/reportutils/testdata/TestGetReports/golden/multiple_non-consequtive_windows

diff --git a/internal/report/export_test.go b/internal/report/export_test.go
new file mode 100644
index 0000000..6a8629f
--- /dev/null
+++ b/internal/report/export_test.go
@@ -0,0 +1,10 @@
+package report
+
+type ReportStash = reportStash
+
+// ReportStash returns the reportStash of the report.
+//
+//nolint:revive // This is a false positive as we returned a typed alias and not the private type.
+func (r Report) ReportStash() ReportStash {
+	return r.reportStash
+}
diff --git a/internal/report/report.go b/internal/report/report.go
new file mode 100644
index 0000000..eb22687
--- /dev/null
+++ b/internal/report/report.go
@@ -0,0 +1,223 @@
+// Package report provides utility functions for handling reports.
+package report
+
+// package report
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"log/slog"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"time"
+
+	"github.com/ubuntu/ubuntu-insights/internal/constants"
+)
+
+var (
+	// ErrInvalidPeriod is returned when a function requiring a period, received an invalid, period that isn't a non-negative integer.
+	ErrInvalidPeriod = errors.New("invalid period, period should be a positive integer")
+
+	// ErrInvalidReportExt is returned when a report file has an invalid extension.
+	ErrInvalidReportExt = errors.New("invalid report file extension")
+
+	// ErrInvalidReportName is returned when a report file has an invalid name that can't be parsed.
+	ErrInvalidReportName = errors.New("invalid report file name")
+)
+
+// Report represents a report file.
+type Report struct {
+	Path      string // Path is the path to the report.
+	Name      string // Name is the name of the report file, including extension.
+	TimeStamp int64  // TimeStamp is the timestamp of the report.
+
+	reportStash reportStash
+}
+
+// reportStash is a helper struct to store a report and its data for movement.
+type reportStash struct {
+	Path string
+	Data []byte
+}
+
+// New creates a new Report object from a path.
+// It does not write to the file system, or validate the path.
+func New(path string) (Report, error) {
+	if filepath.Ext(path) != constants.ReportExt {
+		return Report{}, ErrInvalidReportExt
+	}
+
+	rTime, err := getReportTime(filepath.Base(path))
+	if err != nil {
+		return Report{}, err
+	}
+
+	return Report{Path: path, Name: filepath.Base(path), TimeStamp: rTime}, nil
+}
+
+// ReadJSON reads the JSON data from the report file.
+func (r Report) ReadJSON() ([]byte, error) {
+	// Read the report file
+	data, err := os.ReadFile(r.Path)
+	if err != nil {
+		return nil, fmt.Errorf("failed to read report file: %v", err)
+	}
+
+	if !json.Valid(data) {
+		return nil, fmt.Errorf("invalid JSON data in report file")
+	}
+
+	return data, nil
+}
+
+// getReportTime returns a int64 representation of the report time from the report path.
+func getReportTime(path string) (int64, error) {
+	fileName := filepath.Base(path)
+	i, err := strconv.ParseInt(strings.TrimSuffix(fileName, filepath.Ext(fileName)), 10, 64)
+	if err != nil {
+		return i, fmt.Errorf("%w: %v", ErrInvalidReportName, err)
+	}
+	return i, nil
+}
+
+// GetPeriodStart returns the start of the period window for a given period in seconds.
+func GetPeriodStart(period int) (int64, error) {
+	if period <= 0 {
+		return 0, ErrInvalidPeriod
+	}
+	utcTime := time.Now().UTC().Unix()
+	return utcTime - (utcTime % int64(period)), nil
+}
+
+// GetForPeriod returns the most recent report within a period window for a given directory.
+// Not inclusive of the period end (periodStart + period).
+//
+// For example, given reports 1 and 7, with time 2 and period 7, the function will return the path for report 1.
+func GetForPeriod(dir string, time time.Time, period int) (Report, error) {
+	if period <= 0 {
+		return Report{}, ErrInvalidPeriod
+	}
+
+	periodStart := time.Unix() - (time.Unix() % int64(period))
+	periodEnd := periodStart + int64(period)
+
+	// Reports names are utc timestamps. Get the most recent report within the period window.
+	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
+		}
+
+		// Skip subdirectories.
+		if d.IsDir() && path != dir {
+			return filepath.SkipDir
+		}
+
+		r, err := New(path)
+		if errors.Is(err, ErrInvalidReportExt) || errors.Is(err, ErrInvalidReportName) {
+			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
+		}
+
+		if r.TimeStamp < periodStart {
+			return nil
+		}
+		if r.TimeStamp >= periodEnd {
+			return filepath.SkipDir
+		}
+
+		report = r
+		return nil
+	})
+
+	if err != nil {
+		return Report{}, err
+	}
+
+	return report, nil
+}
+
+// GetPerPeriod returns the latest report within each period window for a given directory.
+// The key of the map is the start of the period window, and the value is a Report object.
+//
+// If period is 1, then all reports in the dir are returned.
+func GetPerPeriod(dir string, period int) (map[int64]Report, error) {
+	if period <= 0 {
+		return nil, ErrInvalidPeriod
+	}
+
+	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
+		}
+
+		if d.IsDir() && path != dir {
+			return filepath.SkipDir
+		}
+
+		r, err := New(path)
+		if errors.Is(err, ErrInvalidReportExt) || errors.Is(err, ErrInvalidReportName) {
+			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
+		}
+
+		periodStart := r.TimeStamp - (r.TimeStamp % int64(period))
+		if existingReport, ok := reports[periodStart]; !ok || existingReport.TimeStamp < r.TimeStamp {
+			reports[periodStart] = r
+		}
+
+		return nil
+	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return reports, nil
+}
+
+// GetAll returns all reports in a given directory.
+// Reports are expected to have the correct file extension, and have a name which can be parsed by a timestamp.
+// Does not traverse subdirectories.
+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
+		}
+
+		if d.IsDir() && path != dir {
+			return filepath.SkipDir
+		}
+
+		r, err := New(path)
+		if errors.Is(err, ErrInvalidReportExt) || errors.Is(err, ErrInvalidReportName) {
+			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
+		}
+
+		reports = append(reports, r)
+		return nil
+	})
+	if err != nil {
+		return nil, err
+	}
+
+	return reports, nil
+}
diff --git a/internal/reportutils/reportutils_test.go b/internal/report/report_test.go
similarity index 79%
rename from internal/reportutils/reportutils_test.go
rename to internal/report/report_test.go
index 9df7102..96b41be 100644
--- a/internal/reportutils/reportutils_test.go
+++ b/internal/report/report_test.go
@@ -1,12 +1,13 @@
-package reportutils_test
+package report_test
 
 import (
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
-	"github.com/ubuntu/ubuntu-insights/internal/reportutils"
+	report "github.com/ubuntu/ubuntu-insights/internal/report"
 	"github.com/ubuntu/ubuntu-insights/internal/testutils"
 )
 
@@ -20,15 +21,15 @@ func TestGetPeriodStart(t *testing.T) {
 	}{
 		"Valid Period": {period: 500},
 
-		"Invalid Negative Period": {period: -500, wantErr: reportutils.ErrInvalidPeriod},
-		"Invalid Zero Period":     {period: 0, wantErr: reportutils.ErrInvalidPeriod},
+		"Invalid Negative Period": {period: -500, wantErr: report.ErrInvalidPeriod},
+		"Invalid Zero Period":     {period: 0, wantErr: report.ErrInvalidPeriod},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
 			t.Parallel()
 
-			got, err := reportutils.GetPeriodStart(tc.period)
+			got, err := report.GetPeriodStart(tc.period)
 			if tc.wantErr != nil {
 				require.ErrorIs(t, err, tc.wantErr)
 				return
@@ -40,25 +41,28 @@ func TestGetPeriodStart(t *testing.T) {
 	}
 }
 
-func TestGetReportTime(t *testing.T) {
+func TestNew(t *testing.T) {
+	t.Parallel()
+
 	tests := map[string]struct {
 		path string
 
 		wantErr bool
 	}{
-		"Valid Report Time":           {path: "1627847285.json", wantErr: false},
-		"Valid Report Time with Path": {path: "/some/dir/1627847285.json", wantErr: false},
-		"Alt Extension":               {path: "1627847285.txt", wantErr: false},
+		"Valid Report":           {path: "1627847285.json"},
+		"Valid Report with Path": {path: "/some/dir/1627847285.json"},
 
 		"Empty File Name":               {path: ".json", wantErr: true},
 		"Invalid Report Time":           {path: "invalid.json", wantErr: true},
 		"Invalid Report Mixed":          {path: "i-1.json", wantErr: true},
 		"Invalid Report Time with Path": {path: "/123/123/invalid.json", wantErr: true},
+		"Alt Extension":                 {path: "1627847285.txt", wantErr: true},
 	}
 
 	for name, tc := range tests {
 		t.Run(name, func(t *testing.T) {
-			got, err := reportutils.GetReportTime(tc.path)
+			t.Parallel()
+			got, err := report.New(tc.path)
 			if tc.wantErr {
 				require.Error(t, err, "expected an error but got none")
 				return
@@ -66,12 +70,12 @@ func TestGetReportTime(t *testing.T) {
 			require.NoError(t, err, "got an unexpected error")
 
 			want := testutils.LoadWithUpdateFromGoldenYAML(t, got)
-			require.Equal(t, want, got, "GetReportTime should return the report time from the report path")
+			require.Equal(t, want, got, "New should return a new report object")
 		})
 	}
 }
 
-func TestGetReportPath(t *testing.T) {
+func TestGetForPeriod(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
@@ -95,8 +99,8 @@ func TestGetReportPath(t *testing.T) {
 		"Negative Timestamp":                {files: []string{"-100.json", "-101.json"}, time: -150, period: 100},
 		"Not Inclusive Period":              {files: []string{"1.json", "7.json"}, time: 2, period: 7},
 
-		"Invalid Negative Period": {files: []string{"1.json", "7.json"}, time: 2, period: -7, wantSpecificErr: reportutils.ErrInvalidPeriod},
-		"Invalid Zero Period":     {files: []string{"1.json", "7.json"}, time: 2, period: 0, wantSpecificErr: reportutils.ErrInvalidPeriod},
+		"Invalid Negative Period": {files: []string{"1.json", "7.json"}, time: 2, period: -7, wantSpecificErr: report.ErrInvalidPeriod},
+		"Invalid Zero Period":     {files: []string{"1.json", "7.json"}, time: 2, period: 0, wantSpecificErr: report.ErrInvalidPeriod},
 
 		"Invalid Dir": {period: 1, invalidDir: true, wantGenericErr: true},
 	}
@@ -107,12 +111,11 @@ func TestGetReportPath(t *testing.T) {
 
 			dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles)
 			require.NoError(t, err, "Setup: failed to setup temporary directory")
-			defer os.RemoveAll(dir)
 			if tc.invalidDir {
 				dir = filepath.Join(dir, "invalid dir")
 			}
 
-			got, err := reportutils.GetReportPath(dir, tc.time, tc.period)
+			r, err := report.GetForPeriod(dir, time.Unix(tc.time, 0), tc.period)
 			if tc.wantSpecificErr != nil {
 				require.ErrorIs(t, err, tc.wantSpecificErr)
 				return
@@ -121,21 +124,17 @@ func TestGetReportPath(t *testing.T) {
 				require.Error(t, err, "expected an error but got none")
 				return
 			}
-
 			require.NoError(t, err, "got an unexpected error")
 
-			if got != "" {
-				got, err = filepath.Rel(dir, got)
-				require.NoError(t, err, "failed to get relative path")
-			}
+			got := sanitizeReportPath(t, r, dir)
 
-			want := testutils.LoadWithUpdateFromGolden(t, got)
+			want := testutils.LoadWithUpdateFromGoldenYAML(t, got)
 			require.Equal(t, want, got, "GetReportPath should return the most recent report within the period window")
 		})
 	}
 }
 
-func TestGetReports(t *testing.T) {
+func TestGetPerPeriod(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
@@ -160,8 +159,8 @@ func TestGetReports(t *testing.T) {
 		"Multiple Non-Consecutive Windows": {files: []string{"1.json", "7.json", "101.json", "107.json", "251.json", "257.json"}, period: 50},
 		"Get All Reports":                  {files: []string{"1.json", "2.json", "3.json", "101.json", "107.json", "251.json", "257.json"}, period: 1},
 
-		"Invalid Negative Period": {files: []string{"1.json", "7.json"}, period: -7, wantSpecificErr: reportutils.ErrInvalidPeriod},
-		"Invalid Zero Period":     {files: []string{"1.json", "7.json"}, period: 0, wantSpecificErr: reportutils.ErrInvalidPeriod},
+		"Invalid Negative Period": {files: []string{"1.json", "7.json"}, period: -7, wantSpecificErr: report.ErrInvalidPeriod},
+		"Invalid Zero Period":     {files: []string{"1.json", "7.json"}, period: 0, wantSpecificErr: report.ErrInvalidPeriod},
 
 		"Invalid Dir": {period: 1, invalidDir: true, wantGenericErr: true},
 	}
@@ -172,12 +171,11 @@ func TestGetReports(t *testing.T) {
 
 			dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles)
 			require.NoError(t, err, "Setup: failed to setup temporary directory")
-			defer os.RemoveAll(dir)
 			if tc.invalidDir {
 				dir = filepath.Join(dir, "invalid dir")
 			}
 
-			got, err := reportutils.GetReports(dir, tc.period)
+			reports, err := report.GetPerPeriod(dir, tc.period)
 			if tc.wantSpecificErr != nil {
 				require.ErrorIs(t, err, tc.wantSpecificErr)
 				return
@@ -188,13 +186,17 @@ func TestGetReports(t *testing.T) {
 			}
 			require.NoError(t, err, "got an unexpected error")
 
+			got := make(map[int64]report.Report, len(reports))
+			for n, r := range reports {
+				got[n] = sanitizeReportPath(t, r, dir)
+			}
 			want := testutils.LoadWithUpdateFromGoldenYAML(t, got)
 			require.Equal(t, want, got, "GetReports should return the most recent report within each period window")
 		})
 	}
 }
 
-func TestGetAllReports(t *testing.T) {
+func TestGetAll(t *testing.T) {
 	t.Parallel()
 
 	tests := map[string]struct {
@@ -219,18 +221,21 @@ func TestGetAllReports(t *testing.T) {
 			t.Parallel()
 			dir, err := setupTmpDir(t, tc.files, tc.subDir, tc.subDirFiles)
 			require.NoError(t, err, "Setup: failed to setup temporary directory")
-			defer os.RemoveAll(dir)
 			if tc.invalidDir {
 				dir = filepath.Join(dir, "invalid dir")
 			}
 
-			got, err := reportutils.GetAllReports(dir)
+			reports, err := report.GetAll(dir)
 			if tc.wantErr {
 				require.Error(t, err, "expected an error but got none")
 				return
 			}
 			require.NoError(t, err, "got an unexpected error")
 
+			got := make([]report.Report, len(reports))
+			for _, r := range reports {
+				got = append(got, sanitizeReportPath(t, r, dir))
+			}
 			want := testutils.LoadWithUpdateFromGoldenYAML(t, got)
 			require.Equal(t, want, got, "GetAllReports should return all reports in the directory")
 		})
@@ -240,10 +245,7 @@ func TestGetAllReports(t *testing.T) {
 func setupTmpDir(t *testing.T, files []string, subDir string, subDirFiles []string) (string, error) {
 	t.Helper()
 
-	dir, err := os.MkdirTemp("", "reportutils-test")
-	if err != nil {
-		return "", err
-	}
+	dir := t.TempDir()
 
 	for _, file := range files {
 		path := filepath.Join(dir, file)
@@ -268,3 +270,17 @@ func setupTmpDir(t *testing.T, files []string, subDir string, subDirFiles []stri
 
 	return dir, nil
 }
+
+func sanitizeReportPath(t *testing.T, r report.Report, dir string) report.Report {
+	t.Helper()
+	if r.Path == "" {
+		return r
+	}
+
+	fp, err := filepath.Rel(dir, r.Path)
+	if err != nil {
+		require.NoError(t, err, "failed to get relative path")
+		return report.Report{}
+	}
+	return report.Report{Path: fp, Name: r.Name, TimeStamp: r.TimeStamp}
+}
diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/empty_directory b/internal/report/testdata/TestGetAll/golden/empty_directory
similarity index 100%
rename from internal/reportutils/testdata/TestGetAllReports/golden/empty_directory
rename to internal/report/testdata/TestGetAll/golden/empty_directory
diff --git a/internal/report/testdata/TestGetAll/golden/files_in_subdir b/internal/report/testdata/TestGetAll/golden/files_in_subdir
new file mode 100644
index 0000000..ccc5761
--- /dev/null
+++ b/internal/report/testdata/TestGetAll/golden/files_in_subdir
@@ -0,0 +1,12 @@
+- path: ""
+  name: ""
+  timestamp: 0
+- path: ""
+  name: ""
+  timestamp: 0
+- path: 1.json
+  name: 1.json
+  timestamp: 1
+- path: 2.json
+  name: 2.json
+  timestamp: 2
diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_extension b/internal/report/testdata/TestGetAll/golden/invalid_file_extension
similarity index 100%
rename from internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_extension
rename to internal/report/testdata/TestGetAll/golden/invalid_file_extension
diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_names b/internal/report/testdata/TestGetAll/golden/invalid_file_names
similarity index 100%
rename from internal/reportutils/testdata/TestGetAllReports/golden/invalid_file_names
rename to internal/report/testdata/TestGetAll/golden/invalid_file_names
diff --git a/internal/report/testdata/TestGetAll/golden/mix_of_valid_and_invalid b/internal/report/testdata/TestGetAll/golden/mix_of_valid_and_invalid
new file mode 100644
index 0000000..e2ad51f
--- /dev/null
+++ b/internal/report/testdata/TestGetAll/golden/mix_of_valid_and_invalid
@@ -0,0 +1,18 @@
+- path: ""
+  name: ""
+  timestamp: 0
+- path: ""
+  name: ""
+  timestamp: 0
+- path: ""
+  name: ""
+  timestamp: 0
+- path: 1.json
+  name: 1.json
+  timestamp: 1
+- path: 2.json
+  name: 2.json
+  timestamp: 2
+- path: 500.json
+  name: 500.json
+  timestamp: 500
diff --git a/internal/report/testdata/TestGetForPeriod/golden/empty_directory b/internal/report/testdata/TestGetForPeriod/golden/empty_directory
new file mode 100644
index 0000000..ec9107d
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/empty_directory
@@ -0,0 +1,3 @@
+path: ""
+name: ""
+timestamp: 0
diff --git a/internal/report/testdata/TestGetForPeriod/golden/empty_subdir b/internal/report/testdata/TestGetForPeriod/golden/empty_subdir
new file mode 100644
index 0000000..ec9107d
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/empty_subdir
@@ -0,0 +1,3 @@
+path: ""
+name: ""
+timestamp: 0
diff --git a/internal/report/testdata/TestGetForPeriod/golden/files_in_subdir b/internal/report/testdata/TestGetForPeriod/golden/files_in_subdir
new file mode 100644
index 0000000..ec9107d
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/files_in_subdir
@@ -0,0 +1,3 @@
+path: ""
+name: ""
+timestamp: 0
diff --git a/internal/report/testdata/TestGetForPeriod/golden/invalid_file_extension b/internal/report/testdata/TestGetForPeriod/golden/invalid_file_extension
new file mode 100644
index 0000000..ec9107d
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/invalid_file_extension
@@ -0,0 +1,3 @@
+path: ""
+name: ""
+timestamp: 0
diff --git a/internal/report/testdata/TestGetForPeriod/golden/invalid_file_names b/internal/report/testdata/TestGetForPeriod/golden/invalid_file_names
new file mode 100644
index 0000000..ec9107d
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/invalid_file_names
@@ -0,0 +1,3 @@
+path: ""
+name: ""
+timestamp: 0
diff --git a/internal/report/testdata/TestGetForPeriod/golden/negative_timestamp b/internal/report/testdata/TestGetForPeriod/golden/negative_timestamp
new file mode 100644
index 0000000..431aa05
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/negative_timestamp
@@ -0,0 +1,3 @@
+path: -100.json
+name: -100.json
+timestamp: -100
diff --git a/internal/report/testdata/TestGetForPeriod/golden/not_inclusive_period b/internal/report/testdata/TestGetForPeriod/golden/not_inclusive_period
new file mode 100644
index 0000000..56c8ba5
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/not_inclusive_period
@@ -0,0 +1,3 @@
+path: 1.json
+name: 1.json
+timestamp: 1
diff --git a/internal/report/testdata/TestGetForPeriod/golden/specific_time_single_valid_report b/internal/report/testdata/TestGetForPeriod/golden/specific_time_single_valid_report
new file mode 100644
index 0000000..7c4d2c9
--- /dev/null
+++ b/internal/report/testdata/TestGetForPeriod/golden/specific_time_single_valid_report
@@ -0,0 +1,3 @@
+path: 2.json
+name: 2.json
+timestamp: 2
diff --git a/internal/reportutils/testdata/TestGetReports/golden/empty_directory b/internal/report/testdata/TestGetPerPeriod/golden/empty_directory
similarity index 100%
rename from internal/reportutils/testdata/TestGetReports/golden/empty_directory
rename to internal/report/testdata/TestGetPerPeriod/golden/empty_directory
diff --git a/internal/reportutils/testdata/TestGetReports/golden/files_in_subdir b/internal/report/testdata/TestGetPerPeriod/golden/files_in_subdir
similarity index 100%
rename from internal/reportutils/testdata/TestGetReports/golden/files_in_subdir
rename to internal/report/testdata/TestGetPerPeriod/golden/files_in_subdir
diff --git a/internal/report/testdata/TestGetPerPeriod/golden/get_all_reports b/internal/report/testdata/TestGetPerPeriod/golden/get_all_reports
new file mode 100644
index 0000000..e3e067a
--- /dev/null
+++ b/internal/report/testdata/TestGetPerPeriod/golden/get_all_reports
@@ -0,0 +1,28 @@
+1:
+    path: 1.json
+    name: 1.json
+    timestamp: 1
+2:
+    path: 2.json
+    name: 2.json
+    timestamp: 2
+3:
+    path: 3.json
+    name: 3.json
+    timestamp: 3
+101:
+    path: 101.json
+    name: 101.json
+    timestamp: 101
+107:
+    path: 107.json
+    name: 107.json
+    timestamp: 107
+251:
+    path: 251.json
+    name: 251.json
+    timestamp: 251
+257:
+    path: 257.json
+    name: 257.json
+    timestamp: 257
diff --git a/internal/report/testdata/TestGetPerPeriod/golden/get_newest_of_period b/internal/report/testdata/TestGetPerPeriod/golden/get_newest_of_period
new file mode 100644
index 0000000..d6fdc70
--- /dev/null
+++ b/internal/report/testdata/TestGetPerPeriod/golden/get_newest_of_period
@@ -0,0 +1,4 @@
+0:
+    path: 7.json
+    name: 7.json
+    timestamp: 7
diff --git a/internal/reportutils/testdata/TestGetReports/golden/invalid_file_extension b/internal/report/testdata/TestGetPerPeriod/golden/invalid_file_extension
similarity index 100%
rename from internal/reportutils/testdata/TestGetReports/golden/invalid_file_extension
rename to internal/report/testdata/TestGetPerPeriod/golden/invalid_file_extension
diff --git a/internal/reportutils/testdata/TestGetReports/golden/invalid_file_names b/internal/report/testdata/TestGetPerPeriod/golden/invalid_file_names
similarity index 100%
rename from internal/reportutils/testdata/TestGetReports/golden/invalid_file_names
rename to internal/report/testdata/TestGetPerPeriod/golden/invalid_file_names
diff --git a/internal/report/testdata/TestGetPerPeriod/golden/mix_of_valid_and_invalid b/internal/report/testdata/TestGetPerPeriod/golden/mix_of_valid_and_invalid
new file mode 100644
index 0000000..5f45d53
--- /dev/null
+++ b/internal/report/testdata/TestGetPerPeriod/golden/mix_of_valid_and_invalid
@@ -0,0 +1,4 @@
+0:
+    path: 2.json
+    name: 2.json
+    timestamp: 2
diff --git a/internal/report/testdata/TestGetPerPeriod/golden/multiple_consecutive_windows b/internal/report/testdata/TestGetPerPeriod/golden/multiple_consecutive_windows
new file mode 100644
index 0000000..d777da2
--- /dev/null
+++ b/internal/report/testdata/TestGetPerPeriod/golden/multiple_consecutive_windows
@@ -0,0 +1,12 @@
+0:
+    path: 7.json
+    name: 7.json
+    timestamp: 7
+100:
+    path: 107.json
+    name: 107.json
+    timestamp: 107
+200:
+    path: 207.json
+    name: 207.json
+    timestamp: 207
diff --git a/internal/report/testdata/TestGetPerPeriod/golden/multiple_non-consecutive_windows b/internal/report/testdata/TestGetPerPeriod/golden/multiple_non-consecutive_windows
new file mode 100644
index 0000000..9fb936e
--- /dev/null
+++ b/internal/report/testdata/TestGetPerPeriod/golden/multiple_non-consecutive_windows
@@ -0,0 +1,12 @@
+0:
+    path: 7.json
+    name: 7.json
+    timestamp: 7
+100:
+    path: 107.json
+    name: 107.json
+    timestamp: 107
+250:
+    path: 257.json
+    name: 257.json
+    timestamp: 257
diff --git a/internal/report/testdata/TestNew/golden/valid_report b/internal/report/testdata/TestNew/golden/valid_report
new file mode 100644
index 0000000..7ce2121
--- /dev/null
+++ b/internal/report/testdata/TestNew/golden/valid_report
@@ -0,0 +1,3 @@
+path: 1627847285.json
+name: 1627847285.json
+timestamp: 1627847285
diff --git a/internal/report/testdata/TestNew/golden/valid_report_with_path b/internal/report/testdata/TestNew/golden/valid_report_with_path
new file mode 100644
index 0000000..252b842
--- /dev/null
+++ b/internal/report/testdata/TestNew/golden/valid_report_with_path
@@ -0,0 +1,3 @@
+path: /some/dir/1627847285.json
+name: 1627847285.json
+timestamp: 1627847285
diff --git a/internal/reportutils/reportutils.go b/internal/reportutils/reportutils.go
deleted file mode 100644
index 932d99c..0000000
--- a/internal/reportutils/reportutils.go
+++ /dev/null
@@ -1,177 +0,0 @@
-// Package reportutils provides utility functions for handling reports.
-package reportutils
-
-// package report
-
-import (
-	"errors"
-	"log/slog"
-	"os"
-	"path/filepath"
-	"strconv"
-	"strings"
-	"time"
-
-	"github.com/ubuntu/ubuntu-insights/internal/constants"
-)
-
-// ErrInvalidPeriod is returned when a function requiring a period, received an invalid, period that isn't a non-negative integer.
-var ErrInvalidPeriod = errors.New("invalid period, period should be a positive integer")
-
-// type Report struct{}
-
-// GetPeriodStart returns the start of the period window for a given period in seconds.
-func GetPeriodStart(period int) (int64, error) {
-	if period <= 0 {
-		return 0, ErrInvalidPeriod
-	}
-	utcTime := time.Now().UTC().Unix()
-	return utcTime - (utcTime % int64(period)), nil
-}
-
-// GetReportTime returns a int64 representation of the report time from the report path.
-func GetReportTime(path string) (int64, error) {
-	fileName := filepath.Base(path)
-	return strconv.ParseInt(strings.TrimSuffix(fileName, filepath.Ext(fileName)), 10, 64)
-}
-
-// Path is a method on Report
-
-// GetReportPath returns the path for the most recent report within a period window, returning an empty string if no report is found.
-// Not inclusive of the period end (periodStart + period).
-//
-// For example, given reports 1 and 7, with time 2 and period 7, the function will return the path for report 1.
-func GetReportPath(dir string, time int64, period int) (string, error) {
-	if period <= 0 {
-		return "", ErrInvalidPeriod
-	}
-
-	periodStart := time - (time % int64(period))
-	periodEnd := periodStart + int64(period)
-
-	// Reports names are utc timestamps. Get the most recent report within the period window.
-	var mostRecentReportPath string
-	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
-		}
-
-		// Skip subdirectories.
-		if d.IsDir() && path != dir {
-			return filepath.SkipDir
-		}
-
-		if filepath.Ext(d.Name()) != constants.ReportExt {
-			slog.Info("Skipping non-report file, invalid extension", "file", d.Name())
-			return nil
-		}
-
-		reportTime, err := GetReportTime(d.Name())
-		if err != nil {
-			slog.Info("Skipping non-report file, invalid file name", "file", d.Name())
-			return nil
-		}
-
-		if reportTime < periodStart {
-			return nil
-		}
-		if reportTime >= periodEnd {
-			return filepath.SkipDir
-		}
-
-		mostRecentReportPath = path
-		return nil
-	})
-
-	if err != nil {
-		return "", err
-	}
-
-	return mostRecentReportPath, nil
-}
-
-// -> map[int64]Report
-
-// GetReports returns the paths for the latest report within each period window for a given directory.
-// The key of the map is the start of the period window, and the report timestamp.
-//
-// If period is 1, then all reports in the dir are returned.
-func GetReports(dir string, period int) (map[int64]int64, error) {
-	if period <= 0 {
-		return nil, ErrInvalidPeriod
-	}
-
-	reports := make(map[int64]int64)
-	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
-		}
-
-		if d.IsDir() && path != dir {
-			return filepath.SkipDir
-		}
-
-		if filepath.Ext(d.Name()) != constants.ReportExt {
-			slog.Info("Skipping non-report file, invalid extension", "file", d.Name())
-			return nil
-		}
-
-		reportTime, err := GetReportTime(d.Name())
-		if err != nil {
-			slog.Info("Skipping non-report file, invalid file name", "file", d.Name())
-			return nil
-		}
-
-		periodStart := reportTime - (reportTime % int64(period))
-		if existingReport, ok := reports[periodStart]; !ok || existingReport < reportTime {
-			reports[periodStart] = reportTime
-		}
-
-		return nil
-	})
-
-	if err != nil {
-		return nil, err
-	}
-
-	return reports, nil
-}
-
-// reports.GetAll(dir string) ([]Report, error)
-
-// GetAllReports returns the filename for all reports within a given directory, which match the expected pattern.
-// Does not traverse subdirectories.
-func GetAllReports(dir string) ([]string, error) {
-	reports := make([]string, 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
-		}
-
-		if d.IsDir() && path != dir {
-			return filepath.SkipDir
-		}
-
-		if filepath.Ext(d.Name()) != constants.ReportExt {
-			slog.Info("Skipping non-report file, invalid extension", "file", d.Name())
-			return nil
-		}
-
-		if _, err := GetReportTime(d.Name()); err != nil {
-			slog.Info("Skipping non-report file, invalid file name", "file", d.Name())
-			return nil
-		}
-
-		reports = append(reports, d.Name())
-		return nil
-	})
-
-	if err != nil {
-		return nil, err
-	}
-
-	return reports, nil
-}
diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir b/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir
deleted file mode 100644
index 6548887..0000000
--- a/internal/reportutils/testdata/TestGetAllReports/golden/files_in_subdir
+++ /dev/null
@@ -1,2 +0,0 @@
-- 1.json
-- 2.json
diff --git a/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid b/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid
deleted file mode 100644
index a8eb048..0000000
--- a/internal/reportutils/testdata/TestGetAllReports/golden/mix_of_valid_and_invalid
+++ /dev/null
@@ -1,3 +0,0 @@
-- 1.json
-- 2.json
-- 500.json
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/empty_directory b/internal/reportutils/testdata/TestGetReportPath/golden/empty_directory
deleted file mode 100644
index e69de29..0000000
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/empty_subdir b/internal/reportutils/testdata/TestGetReportPath/golden/empty_subdir
deleted file mode 100644
index e69de29..0000000
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/files_in_subdir b/internal/reportutils/testdata/TestGetReportPath/golden/files_in_subdir
deleted file mode 100644
index e69de29..0000000
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_extension b/internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_extension
deleted file mode 100644
index e69de29..0000000
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_names b/internal/reportutils/testdata/TestGetReportPath/golden/invalid_file_names
deleted file mode 100644
index e69de29..0000000
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/negative_timestamp b/internal/reportutils/testdata/TestGetReportPath/golden/negative_timestamp
deleted file mode 100644
index 411e61e..0000000
--- a/internal/reportutils/testdata/TestGetReportPath/golden/negative_timestamp
+++ /dev/null
@@ -1 +0,0 @@
--100.json
\ No newline at end of file
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/not_inclusive_period b/internal/reportutils/testdata/TestGetReportPath/golden/not_inclusive_period
deleted file mode 100644
index a3d3d4c..0000000
--- a/internal/reportutils/testdata/TestGetReportPath/golden/not_inclusive_period
+++ /dev/null
@@ -1 +0,0 @@
-1.json
\ No newline at end of file
diff --git a/internal/reportutils/testdata/TestGetReportPath/golden/specific_time_single_valid_report b/internal/reportutils/testdata/TestGetReportPath/golden/specific_time_single_valid_report
deleted file mode 100644
index e1f33a0..0000000
--- a/internal/reportutils/testdata/TestGetReportPath/golden/specific_time_single_valid_report
+++ /dev/null
@@ -1 +0,0 @@
-2.json
\ No newline at end of file
diff --git a/internal/reportutils/testdata/TestGetReportTime/golden/alt_extension b/internal/reportutils/testdata/TestGetReportTime/golden/alt_extension
deleted file mode 100644
index d29665a..0000000
--- a/internal/reportutils/testdata/TestGetReportTime/golden/alt_extension
+++ /dev/null
@@ -1 +0,0 @@
-1627847285
diff --git a/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time b/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time
deleted file mode 100644
index d29665a..0000000
--- a/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time
+++ /dev/null
@@ -1 +0,0 @@
-1627847285
diff --git a/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time_with_path b/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time_with_path
deleted file mode 100644
index d29665a..0000000
--- a/internal/reportutils/testdata/TestGetReportTime/golden/valid_report_time_with_path
+++ /dev/null
@@ -1 +0,0 @@
-1627847285
diff --git a/internal/reportutils/testdata/TestGetReports/golden/get_all_reports b/internal/reportutils/testdata/TestGetReports/golden/get_all_reports
deleted file mode 100644
index e295670..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/get_all_reports
+++ /dev/null
@@ -1,7 +0,0 @@
-1: 1
-2: 2
-3: 3
-101: 101
-107: 107
-251: 251
-257: 257
diff --git a/internal/reportutils/testdata/TestGetReports/golden/get_newest_of_period b/internal/reportutils/testdata/TestGetReports/golden/get_newest_of_period
deleted file mode 100644
index 017ba7a..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/get_newest_of_period
+++ /dev/null
@@ -1 +0,0 @@
-0: 7
diff --git a/internal/reportutils/testdata/TestGetReports/golden/mix_of_valid_and_invalid b/internal/reportutils/testdata/TestGetReports/golden/mix_of_valid_and_invalid
deleted file mode 100644
index f196b73..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/mix_of_valid_and_invalid
+++ /dev/null
@@ -1 +0,0 @@
-0: 2
diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows
deleted file mode 100644
index 8e9a957..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/multiple_consecutive_windows
+++ /dev/null
@@ -1,3 +0,0 @@
-0: 7
-100: 107
-200: 207
diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_consequtive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_consequtive_windows
deleted file mode 100644
index 8e9a957..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/multiple_consequtive_windows
+++ /dev/null
@@ -1,3 +0,0 @@
-0: 7
-100: 107
-200: 207
diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows
deleted file mode 100644
index f2cb19d..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consecutive_windows
+++ /dev/null
@@ -1,3 +0,0 @@
-0: 7
-100: 107
-250: 257
diff --git a/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consequtive_windows b/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consequtive_windows
deleted file mode 100644
index f2cb19d..0000000
--- a/internal/reportutils/testdata/TestGetReports/golden/multiple_non-consequtive_windows
+++ /dev/null
@@ -1,3 +0,0 @@
-0: 7
-100: 107
-250: 257
diff --git a/internal/uploader/internal_test.go b/internal/uploader/internal_test.go
index f9882d3..27be531 100644
--- a/internal/uploader/internal_test.go
+++ b/internal/uploader/internal_test.go
@@ -10,6 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/ubuntu/ubuntu-insights/internal/constants"
 	"github.com/ubuntu/ubuntu-insights/internal/fileutils"
+	"github.com/ubuntu/ubuntu-insights/internal/report"
 )
 
 func TestUploadBadFile(t *testing.T) {
@@ -25,21 +26,22 @@ func TestUploadBadFile(t *testing.T) {
 		url          string
 		consent      bool
 
+		rNewErr bool
 		wantErr bool
 	}{
 		"Ok":           {fName: "0.json", fileContents: basicContent, wantErr: false},
 		"Missing File": {fName: "0.json", fileContents: basicContent, missingFile: true, wantErr: true},
 		"File Is Dir":  {fName: "0.json", fileIsDir: true, wantErr: true},
-		"Non-numeric":  {fName: "not-numeric.json", fileContents: basicContent, wantErr: true},
-		"Bad File Ext": {fName: "0.txt", fileContents: basicContent},
+		"Non-numeric":  {fName: "not-numeric.json", fileContents: basicContent, rNewErr: true},
+		"Bad File Ext": {fName: "0.txt", fileContents: basicContent, rNewErr: true},
 		"Bad Contents": {fName: "0.json", fileContents: badContent, wantErr: true},
 		"Bad URL":      {fName: "0.json", fileContents: basicContent, url: "http://bad host:1234", wantErr: true},
 
 		"Ok Consent":           {fName: "0.json", fileContents: basicContent, consent: true, wantErr: false},
 		"Missing File Consent": {fName: "0.json", fileContents: basicContent, missingFile: true, consent: true, wantErr: true},
 		"File Is Dir Consent":  {fName: "0.json", fileIsDir: true, consent: true, wantErr: true},
-		"Non-numeric Consent":  {fName: "not-numeric.json", fileContents: basicContent, consent: true, wantErr: true},
-		"Bad File Ext Consent": {fName: "0.txt", fileContents: basicContent, consent: true},
+		"Non-numeric Consent":  {fName: "not-numeric.json", fileContents: basicContent, consent: true, rNewErr: true},
+		"Bad File Ext Consent": {fName: "0.txt", fileContents: basicContent, consent: true, rNewErr: true},
 		"Bad Contents Consent": {fName: "0.json", fileContents: badContent, consent: true, wantErr: true},
 		"Bad URL Consent":      {fName: "0.json", fileContents: basicContent, url: "http://bad host:1234", consent: true, wantErr: true},
 	}
@@ -77,8 +79,13 @@ func TestUploadBadFile(t *testing.T) {
 			if tc.url == "" {
 				tc.url = ts.URL
 			}
-
-			err := um.upload(tc.fName, tc.url, tc.consent, false)
+			r, err := report.New(fPath)
+			if tc.rNewErr {
+				require.Error(t, err, "Setup: failed to create report object")
+				return
+			}
+			require.NoError(t, err, "Setup: failed to create report object")
+			err = um.upload(r, tc.url, tc.consent, false)
 			if tc.wantErr {
 				require.Error(t, err)
 				return
diff --git a/internal/uploader/upload.go b/internal/uploader/upload.go
index 2f181e3..cb04d46 100644
--- a/internal/uploader/upload.go
+++ b/internal/uploader/upload.go
@@ -16,7 +16,7 @@ import (
 
 	"github.com/ubuntu/ubuntu-insights/internal/constants"
 	"github.com/ubuntu/ubuntu-insights/internal/fileutils"
-	"github.com/ubuntu/ubuntu-insights/internal/reportutils"
+	"github.com/ubuntu/ubuntu-insights/internal/report"
 )
 
 var (
@@ -34,7 +34,7 @@ func (um Uploader) Upload(force bool) error {
 		return fmt.Errorf("upload failed to get consent state: %v", err)
 	}
 
-	reports, err := reportutils.GetAllReports(um.collectedDir)
+	reports, err := report.GetAll(um.collectedDir)
 	if err != nil {
 		return fmt.Errorf("failed to get reports: %v", err)
 	}
@@ -45,17 +45,17 @@ func (um Uploader) Upload(force bool) error {
 	}
 
 	var wg sync.WaitGroup
-	for _, name := range reports {
+	for _, r := range reports {
 		wg.Add(1)
-		go func(name string) {
+		go func(r report.Report) {
 			defer wg.Done()
-			err := um.upload(name, url, consent, force)
+			err := um.upload(r, url, consent, force)
 			if errors.Is(err, ErrReportNotMature) {
-				slog.Debug("Skipped report upload, not mature enough", "file", name, "source", um.source)
+				slog.Debug("Skipped report upload, not mature enough", "file", r.Name, "source", um.source)
 			} else if err != nil {
-				slog.Warn("Failed to upload report", "file", name, "source", um.source, "error", err)
+				slog.Warn("Failed to upload report", "file", r.Name, "source", um.source, "error", err)
 			}
-		}(name)
+		}(r)
 	}
 	wg.Wait()
 
@@ -64,21 +64,15 @@ func (um Uploader) Upload(force bool) error {
 
 // upload uploads an individual report to the server. It returns an error if the report is not mature enough to be uploaded, or if the upload fails.
 // It also moves the report to the uploaded directory after a successful upload.
-func (um Uploader) upload(name, url string, consent, force bool) error {
-	slog.Debug("Uploading report", "file", name, "consent", consent, "force", force)
+func (um Uploader) upload(r report.Report, url string, consent, force bool) error {
+	slog.Debug("Uploading report", "file", r.Name, "consent", consent, "force", force)
 
-	// TODO… pass the Report object directly.
-	ts, err := reportutils.GetReportTime(name)
-	if err != nil {
-		return fmt.Errorf("failed to parse report time from filename: %v", err)
-	}
-
-	if um.timeProvider.Now().Add(time.Duration(-um.minAge)*time.Second).Before(time.Unix(ts, 0)) && !force {
+	if um.timeProvider.Now().Add(time.Duration(-um.minAge)*time.Second).Before(time.Unix(r.TimeStamp, 0)) && !force {
 		return ErrReportNotMature
 	}
 
 	// Check for duplicate reports.
-	_, err = os.Stat(filepath.Join(um.uploadedDir, name))
+	_, err := os.Stat(filepath.Join(um.uploadedDir, r.Name))
 	if err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("failed to check if report has already been uploaded: %v", err)
 	}
@@ -88,9 +82,9 @@ func (um Uploader) upload(name, url string, consent, force bool) error {
 		return fmt.Errorf("report has already been uploaded")
 	}
 
-	origData, err := um.readJSON(name)
+	origData, err := r.ReadJSON()
 	if err != nil {
-		return fmt.Errorf("failed to get payload: %v", err)
+		return fmt.Errorf("failed to read report: %v", err)
 	}
 	data := origData
 	if !consent {
@@ -108,11 +102,11 @@ func (um Uploader) upload(name, url string, consent, force bool) error {
 
 	// 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, name), filepath.Join(um.collectedDir, name), data); err != nil {
+	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)
 	}
 	if err := send(url, data); err != nil {
-		if moveErr := um.moveReport(filepath.Join(um.collectedDir, name), filepath.Join(um.uploadedDir, name), origData); moveErr != 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)
 		}
 		return fmt.Errorf("failed to send data: %v", err)
@@ -130,21 +124,6 @@ func (um Uploader) getURL() (string, error) {
 	return u.String(), nil
 }
 
-// readJSON reads the JSON data from the report file.
-func (um Uploader) readJSON(file string) ([]byte, error) {
-	// Read the report file
-	data, err := os.ReadFile(path.Join(um.collectedDir, file))
-	if err != nil {
-		return nil, fmt.Errorf("failed to read report file: %v", err)
-	}
-
-	if !json.Valid(data) {
-		return nil, fmt.Errorf("invalid JSON data in report file")
-	}
-
-	return data, nil
-}
-
 func (um Uploader) moveReport(writePath, removePath string, data []byte) error {
 	// (Report).MarkAsProcessed(data)
 	// (Report).UndoProcessed

From 42ce4460c2d38f5d0e4c18fe7bd5a974251a866e 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 4/4] 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                   |  32 +-
 15 files changed, 422 insertions(+), 108 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..9fcdec9 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"
 )
 
@@ -25,7 +24,9 @@ var (
 )
 
 // Upload uploads the reports corresponding to the source to the configured server.
-// Does not do duplicate checks.
+//
+// It will only upload reports that are mature enough, and have not been uploaded before.
+// If force is true, maturity and duplicate check will be skipped.
 func (um Uploader) Upload(force bool) error {
 	slog.Debug("Uploading reports")
 
@@ -101,13 +102,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 +125,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))