revert prev commit; add test functions

cmd/notify-mailer/main.go

@@ -1,7 +1,6 @@
 package notmain
 
 import (
-	"bufio"
 	"context"
 	"encoding/csv"
 	"encoding/json"
@@ -123,21 +122,15 @@ func (m *mailer) makeMessageBody(recipients []recipient) (string, error) {
 	return messageBody.String(), nil
 }
 
-// Write the keys of addressToRecipientMap to filename
+// Take an addressToRecipientMap, then marshal and write it to saveEmailsTo
 func writeEmailsToFile(filename string, addressToRecipient addressToRecipientMap) error {
-	outfile, err := os.Create(filename)
+	jsonData, err := json.Marshal(addressToRecipient)
 	if err != nil {
 		return err
 	}
 
-	defer outfile.Close()
-	for key := range addressToRecipient {
-		_, err = outfile.WriteString(key + "\n")
-		if err != nil {
-			return err
-		}
-	}
-	return nil
+	err = os.WriteFile(filename, jsonData, 0644)
+	return err
 }
 
 func (m *mailer) run(ctx context.Context) error {
@@ -149,7 +142,7 @@ func (m *mailer) run(ctx context.Context) error {
 	totalRecipients := len(m.recipients)
 	m.log.Infof("Resolving addresses for (%d) recipients", totalRecipients)
 
-	addressToRecipient, err := m.resolveAddresses(ctx)
+	addressToRecipient, _, err := m.resolveAddresses(ctx)
 	if err != nil {
 		return err
 	}
@@ -270,16 +263,17 @@ func (m *mailer) run(ctx context.Context) error {
 }
 
 // resolveAddresses creates a mapping of email addresses to (a list of)
-// `recipient`s that resolve to that email address.
-func (m *mailer) resolveAddresses(ctx context.Context) (addressToRecipientMap, error) {
+// `recipient`s that resolve to that email address. Return true bool if
+// skip resolve address steps and return readEmailsMap instead (for testing purposes)
+func (m *mailer) resolveAddresses(ctx context.Context) (addressToRecipientMap, bool, error) {
 	if m.saveEmailsTo == "" && m.readEmailsMap != nil {
-		return m.readEmailsMap, nil
+		return m.readEmailsMap, true, nil
 	}
 	result := make(addressToRecipientMap, len(m.recipients))
 	for _, recipient := range m.recipients {
 		addresses, err := getAddressForID(ctx, recipient.id, m.dbMap)
 		if err != nil {
-			return nil, err
+			return nil, false, err
 		}
 
 		for _, address := range addresses {
@@ -293,7 +287,7 @@ func (m *mailer) resolveAddresses(ctx context.Context) (addressToRecipientMap, e
 			}
 		}
 	}
-	return result, nil
+	return result, false, nil
 }
 
 // dbSelector abstracts over a subset of methods from `borp.DbMap` objects to
@@ -456,20 +450,19 @@ func readRecipientsList(filename string, delimiter rune) ([]recipient, string, e
 	}
 }
 
-// Read the emails in readEmailsFile and extract them as keys in an addressToRecipientMap
+// Read filename then extract and return parsed addressToRecipientMap
 func readEmailsFile(filename string) (addressToRecipientMap, error) {
-	infile, err := os.Open(filename)
+	data, err := os.ReadFile(filename)
 	if err != nil {
 		return nil, err
 	}
-	defer infile.Close()
 
-	scanner := bufio.NewScanner(infile)
-	addressToRecipientMap := make(addressToRecipientMap)
-	for scanner.Scan() {
-		addressToRecipientMap[scanner.Text()] = []recipient{}
+	var readEmailsMap addressToRecipientMap
+	err = json.Unmarshal(data, &readEmailsMap)
+	if err != nil {
+		return nil, err
 	}
-	return addressToRecipientMap, nil
+	return readEmailsMap, nil
 }
 
 const usageIntro = `
@@ -607,7 +600,7 @@ func main() {
 	var readEmailsMap addressToRecipientMap
 	if *readEmailsFrom != "" {
 		readEmailsMap, err = readEmailsFile(*readEmailsFrom)
-		cmd.FailOnError(err, "Couldn't parse readEmailsFrom file")
+		cmd.FailOnError(err, "Could not parse readEmailsFrom file")
 	}
 
 	// Load and parse message body.

cmd/notify-mailer/main_test.go

@@ -1,12 +1,15 @@
 package notmain
 
 import (
+	"bufio"
 	"context"
 	"database/sql"
 	"errors"
 	"fmt"
 	"io"
 	"os"
+	"reflect"
+	"strings"
 	"testing"
 	"text/template"
 	"time"
@@ -763,7 +766,7 @@ func TestResolveEmails(t *testing.T) {
 		clk:           clock.NewFake(),
 	}
 
-	addressesToRecipients, err := m.resolveAddresses(context.Background())
+	addressesToRecipients, _, err := m.resolveAddresses(context.Background())
 	test.AssertNotError(t, err, "failed to resolveEmailAddresses")
 
 	expected := []string{
@@ -781,54 +784,124 @@ func TestResolveEmails(t *testing.T) {
 	}
 }
 
-// Create a test addressToRecipientMap, write the emails to a file,
-// read and extract the emails from the file to a new map.
-// Test original and new map have same set of emails.
-func TestReadWriteEmailFiles(t *testing.T) {
-	filename := "testdata/notify-mailer-test.txt"
-	defer os.Remove(filename)
-
+func GetTestAddressToRecipientMap() addressToRecipientMap {
 	// Make and fill a test addressToRecipientMap
-	originalMap := make(addressToRecipientMap)
-	originalMap["[email protected]"] = []recipient{
-		{id: 10, Data: map[string]string{"date": "2018-11-21", "domainName": "example.com"}},
-		{id: 23, Data: map[string]string{"date": "2018-11-22", "domainName": "example.net"}},
+	myMap := make(addressToRecipientMap) // exclude id because it won't be exported
+	myMap["[email protected]"] = []recipient{
+		{Data: map[string]string{"date": "2018-11-21", "domainName": "example.com"}},
+		{Data: map[string]string{"date": "2018-11-22", "domainName": "example.net"}},
 	}
-	originalMap["[email protected]"] = []recipient{
-		{id: 11, Data: map[string]string{"date": "2019-09-21", "domainName": "test.com"}},
-		{id: 24, Data: map[string]string{"date": "2024-01-01", "domainName": "foo.bar"}},
-		{id: 156, Data: map[string]string{"date": "2025-01-04", "domainName": "foo.baz"}},
+	myMap["[email protected]"] = []recipient{
+		{Data: map[string]string{"date": "2019-09-21", "domainName": "test.com"}},
+		{Data: map[string]string{"date": "2024-01-01", "domainName": "foo.bar"}},
+		{Data: map[string]string{"date": "2025-01-04", "domainName": "foo.baz"}},
 	}
-	originalMap["[email protected]"] = []recipient{
-		{id: 217262284, Data: map[string]string{"date": "2018-11-21", "domainName": "example.org"}},
+	myMap["[email protected]"] = []recipient{
+		{Data: map[string]string{"date": "2018-11-21", "domainName": "example.org"}},
 	}
+	return myMap
+}
 
-	// Write emails from originalMap to filename
+// Test that map written to file and map read from that file are the same.
+func TestReadWriteEmailFiles(t *testing.T) {
+	filename := "testdata/notify-mailer-test.json"
+	defer os.Remove(filename)
+
+	// Make and fill a test addressToRecipientMap
+	originalMap := GetTestAddressToRecipientMap()
+
+	// Marshal and write map to file
 	err := writeEmailsToFile(filename, originalMap)
 	if err != nil {
 		t.Errorf("error writing email map to file: %s", err)
 	}
 
-	// Read emails from filename and add as keys to readMap
+	// Read and unmarshal map from file
 	readMap, err := readEmailsFile(filename)
 	if err != nil {
 		t.Errorf("error reading email map file: %s", err)
 	}
 
-	// Check originalMap and readMap have exact same set of emails/keys
-	for k := range originalMap {
-		if _, ok := readMap[k]; !ok {
-			t.Error("orininalMap has key that is missing in readMap")
-		}
-		delete(readMap, k)
+	// Check if originalMap and readMap the same
+	if !reflect.DeepEqual(readMap, originalMap) {
+		t.Errorf("maps not the same")
+	}
+}
+
+// Test that map read from testdata/addressToRecipientMap matches the
+// expected map and is addressed correctly by resolveAddresses()
+func TestReadFilesFrom(t *testing.T) {
+	tmpl := template.Must(template.New("letter").Parse("an email body"))
+
+	readEmailsMap, err := readEmailsFile("testdata/addressToRecipientMap.json")
+	if err != nil {
+		t.Errorf("error reading email map file: %s", err)
 	}
-	if len(readMap) > 0 {
-		t.Error("readMap has key that is not present in originalMap")
+
+	expectedMap := GetTestAddressToRecipientMap()
+	if !reflect.DeepEqual(expectedMap, readEmailsMap) {
+		t.Errorf("parsed map from read file doesn't match expected map")
+	}
+
+	dbMap := mockEmailResolver{}
+	mc := &mocks.Mailer{}
+	m := &mailer{
+		log:           blog.UseMock(),
+		mailer:        mc,
+		dbMap:         dbMap,
+		subject:       "Test",
+		recipients:    []recipient{},
+		emailTemplate: tmpl,
+		targetRange:   interval{end: "\xFF"},
+		sleepInterval: 0,
+		clk:           clock.NewFake(),
+		readEmailsMap: readEmailsMap,
+	}
+
+	resolvedMap, isRead, err := m.resolveAddresses(context.Background())
+	if err != nil {
+		t.Errorf("error running resolveAddresses: %s", err)
+	}
+	if !isRead {
+		t.Error("Addresses should have been read from file, not resolved")
+	}
+	if !reflect.DeepEqual(readEmailsMap, resolvedMap) {
+		t.Error("Resolved map not same as map read from file")
 	}
 }
 
-// test scenarios:
-// DONE write --> read path gives same map
-// test read-only path
-// test save-only path
-// test batch path with read + save
+// Test that map written to testdata/saveemailsto-test.json includes
+// the expected data (using testdata/addressToRecipientMap.json for comparison)
+func TestSaveEmailsTo(t *testing.T) {
+	// write addressToRecipientMap to testdata/saveemailsto-test.json
+	saveEmailsTo := "testdata/saveemailsto-test.json"
+	addressToRecipientMap := GetTestAddressToRecipientMap()
+	err := writeEmailsToFile(saveEmailsTo, addressToRecipientMap)
+	if err != nil {
+		t.Errorf("error writing email map to file: %s", err)
+	}
+	defer os.Remove(saveEmailsTo)
+
+	// get saveEmailsTo file output as string
+	saveFile, err := os.ReadFile(saveEmailsTo)
+	if err != nil {
+		t.Error("error reading saveEmailsTo file")
+	}
+	saveData := string(saveFile)
+
+	// open testdata/addressToRecipientMap.json for comparison
+	expectedFile, err := os.Open("testdata/addressToRecipientMap.json")
+	if err != nil {
+		t.Error("error reading testdata/addressToRecipientMap.json")
+	}
+	defer expectedFile.Close()
+
+	// for each line in the expected file, check that it exists in the saveEmailsTo file
+	scanner := bufio.NewScanner(expectedFile)
+	for scanner.Scan() {
+		if !strings.Contains(saveData, strings.TrimSpace(scanner.Text())) {
+			fmt.Println(scanner.Text())
+			t.Errorf("expected data missing from saveEmailsTo file: %s", strings.TrimSpace(scanner.Text()))
+		}
+	}
+}

cmd/notify-mailer/testdata/addressToRecipientMap.json

@@ -0,0 +1,16 @@
+{"[email protected]": 
+    [
+        {"Data":{"date":"2019-09-21","domainName":"test.com"}},
+        {"Data":{"date":"2024-01-01","domainName":"foo.bar"}},
+        {"Data":{"date":"2025-01-04","domainName":"foo.baz"}}
+    ],
+    "[email protected]":
+    [
+        {"Data":{"date":"2018-11-21","domainName":"example.org"}}
+    ],
+    "[email protected]":
+    [
+        {"Data":{"date":"2018-11-21","domainName":"example.com"}},
+        {"Data":{"date":"2018-11-22","domainName":"example.net"}}
+    ]
+}