notify-mailer: Improve terminology consistency and general cleanup (#5485)

### Improve consistency
- Make registration `id` an `int64`
- Use `address`, `recipient`, and `record` terminology
- Use `errors.New()` in place of `fmt.Errorf()`
- Use `strings.Builder` in place of `bytes.Buffer`
- Use `errors.Is()` when checking for sentinel errors
- Remove unused (duplicate) `cmd.PasswordFile` in `config`
- Remove unused `cmd.Features` in `config`

### Improve readability
- Use godoc standard comments
- Replace multiple calls to `len(someVariable)` with `totalSomeVariable`

Part of #5420
This commit is contained in:
Samantha 2021-06-15 10:09:19 -07:00 committed by GitHub
parent b5aab29407
commit 205223abbc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 208 additions and 193 deletions

View File

@ -1,7 +1,6 @@
package main
import (
"bytes"
"encoding/csv"
"encoding/json"
"errors"
@ -20,7 +19,6 @@ import (
"github.com/jmhodges/clock"
"github.com/letsencrypt/boulder/cmd"
"github.com/letsencrypt/boulder/db"
"github.com/letsencrypt/boulder/features"
blog "github.com/letsencrypt/boulder/log"
bmail "github.com/letsencrypt/boulder/mail"
"github.com/letsencrypt/boulder/metrics"
@ -35,31 +33,33 @@ type mailer struct {
mailer bmail.Mailer
subject string
emailTemplate *template.Template
destinations []recipient
recipients []recipient
targetRange interval
sleepInterval time.Duration
}
// interval defines a range of email addresses to send to, alphabetically.
// The "start" field is inclusive and the "end" field is exclusive.
// To include everything, set "end" to "\xFF".
// interval defines a range of email addresses to send to in alphabetical order.
// The `start` field is inclusive and the `end` field is exclusive. To include
// everything, set `end` to \xFF.
type interval struct {
start string
end string
}
type contactJSON struct {
ID int
// contactQueryResult is a receiver for queries to the `registrations` table.
type contactQueryResult struct {
// ID is exported to receive the value of `id`.
ID int64
// Contact is exported to receive the value of `contact`.
Contact []byte
}
func (i *interval) ok() error {
if i.start > i.end {
return fmt.Errorf(
"interval start value (%s) is greater than end value (%s)",
return fmt.Errorf("interval start value (%s) is greater than end value (%s)",
i.start, i.end)
}
return nil
}
@ -67,37 +67,35 @@ func (i *interval) includes(s string) bool {
return s >= i.start && s < i.end
}
// ok ensures that both the `targetRange` and `sleepInterval` are valid.
func (m *mailer) ok() error {
// Make sure the checkpoint range is OK
if checkpointErr := m.targetRange.ok(); checkpointErr != nil {
return checkpointErr
if err := m.targetRange.ok(); err != nil {
return err
}
// Do not allow a negative sleep interval
if m.sleepInterval < 0 {
return fmt.Errorf(
"sleep interval (%d) is < 0", m.sleepInterval)
}
return nil
}
func (m *mailer) printStatus(to string, cur, total int, start time.Time) {
// Should never happen
if total <= 0 || cur < 1 || cur > total {
m.log.AuditErrf("invalid cur (%d) or total (%d)", cur, total)
func (m *mailer) logStatus(to string, current, total int, start time.Time) {
// Should never happen.
if total <= 0 || current < 1 || current > total {
m.log.AuditErrf("Invalid current (%d) or total (%d)", current, total)
}
completion := (float32(cur) / float32(total)) * 100
completion := (float32(current) / float32(total)) * 100
now := m.clk.Now()
elapsed := now.Sub(start)
m.log.Infof("Sending to %q. Message %d of %d [%.2f%%]. Elapsed: %s",
to, cur, total, completion, elapsed)
m.log.Infof("Sending message (%d) of (%d) to address (%s) [%.2f%%] time elapsed (%s)",
current, total, to, completion, elapsed)
}
func sortAddresses(input emailToRecipientMap) []string {
func sortAddresses(input addressToRecipientMap) []string {
var addresses []string
for k := range input {
addresses = append(addresses, k)
for address := range input {
addresses = append(addresses, address)
}
sort.Strings(addresses)
return addresses
@ -108,122 +106,128 @@ func (m *mailer) run() error {
return err
}
m.log.Infof("Resolving %d destination addresses", len(m.destinations))
addressesToRecipients, err := m.resolveEmailAddresses()
totalRecipients := len(m.recipients)
m.log.Infof("Resolving addresses for (%d) recipients", totalRecipients)
addressToRecipient, err := m.resolveAddresses()
if err != nil {
return err
}
if len(addressesToRecipients) == 0 {
return fmt.Errorf("zero recipients after looking up addresses?")
totalAddresses := len(addressToRecipient)
if totalAddresses == 0 {
return errors.New("0 recipients remained after resolving addresses")
}
m.log.Infof("Resolved destination addresses. %d accounts became %d addresses.",
len(m.destinations), len(addressesToRecipients))
var biggest int
var biggestAddress string
for k, v := range addressesToRecipients {
if len(v) > biggest {
biggest = len(v)
biggestAddress = k
m.log.Infof("%d recipients were resolved to %d addresses", totalRecipients, totalAddresses)
var mostRecipients string
var mostRecipientsLen int
for k, v := range addressToRecipient {
if len(v) > mostRecipientsLen {
mostRecipientsLen = len(v)
mostRecipients = k
}
}
m.log.Infof("Most frequent address %q had %d associated lines", biggestAddress, biggest)
m.log.Infof("Address %q was associated with the most recipients (%d)",
mostRecipients, mostRecipientsLen)
err = m.mailer.Connect()
if err != nil {
return err
}
defer func() {
_ = m.mailer.Close()
}()
defer func() { _ = m.mailer.Close() }()
startTime := m.clk.Now()
sortedAddresses := sortAddresses(addressesToRecipients)
numAddresses := len(addressesToRecipients)
sortedAddresses := sortAddresses(addressToRecipient)
var sent int
for i, address := range sortedAddresses {
if !m.targetRange.includes(address) {
m.log.Debugf("skipping %q: out of target range")
m.log.Debugf("Address %q is outside of target range, skipping", address)
continue
}
if err := policy.ValidEmail(address); err != nil {
m.log.Infof("skipping %q: %s", address, err)
m.log.Infof("Skipping %q due to policy violation: %s", address, err)
continue
}
recipients := addressesToRecipients[address]
m.printStatus(address, i+1, numAddresses, startTime)
var mailBody bytes.Buffer
err = m.emailTemplate.Execute(&mailBody, recipients)
recipients := addressToRecipient[address]
m.logStatus(address, i+1, totalAddresses, startTime)
var messageBody strings.Builder
err = m.emailTemplate.Execute(&messageBody, recipients)
if err != nil {
return err
}
if mailBody.Len() == 0 {
return fmt.Errorf("email body was empty after interpolation.")
if messageBody.Len() == 0 {
return errors.New("message body was empty after interpolation")
}
err := m.mailer.SendMail([]string{address}, m.subject, mailBody.String())
err := m.mailer.SendMail([]string{address}, m.subject, messageBody.String())
if err != nil {
var recoverableSMTPErr bmail.RecoverableSMTPError
if errors.As(err, &recoverableSMTPErr) {
m.log.Errf("address %q was rejected by server: %s", address, err)
m.log.Errf("Address %q was rejected by the server due to: %s", address, err)
continue
}
return fmt.Errorf("sending mail %d of %d to %q: %s",
return fmt.Errorf("while sending mail (%d) of (%d) to address %q: %s",
i, len(sortedAddresses), address, err)
}
sent++
m.clk.Sleep(m.sleepInterval)
}
if sent == 0 {
return fmt.Errorf("sent zero messages. Check recipients and configured interval")
return errors.New("0 messages sent, check recipients or configured interval")
}
return nil
}
// resolveEmailAddresses looks up the id of each recipient to find that
// account's email addresses, then adds that recipient to a map from address to
// recipient struct.
func (m *mailer) resolveEmailAddresses() (emailToRecipientMap, error) {
result := make(emailToRecipientMap, len(m.destinations))
for _, r := range m.destinations {
// Get the email address for the reg ID
emails, err := emailsForReg(r.id, m.dbMap)
// resolveAddresses creates a mapping of email addresses to (a list of)
// `recipient`s that resolve to that email address.
func (m *mailer) resolveAddresses() (addressToRecipientMap, error) {
result := make(addressToRecipientMap, len(m.recipients))
for _, recipient := range m.recipients {
addresses, err := getAddressForID(recipient.id, m.dbMap)
if err != nil {
return nil, err
}
for _, email := range emails {
parsedEmail, err := mail.ParseAddress(email)
for _, address := range addresses {
parsed, err := mail.ParseAddress(address)
if err != nil {
m.log.Errf("unparsable email for reg ID %d : %q", r.id, email)
m.log.Errf("Unparsable address %q, skipping ID (%d)", address, recipient.id)
continue
}
addr := parsedEmail.Address
result[addr] = append(result[addr], r)
result[parsed.Address] = append(result[parsed.Address], recipient)
}
}
return result, nil
}
// Since the only thing we use from gorp is the SelectOne method on the
// gorp.DbMap object, we just define an interface with that method
// instead of importing all of gorp. This facilitates mock implementations for
// unit tests
// dbSelector abstracts over a subset of methods from `gorp.DbMap` objects to
// facilitate mocking in unit tests.
type dbSelector interface {
SelectOne(holder interface{}, query string, args ...interface{}) error
}
// Finds the email addresses associated with a reg ID
func emailsForReg(id int, dbMap dbSelector) ([]string, error) {
var contact contactJSON
err := dbMap.SelectOne(&contact,
`SELECT id, contact
// getAddressForID queries the database for the email address associated with
// the provided registration ID.
func getAddressForID(id int64, dbMap dbSelector) ([]string, error) {
var result contactQueryResult
err := dbMap.SelectOne(&result,
`SELECT id,
contact
FROM registrations
WHERE contact != 'null' AND id = :id;`,
map[string]interface{}{
"id": id,
})
WHERE contact != 'null'
AND id = :id;`,
map[string]interface{}{"id": id})
if err != nil {
if db.IsNoRows(err) {
return []string{}, nil
@ -231,86 +235,98 @@ func emailsForReg(id int, dbMap dbSelector) ([]string, error) {
return nil, err
}
var contactFields []string
var addresses []string
err = json.Unmarshal(contact.Contact, &contactFields)
var contacts []string
err = json.Unmarshal(result.Contact, &contacts)
if err != nil {
return nil, err
}
for _, entry := range contactFields {
if strings.HasPrefix(entry, "mailto:") {
addresses = append(addresses, strings.TrimPrefix(entry, "mailto:"))
var addresses []string
for _, contact := range contacts {
if strings.HasPrefix(contact, "mailto:") {
addresses = append(addresses, strings.TrimPrefix(contact, "mailto:"))
}
}
return addresses, nil
}
// recipient represents one line in the input CSV, containing an account and
// (optionally) some extra fields related to that account.
// recipient represents a single record from the recipient list file.
type recipient struct {
id int
Extra map[string]string
id int64
// Data is exported so the contents can be referenced from message template.
Data map[string]string
}
// emailToRecipientMap maps from an email address to a list of recipients with
// that email address.
type emailToRecipientMap map[string][]recipient
// addressToRecipientMap maps email addresses to a list of `recipient`s that
// resolve to that email address.
type addressToRecipientMap map[string][]recipient
// readRecipientsList reads a CSV filename and parses that file into a list of
// recipient structs. It puts any columns after the first into a per-recipient
// map from column name -> value.
// readRecipientsList parses the contents of a recipient list file into a list
// of `recipient` objects.
func readRecipientsList(filename string) ([]recipient, error) {
f, err := os.Open(filename)
if err != nil {
return nil, err
}
reader := csv.NewReader(f)
record, err := reader.Read()
if err != nil {
return nil, err
}
if len(record) == 0 {
return nil, fmt.Errorf("no entries in CSV")
}
if record[0] != "id" {
return nil, fmt.Errorf("first field of CSV input must be an ID.")
}
var columnNames []string
for _, v := range record[1:] {
columnNames = append(columnNames, strings.TrimSpace(v))
return nil, errors.New("no records in CSV")
}
results := []recipient{}
if record[0] != "id" {
return nil, errors.New("first field of CSV input must be \"id\"")
}
var dataColumns []string
for _, v := range record[1:] {
dataColumns = append(dataColumns, strings.TrimSpace(v))
}
var recipients []recipient
for {
record, err := reader.Read()
if err == io.EOF {
if len(results) == 0 {
return nil, fmt.Errorf("no entries after the header in CSV")
if errors.Is(err, io.EOF) {
// Finished parsing the file.
if len(recipients) == 0 {
return nil, fmt.Errorf("no records after the header in CSV")
}
return results, nil
}
if err != nil {
return recipients, nil
} else if err != nil {
return nil, err
}
if len(record) == 0 {
return nil, fmt.Errorf("empty line in CSV")
}
if len(record) != len(columnNames)+1 {
return nil, fmt.Errorf("Number of columns in CSV line didn't match header columns."+
" Got %d, expected %d. Line: %v", len(record), len(columnNames)+1, record)
if len(record) != len(dataColumns)+1 {
return nil, fmt.Errorf("got (%d) columns, for (%d) header columns, for line %q",
len(record), len(dataColumns)+1, record)
}
id, err := strconv.Atoi(record[0])
// Ensure the ID in the record can be parsed as a valid ID.
recordID := record[0]
id, err := strconv.ParseInt(recordID, 10, 64)
if err != nil {
return nil, err
}
recip := recipient{
id: id,
Extra: make(map[string]string),
return nil, fmt.Errorf(
"%q couldn't be parsed as a registration ID due to: %s", recordID, err)
}
// Create a mapping of column names to extra data columns (anything
// after `id`).
data := make(map[string]string)
for i, v := range record[1:] {
recip.Extra[columnNames[i]] = v
data[dataColumns[i]] = v
}
results = append(results, recip)
recipients = append(recipients, recipient{id, data})
}
}
@ -404,15 +420,6 @@ func main() {
end := flag.String("end", "\xFF", "Alphabetically highest email address (exclusive).")
reconnBase := flag.Duration("reconnectBase", 1*time.Second, "Base sleep duration between reconnect attempts")
reconnMax := flag.Duration("reconnectMax", 5*60*time.Second, "Max sleep duration between reconnect attempts after exponential backoff")
type config struct {
NotifyMailer struct {
DB cmd.DBConfig
cmd.PasswordConfig
cmd.SMTPConfig
Features map[string]bool
}
Syslog cmd.SyslogConfig
}
configFile := flag.String("config", "", "File containing a JSON config.")
flag.Usage = func() {
@ -421,6 +428,7 @@ func main() {
flag.PrintDefaults()
}
// Validate required args.
flag.Parse()
if *from == "" || *subject == "" || *bodyFile == "" || *configFile == "" ||
*recipientListFile == "" {
@ -429,48 +437,50 @@ func main() {
}
configData, err := ioutil.ReadFile(*configFile)
cmd.FailOnError(err, fmt.Sprintf("Reading %q", *configFile))
cmd.FailOnError(err, "Couldn't load JSON config file")
type config struct {
NotifyMailer struct {
DB cmd.DBConfig
cmd.SMTPConfig
}
Syslog cmd.SyslogConfig
}
// Parse JSON config.
var cfg config
err = json.Unmarshal(configData, &cfg)
cmd.FailOnError(err, "Unmarshaling config")
err = features.Set(cfg.NotifyMailer.Features)
cmd.FailOnError(err, "Failed to set feature flags")
cmd.FailOnError(err, "Couldn't unmarshal JSON config file")
log := cmd.NewLogger(cfg.Syslog)
defer log.AuditPanic()
// Setup database client.
dbURL, err := cfg.NotifyMailer.DB.URL()
cmd.FailOnError(err, "Couldn't load DB URL")
dbSettings := sa.DbSettings{
MaxOpenConns: 10,
}
dbMap, err := sa.NewDbMap(dbURL, dbSettings)
cmd.FailOnError(err, "Could not connect to database")
// Load email body
body, err := ioutil.ReadFile(*bodyFile)
cmd.FailOnError(err, fmt.Sprintf("Reading %q", *bodyFile))
template, err := template.New("email").Parse(string(body))
cmd.FailOnError(err, fmt.Sprintf("Parsing template %q", *bodyFile))
dbMap, err := sa.NewDbMap(dbURL, sa.DbSettings{MaxOpenConns: 10})
cmd.FailOnError(err, "Couldn't create database connection")
// Load and parse message body.
template, err := template.New("email").ParseFiles(*bodyFile)
cmd.FailOnError(err, "Couldn't parse message template")
address, err := mail.ParseAddress(*from)
cmd.FailOnError(err, fmt.Sprintf("Parsing %q", *from))
cmd.FailOnError(err, fmt.Sprintf("Couldn't parse %q to address", *from))
recipients, err := readRecipientsList(*recipientListFile)
cmd.FailOnError(err, fmt.Sprintf("Reading %q", *recipientListFile))
targetRange := interval{
start: *start,
end: *end,
}
cmd.FailOnError(err, "Couldn't populate recipients")
var mailClient bmail.Mailer
if *dryRun {
log.Infof("Doing a dry run.")
log.Infof("Starting %s in dry-run mode", cmd.VersionString())
mailClient = bmail.NewDryRun(*address, log)
} else {
log.Infof("Starting %s", cmd.VersionString())
smtpPassword, err := cfg.NotifyMailer.PasswordConfig.Pass()
cmd.FailOnError(err, "Failed to load SMTP password")
cmd.FailOnError(err, "Couldn't load SMTP password from file")
mailClient = bmail.New(
cfg.NotifyMailer.Server,
cfg.NotifyMailer.Port,
@ -490,12 +500,17 @@ func main() {
dbMap: dbMap,
mailer: mailClient,
subject: *subject,
destinations: recipients,
recipients: recipients,
emailTemplate: template,
targetRange: targetRange,
targetRange: interval{
start: *start,
end: *end,
},
sleepInterval: *sleep,
}
err = m.run()
cmd.FailOnError(err, "mailer.send returned error")
cmd.FailOnError(err, "Couldn't complete")
log.Info("Completed successfully")
}

View File

@ -86,7 +86,7 @@ func TestSleepInterval(t *testing.T) {
sleepInterval: sleepLen * time.Second,
targetRange: interval{start: "", end: "\xFF"},
clk: newFakeClock(t),
destinations: recipients,
recipients: recipients,
dbMap: dbMap,
}
@ -107,7 +107,7 @@ func TestSleepInterval(t *testing.T) {
sleepInterval: 0,
targetRange: interval{end: "\xFF"},
clk: newFakeClock(t),
destinations: recipients,
recipients: recipients,
dbMap: dbMap,
}
@ -135,7 +135,7 @@ func TestMailIntervals(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: testSubject,
destinations: recipients,
recipients: recipients,
emailTemplate: tmpl,
targetRange: interval{start: "\xFF", end: "\xFF\xFF"},
sleepInterval: 0,
@ -154,7 +154,7 @@ func TestMailIntervals(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: testSubject,
destinations: recipients,
recipients: recipients,
emailTemplate: tmpl,
targetRange: interval{},
sleepInterval: -10,
@ -174,7 +174,7 @@ func TestMailIntervals(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: testSubject,
destinations: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}},
recipients: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}},
emailTemplate: tmpl,
targetRange: interval{start: "test-example-updated@letsencrypt.org", end: "\xFF"},
sleepInterval: 0,
@ -206,7 +206,7 @@ func TestMailIntervals(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: testSubject,
destinations: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}},
recipients: []recipient{{id: 1}, {id: 2}, {id: 3}, {id: 4}},
emailTemplate: tmpl,
targetRange: interval{end: "test-example-updated@letsencrypt.org"},
sleepInterval: 0,
@ -243,7 +243,7 @@ func TestMessageContentStatic(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: testSubject,
destinations: []recipient{{id: 1}},
recipients: []recipient{{id: 1}},
emailTemplate: template.Must(template.New("letter").Parse("an email body")),
targetRange: interval{end: "\xFF"},
sleepInterval: 0,
@ -267,7 +267,7 @@ func TestMessageContentInterpolated(t *testing.T) {
recipients := []recipient{
{
id: 1,
Extra: map[string]string{
Data: map[string]string{
"validationMethod": "eyeballing it",
},
},
@ -275,13 +275,13 @@ func TestMessageContentInterpolated(t *testing.T) {
dbMap := mockEmailResolver{}
mc := &mocks.Mailer{}
m := &mailer{
log: blog.UseMock(),
mailer: mc,
dbMap: dbMap,
subject: "Test Subject",
destinations: recipients,
log: blog.UseMock(),
mailer: mc,
dbMap: dbMap,
subject: "Test Subject",
recipients: recipients,
emailTemplate: template.Must(template.New("letter").Parse(
`issued by {{range .}}{{ .Extra.validationMethod }}{{end}}`)),
`issued by {{range .}}{{ .Data.validationMethod }}{{end}}`)),
targetRange: interval{end: "\xFF"},
sleepInterval: 0,
clk: newFakeClock(t),
@ -305,25 +305,25 @@ func TestMessageContentInterpolatedMultiple(t *testing.T) {
recipients := []recipient{
{
id: 200,
Extra: map[string]string{
Data: map[string]string{
"domain": "blog.example.com",
},
},
{
id: 201,
Extra: map[string]string{
Data: map[string]string{
"domain": "nas.example.net",
},
},
{
id: 202,
Extra: map[string]string{
Data: map[string]string{
"domain": "mail.example.org",
},
},
{
id: 203,
Extra: map[string]string{
Data: map[string]string{
"domain": "panel.example.net",
},
},
@ -331,14 +331,14 @@ func TestMessageContentInterpolatedMultiple(t *testing.T) {
dbMap := mockEmailResolver{}
mc := &mocks.Mailer{}
m := &mailer{
log: blog.UseMock(),
mailer: mc,
dbMap: dbMap,
subject: "Test Subject",
destinations: recipients,
log: blog.UseMock(),
mailer: mc,
dbMap: dbMap,
subject: "Test Subject",
recipients: recipients,
emailTemplate: template.Must(template.New("letter").Parse(
`issued for:
{{range .}}{{ .Extra.domain }}
{{range .}}{{ .Data.domain }}
{{end}}Thanks`)),
targetRange: interval{end: "\xFF"},
sleepInterval: 0,
@ -371,7 +371,7 @@ type mockEmailResolver struct{}
// into a list of anonymous structs
func (bs mockEmailResolver) SelectOne(output interface{}, _ string, args ...interface{}) error {
// The "dbList" is just a list of contact records in memory
dbList := []contactJSON{
dbList := []contactQueryResult{
{
ID: 1,
Contact: []byte(`["mailto:example@letsencrypt.org"]`),
@ -423,21 +423,21 @@ func (bs mockEmailResolver) SelectOne(output interface{}, _ string, args ...inte
}
// Play the type cast game so that we can dig into the arguments map and get
// out an integer "id" parameter
// out an int64 `id` parameter.
argsRaw := args[0]
argsMap, ok := argsRaw.(map[string]interface{})
if !ok {
return fmt.Errorf("incorrect args type %T", args)
}
idRaw := argsMap["id"]
id, ok := idRaw.(int)
id, ok := idRaw.(int64)
if !ok {
return fmt.Errorf("incorrect args ID type %T", id)
}
// Play the type cast game to get a pointer to the output `contactJSON`
// pointer so we can write the result from the db list
outputPtr, ok := output.(*contactJSON)
// Play the type cast game to get a `*contactQueryResult` so we can write
// the result from the db list.
outputPtr, ok := output.(*contactQueryResult)
if !ok {
return fmt.Errorf("incorrect output type %T", output)
}
@ -508,14 +508,14 @@ func TestResolveEmails(t *testing.T) {
mailer: mc,
dbMap: dbMap,
subject: "Test",
destinations: recipients,
recipients: recipients,
emailTemplate: tmpl,
targetRange: interval{end: "\xFF"},
sleepInterval: 0,
clk: newFakeClock(t),
}
addressesToRecipients, err := m.resolveEmailAddresses()
addressesToRecipients, err := m.resolveAddresses()
test.AssertNotError(t, err, "failed to resolveEmailAddresses")
expected := []string{