Improve uploader tests

internal/constants/constants.go

@@ -3,6 +3,7 @@
 package constants
 
 import (
+	"encoding/json"
 	"log/slog"
 	"os"
 	"path/filepath"
@@ -38,7 +39,7 @@ const (
 )
 
 // OptOutJSON is the data sent in case of Opt-Out choice.
-var OptOutJSON = struct{ OptOut bool }{OptOut: true}
+var OptOutJSON = json.RawMessage(`{"OptOut": true}`)
 
 type options struct {
 	baseDir func() (string, error)

internal/testutils/files.go

@@ -3,7 +3,10 @@
 package testutils
 
 import (
+	"bytes"
+	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
@@ -52,3 +55,46 @@ func CopyDir(srcDir, dstDir string) error {
 		return CopyFile(path, dstPath)
 	})
 }
+
+// GetDirContents returns the contents of a directory as a map of file paths to file contents.
+// The contents are read as strings.
+// The maxDepth parameter limits the depth of the directory tree to read.
+func GetDirContents(t *testing.T, dir string, maxDepth uint) (map[string]string, error) {
+	t.Helper()
+
+	files := make(map[string]string)
+
+	err := filepath.WalkDir(dir, func(path string, d fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+
+		if path == dir {
+			return nil
+		}
+
+		relPath, err := filepath.Rel(dir, path)
+		if err != nil {
+			return err
+		}
+
+		depth := uint(len(filepath.SplitList(relPath)))
+		if depth > maxDepth {
+			return fmt.Errorf("max depth %d exceeded at %s", maxDepth, relPath)
+		}
+
+		if !d.IsDir() {
+			content, err := os.ReadFile(path)
+			if err != nil {
+				return err
+			}
+			// Normalize content between Windows and Linux
+			content = bytes.ReplaceAll(content, []byte("\r\n"), []byte("\n"))
+			files[filepath.ToSlash(relPath)] = string(content)
+		}
+
+		return nil
+	})
+
+	return files, err
+}

internal/uploader/internal_test.go

@@ -1 +1,192 @@
 package uploader
+
+import (
+	"math"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/ubuntu/ubuntu-insights/internal/constants"
+	"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()
+
+	tests := map[string]struct {
+		fName        string
+		fileContents string
+		missingFile  bool
+		fileIsDir    bool
+		url          string
+		consent      bool
+		minAge       uint
+
+		wantErr bool
+	}{
+		"Ok":              {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, wantErr: false},
+		"Missing File":    {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, missingFile: true, wantErr: true},
+		"File Is Dir":     {fName: "0.json", fileIsDir: true},
+		"Non-numeric":     {fName: "not-numeric.json", fileContents: `{"Content":true, "string": "string"}`, wantErr: true},
+		"Bad File Ext":    {fName: "0.txt", fileContents: `{"Content":true, "string": "string"}`},
+		"Bad Contents":    {fName: "0.json", fileContents: `bad content`},
+		"minAge Overflow": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, minAge: math.MaxUint64, wantErr: true},
+		"Bad URL":         {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, url: "http://bad host:1234", wantErr: true},
+
+		"Ok Consent":              {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, consent: true, wantErr: false},
+		"Missing File Consent":    {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, 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: `{"Content":true, "string": "string"}`, consent: true, wantErr: true},
+		"Bad File Ext Consent":    {fName: "0.txt", fileContents: `{"Content":true, "string": "string"}`, consent: true},
+		"Bad Contents Consent":    {fName: "0.json", fileContents: `bad content`, consent: true, wantErr: true},
+		"minAge Overflow Consent": {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, minAge: math.MaxUint64, consent: true, wantErr: true},
+		"Bad URL Consent":         {fName: "0.json", fileContents: `{"Content":true, "string": "string"}`, url: "http://bad host:1234", consent: true, wantErr: true},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+			require.False(t, tc.missingFile && tc.fileIsDir, "Test case cannot have both missing file and file is dir")
+
+			dir := t.TempDir()
+
+			um := &Manager{
+				collectedDir: filepath.Join(dir, constants.LocalFolder),
+				uploadedDir:  filepath.Join(dir, constants.UploadedFolder),
+				minAge:       tc.minAge,
+				timeProvider: mockTimeProvider{currentTime: 0},
+			}
+
+			require.NoError(t, os.Mkdir(um.collectedDir, 0750), "Setup: failed to create uploaded folder")
+			require.NoError(t, os.Mkdir(um.uploadedDir, 0750), "Setup: failed to create collected folder")
+			fPath := filepath.Join(um.collectedDir, tc.fName)
+
+			if !tc.missingFile && !tc.fileIsDir {
+				require.NoError(t, fileutils.AtomicWrite(fPath, []byte(tc.fileContents)), "Setup: failed to create report file")
+			}
+			if tc.fileIsDir {
+				require.NoError(t, os.Mkdir(fPath, 0750), "Setup: failed to create directory")
+			}
+
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(http.StatusOK)
+			}))
+			t.Cleanup(func() { ts.Close() })
+
+			if tc.url == "" {
+				tc.url = ts.URL
+			}
+
+			err := um.upload(tc.fName, tc.url, tc.consent)
+			if tc.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}
+
+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 := &Manager{
+				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")
+
+			if tc.fileExists {
+				f, err := os.Create(filepath.Join(um.collectedDir, "report.json"))
+				require.NoError(t, err)
+				f.Close()
+			}
+
+			err := um.moveReport("report.json", []byte("payload"))
+			if tc.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			_, err = os.Stat(filepath.Join(um.uploadedDir, "report.json"))
+			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()
+
+	tests := map[string]struct {
+		url            string
+		noServer       bool
+		serverResponse int
+
+		wantErr bool
+	}{
+		"No Server":    {noServer: true, wantErr: true},
+		"Bad URL":      {url: "http://local host:1234", serverResponse: http.StatusOK, wantErr: true},
+		"Bad Response": {serverResponse: http.StatusForbidden, wantErr: true},
+
+		"Success": {serverResponse: http.StatusOK},
+	}
+
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(tc.serverResponse)
+			}))
+			t.Cleanup(func() { ts.Close() })
+
+			if tc.url == "" {
+				tc.url = ts.URL
+			}
+			if tc.noServer {
+				ts.Close()
+			}
+
+			err := send(tc.url, []byte("payload"))
+			if tc.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+		})
+	}
+}

internal/uploader/testdata/TestUpload/golden/consent_manager_false

@@ -1 +1 @@
-uploaded/1.json: '{"OptOut":true}'
+uploaded/1.json: '{"OptOut": true}'

internal/uploader/testdata/TestUpload/golden/consent_manager_global_false,_source_true

@@ -1 +1 @@
-uploaded/1.json: '{"OptOut":true}'
+uploaded/1.json: '{"OptOut": true}'

internal/uploader/testdata/TestUpload/golden/consent_manager_global_true,_source_false

@@ -1 +1 @@
-uploaded/1.json: '{"OptOut":true}'
+uploaded/1.json: '{"OptOut": true}'

internal/uploader/testdata/TestUpload/golden/invalid_directory

@@ -0,0 +1 @@
+{}

internal/uploader/upload.go

@@ -58,37 +58,40 @@ func (um Manager) Upload() error {
 	return nil
 }
 
-func (um Manager) upload(file, url string, consent bool) error {
-	slog.Debug("Uploading report", "file", file, "consent", consent)
+func (um Manager) upload(fName, url string, consent bool) error {
+	slog.Debug("Uploading report", "file", fName, "consent", consent)
 
-	ts, err := reportutils.GetReportTime(file)
+	ts, err := reportutils.GetReportTime(fName)
 	if err != nil {
 		return fmt.Errorf("failed to parse report time from filename: %v", err)
 	}
 
 	// Report maturity check
 	if um.minAge > math.MaxInt64 {
-		return fmt.Errorf("min age is too large: %d", um.minAge)
+		return ErrMinAgeOverflow
 	}
-	if ts+int64(um.minAge) > um.timeProvider.NowUnix() {
-		slog.Debug("Skipping report due to min age", "timestamp", file, "minAge", um.minAge)
+	if ts > um.timeProvider.NowUnix()-int64(um.minAge) {
+		slog.Debug("Skipping report due to min age", "timestamp", fName, "minAge", um.minAge)
 		return ErrReportNotMature
 	}
 
-	payload, err := um.getPayload(file, consent)
+	payload, err := um.getPayload(fName, consent)
 	if err != nil {
 		return fmt.Errorf("failed to get payload: %v", err)
 	}
 	slog.Debug("Uploading", "payload", payload)
 
-	if !um.dryRun {
-		if err := send(url, payload); err != nil {
-			return fmt.Errorf("failed to send data: %v", err)
-		}
+	if um.dryRun {
+		slog.Debug("Dry run, skipping upload")
+		return nil
+	}
 
-		if err := um.moveReport(file, payload); err != nil {
-			return fmt.Errorf("failed to move report after uploading: %v", err)
-		}
+	if err := send(url, payload); err != nil {
+		return fmt.Errorf("failed to send data: %v", err)
+	}
+
+	if err := um.moveReport(fName, payload); err != nil {
+		return fmt.Errorf("failed to move report after uploading: %v", err)
 	}
 
 	return nil
@@ -104,32 +107,31 @@ func (um Manager) getURL() (string, error) {
 }
 
 func (um Manager) getPayload(file string, consent bool) ([]byte, error) {
-	path := path.Join(um.collectedDir, file)
+	if !consent {
+		// Return the opt-out JSON
+		data, err := constants.OptOutJSON.MarshalJSON()
+		if err != nil {
+			return nil, fmt.Errorf("failed to marshal opt-out JSON data: %v", err)
+		}
+		return data, nil
+	}
+
 	var jsonData map[string]interface{}
 
-	data, err := json.Marshal(constants.OptOutJSON)
+	// Read the report file
+	data, err := os.ReadFile(path.Join(um.collectedDir, file))
 	if err != nil {
-		return nil, fmt.Errorf("failed to marshal JSON data")
+		return nil, fmt.Errorf("failed to read report file: %v", err)
 	}
-	if consent {
-		// Read the report file
-		data, err := os.ReadFile(path)
-		if err != nil {
-			return nil, fmt.Errorf("failed to read report file: %v", err)
-		}
-
-		// Remashal 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)
-		}
 
-		// Marshal the JSON data back to bytes
-		data, err = json.Marshal(jsonData)
-		if err != nil {
-			return nil, fmt.Errorf("failed to marshal JSON data: %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)
+	}
 
-		return data, nil
+	data, err = json.Marshal(jsonData)
+	if err != nil {
+		return nil, fmt.Errorf("failed to marshal JSON data: %v", err)
 	}
 
 	return data, nil