From a9cc697fcbc631a4041ffe106504d3990aa11555 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Wed, 19 Jun 2019 15:04:54 -0700
Subject: [PATCH 01/12] Move domains functions to a separate file

This commit only consists of renaming files and moving some functions.
---
 db/policy.go                              | 117 ++++++++++++++++++++++
 db/sqldb.go                               | 109 --------------------
 models/{domain.go => policy.go}           |   0
 models/{domain_test.go => policy_test.go} |   0
 4 files changed, 117 insertions(+), 109 deletions(-)
 create mode 100644 db/policy.go
 rename models/{domain.go => policy.go} (100%)
 rename models/{domain_test.go => policy_test.go} (100%)

diff --git a/db/policy.go b/db/policy.go
new file mode 100644
index 00000000..2505deab
--- /dev/null
+++ b/db/policy.go
@@ -0,0 +1,117 @@
+package db
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	"github.com/EFForg/starttls-backend/models"
+)
+
+func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.Domain, error) {
+	query := fmt.Sprintf(sqlQuery, "domain, email, data, status, last_updated, queue_weeks")
+	data := models.Domain{}
+	var rawMXs string
+	err := db.conn.QueryRow(query, args...).Scan(
+		&data.Name, &data.Email, &rawMXs, &data.State, &data.LastUpdated, &data.QueueWeeks)
+	data.MXs = strings.Split(rawMXs, ",")
+	if len(rawMXs) == 0 {
+		data.MXs = []string{}
+	}
+	return data, err
+}
+
+func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ([]models.Domain, error) {
+	query := fmt.Sprintf("SELECT domain, email, data, status, last_updated, queue_weeks FROM domains WHERE %s", condition)
+	rows, err := db.conn.Query(query, args...)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	domains := []models.Domain{}
+	for rows.Next() {
+		var domain models.Domain
+		var rawMXs string
+		if err := rows.Scan(&domain.Name, &domain.Email, &rawMXs, &domain.State, &domain.LastUpdated, &domain.QueueWeeks); err != nil {
+			return nil, err
+		}
+		domain.MXs = strings.Split(rawMXs, ",")
+		domains = append(domains, domain)
+	}
+	return domains, nil
+}
+
+// =============== models.DomainStore impl ===============
+
+// PutDomain inserts a particular domain into the database. If the domain does
+// not yet exist in the database, we initialize it with StateUnconfirmed
+// If there is already a domain in the database with StateUnconfirmed, performs
+// an update of the fields.
+func (db *SQLDatabase) PutDomain(domain models.Domain) error {
+	_, err := db.conn.Exec("INSERT INTO domains(domain, email, data, status, queue_weeks, mta_sts) "+
+		"VALUES($1, $2, $3, $4, $5, $6) "+
+		"ON CONFLICT ON CONSTRAINT domains_pkey DO UPDATE SET email=$2, data=$3, queue_weeks=$5",
+		domain.Name, domain.Email, strings.Join(domain.MXs[:], ","),
+		models.StateUnconfirmed, domain.QueueWeeks, domain.MTASTS)
+	return err
+}
+
+// GetDomain retrieves the status and information associated with a particular
+// mailserver domain.
+func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.Domain, error) {
+	return db.queryDomain("SELECT %s FROM domains WHERE domain=$1 AND status=$2", domain, state)
+}
+
+// GetDomains retrieves all the domains which match a particular state,
+// that are not in MTA_STS mode
+func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.Domain, error) {
+	return db.queryDomainsWhere("status=$1", state)
+}
+
+// GetMTASTSDomains retrieves domains which wish their policy to be queued with their MTASTS.
+func (db SQLDatabase) GetMTASTSDomains() ([]models.Domain, error) {
+	return db.queryDomainsWhere("mta_sts=TRUE")
+}
+
+// SetStatus sets the status of a particular domain object to |state|.
+func (db SQLDatabase) SetStatus(domain string, state models.DomainState) error {
+	var testingStart time.Time
+	if state == models.StateTesting {
+		testingStart = time.Now()
+	}
+	_, err := db.conn.Exec("UPDATE domains SET status = $1, testing_start = $2 WHERE domain=$3",
+		state, testingStart, domain)
+	return err
+}
+
+// RemoveDomain removes a particular domain and returns it.
+func (db SQLDatabase) RemoveDomain(domain string, state models.DomainState) (models.Domain, error) {
+	return db.queryDomain("DELETE FROM domains WHERE domain=$1 AND status=$2 RETURNING %s")
+}
+
+// DomainsToValidate [interface Validator] retrieves domains from the
+// DB whose policies should be validated.
+func (db SQLDatabase) DomainsToValidate() ([]string, error) {
+	domains := []string{}
+	data, err := db.GetDomains(models.StateTesting)
+	if err != nil {
+		return domains, err
+	}
+	for _, domainInfo := range data {
+		domains = append(domains, domainInfo.Name)
+	}
+	return domains, nil
+}
+
+// HostnamesForDomain [interface Validator] retrieves the hostname policy for
+// a particular domain.
+func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) {
+	data, err := db.GetDomain(domain, models.StateEnforce)
+	if err != nil {
+		data, err = db.GetDomain(domain, models.StateTesting)
+	}
+	if err != nil {
+		return []string{}, err
+	}
+	return data.MXs, nil
+}
diff --git a/db/sqldb.go b/db/sqldb.go
index 9b4117d1..b22f1492 100644
--- a/db/sqldb.go
+++ b/db/sqldb.go
@@ -7,7 +7,6 @@ import (
 	"log"
 	"math/rand"
 	"net/url"
-	"strings"
 	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
@@ -213,54 +212,6 @@ func (db SQLDatabase) GetAllScans(domain string) ([]models.Scan, error) {
 	return scans, nil
 }
 
-// =============== models.DomainStore impl ===============
-
-// PutDomain inserts a particular domain into the database. If the domain does
-// not yet exist in the database, we initialize it with StateUnconfirmed
-// If there is already a domain in the database with StateUnconfirmed, performs
-// an update of the fields.
-func (db *SQLDatabase) PutDomain(domain models.Domain) error {
-	_, err := db.conn.Exec("INSERT INTO domains(domain, email, data, status, queue_weeks, mta_sts) "+
-		"VALUES($1, $2, $3, $4, $5, $6) "+
-		"ON CONFLICT ON CONSTRAINT domains_pkey DO UPDATE SET email=$2, data=$3, queue_weeks=$5",
-		domain.Name, domain.Email, strings.Join(domain.MXs[:], ","),
-		models.StateUnconfirmed, domain.QueueWeeks, domain.MTASTS)
-	return err
-}
-
-// GetDomain retrieves the status and information associated with a particular
-// mailserver domain.
-func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.Domain, error) {
-	return db.queryDomain("SELECT %s FROM domains WHERE domain=$1 AND status=$2", domain, state)
-}
-
-// GetDomains retrieves all the domains which match a particular state,
-// that are not in MTA_STS mode
-func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.Domain, error) {
-	return db.queryDomainsWhere("status=$1", state)
-}
-
-// GetMTASTSDomains retrieves domains which wish their policy to be queued with their MTASTS.
-func (db SQLDatabase) GetMTASTSDomains() ([]models.Domain, error) {
-	return db.queryDomainsWhere("mta_sts=TRUE")
-}
-
-// SetStatus sets the status of a particular domain object to |state|.
-func (db SQLDatabase) SetStatus(domain string, state models.DomainState) error {
-	var testingStart time.Time
-	if state == models.StateTesting {
-		testingStart = time.Now()
-	}
-	_, err := db.conn.Exec("UPDATE domains SET status = $1, testing_start = $2 WHERE domain=$3",
-		state, testingStart, domain)
-	return err
-}
-
-// RemoveDomain removes a particular domain and returns it.
-func (db SQLDatabase) RemoveDomain(domain string, state models.DomainState) (models.Domain, error) {
-	return db.queryDomain("DELETE FROM domains WHERE domain=$1 AND status=$2 RETURNING %s")
-}
-
 // EMAIL BLACKLIST DB FUNCTIONS
 
 // PutBlacklistedEmail adds a bounce or complaint notification to the email blacklist.
@@ -304,66 +255,6 @@ func (db SQLDatabase) ClearTables() error {
 	})
 }
 
-func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.Domain, error) {
-	query := fmt.Sprintf(sqlQuery, "domain, email, data, status, last_updated, queue_weeks")
-	data := models.Domain{}
-	var rawMXs string
-	err := db.conn.QueryRow(query, args...).Scan(
-		&data.Name, &data.Email, &rawMXs, &data.State, &data.LastUpdated, &data.QueueWeeks)
-	data.MXs = strings.Split(rawMXs, ",")
-	if len(rawMXs) == 0 {
-		data.MXs = []string{}
-	}
-	return data, err
-}
-
-func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ([]models.Domain, error) {
-	query := fmt.Sprintf("SELECT domain, email, data, status, last_updated, queue_weeks FROM domains WHERE %s", condition)
-	rows, err := db.conn.Query(query, args...)
-	if err != nil {
-		return nil, err
-	}
-	defer rows.Close()
-	domains := []models.Domain{}
-	for rows.Next() {
-		var domain models.Domain
-		var rawMXs string
-		if err := rows.Scan(&domain.Name, &domain.Email, &rawMXs, &domain.State, &domain.LastUpdated, &domain.QueueWeeks); err != nil {
-			return nil, err
-		}
-		domain.MXs = strings.Split(rawMXs, ",")
-		domains = append(domains, domain)
-	}
-	return domains, nil
-}
-
-// DomainsToValidate [interface Validator] retrieves domains from the
-// DB whose policies should be validated.
-func (db SQLDatabase) DomainsToValidate() ([]string, error) {
-	domains := []string{}
-	data, err := db.GetDomains(models.StateTesting)
-	if err != nil {
-		return domains, err
-	}
-	for _, domainInfo := range data {
-		domains = append(domains, domainInfo.Name)
-	}
-	return domains, nil
-}
-
-// HostnamesForDomain [interface Validator] retrieves the hostname policy for
-// a particular domain.
-func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) {
-	data, err := db.GetDomain(domain, models.StateEnforce)
-	if err != nil {
-		data, err = db.GetDomain(domain, models.StateTesting)
-	}
-	if err != nil {
-		return []string{}, err
-	}
-	return data.MXs, nil
-}
-
 // GetHostnameScan retrives most recent scan from database.
 func (db *SQLDatabase) GetHostnameScan(hostname string) (checker.HostnameResult, error) {
 	result := checker.HostnameResult{
diff --git a/models/domain.go b/models/policy.go
similarity index 100%
rename from models/domain.go
rename to models/policy.go
diff --git a/models/domain_test.go b/models/policy_test.go
similarity index 100%
rename from models/domain_test.go
rename to models/policy_test.go

From d84755659aaad125ede7eb9e2d5e6528945fe53c Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Wed, 19 Jun 2019 16:20:54 -0700
Subject: [PATCH 02/12] s/Domain/PolicySubmission

---
 api.go                     | 12 ++++++------
 db/db.go                   |  8 ++++----
 db/policy.go               | 20 ++++++++++----------
 db/scripts/init_tables.sql | 19 +++++++++++++++++++
 db/sqldb_test.go           | 28 ++++++++++++++--------------
 email.go                   |  4 ++--
 main_test.go               |  2 +-
 models/policy.go           | 30 ++++++++++++------------------
 models/policy_test.go      | 24 ++++++++++++------------
 models/token_test.go       |  2 +-
 queue_test.go              |  2 +-
 11 files changed, 82 insertions(+), 69 deletions(-)

diff --git a/api.go b/api.go
index 960d99f2..98ca0a2b 100644
--- a/api.go
+++ b/api.go
@@ -61,7 +61,7 @@ type PolicyList interface {
 type EmailSender interface {
 	// SendValidation sends a validation e-mail for a particular domain,
 	// with a particular validation token.
-	SendValidation(*models.Domain, string) error
+	SendValidation(*models.PolicySubmission, string) error
 }
 
 // APIResponse wraps all the responses from this API.
@@ -90,7 +90,7 @@ func (api *API) wrapper(handler apiHandler) func(w http.ResponseWriter, r *http.
 }
 
 func defaultCheck(api API, domain string) (checker.DomainResult, error) {
-	policyChan := models.Domain{Name: domain}.AsyncPolicyListCheck(api.Database, api.List)
+	policyChan := models.PolicySubmission{Name: domain}.AsyncPolicyListCheck(api.Database, api.List)
 	c := checker.Checker{
 		Cache: &checker.ScanCache{
 			ScanStore:  api.Database,
@@ -173,13 +173,13 @@ const MaxHostnames = 8
 
 // Extracts relevant parameters from http.Request for a POST to /api/queue
 // TODO: also validate hostnames as FQDNs.
-func getDomainParams(r *http.Request) (models.Domain, error) {
+func getDomainParams(r *http.Request) (models.PolicySubmission, error) {
 	name, err := getASCIIDomain(r)
 	if err != nil {
-		return models.Domain{}, err
+		return models.PolicySubmission{}, err
 	}
 	mtasts := r.FormValue("mta-sts")
-	domain := models.Domain{
+	domain := models.PolicySubmission{
 		Name:   name,
 		MTASTS: mtasts == "on",
 		State:  models.StateUnconfirmed,
@@ -221,7 +221,7 @@ func getDomainParams(r *http.Request) (models.Domain, error) {
 //        domain: Mail domain to queue a TLS policy for.
 //				mta_sts: "on" if domain supports MTA-STS, else "".
 //        hostnames: List of MX hostnames to put into this domain's TLS policy. Up to 8.
-//        Sets models.Domain object as response.
+//        Sets models.PolicySubmission object as response.
 //        weeks (optional, default 4): How many weeks is this domain queued for.
 //        email (optional): Contact email associated with domain.
 //   GET  /api/queue?domain=<domain>
diff --git a/db/db.go b/db/db.go
index 2bf0724d..cfdbbc04 100644
--- a/db/db.go
+++ b/db/db.go
@@ -40,13 +40,13 @@ type Database interface {
 	// Gets counts per day of hosts supporting MTA-STS for a given source.
 	GetStats(string) (stats.Series, error)
 	// Upserts domain state.
-	PutDomain(models.Domain) error
+	PutDomain(models.PolicySubmission) error
 	// Retrieves state of a domain
-	GetDomain(string, models.DomainState) (models.Domain, error)
+	GetDomain(string, models.DomainState) (models.PolicySubmission, error)
 	// Retrieves all domains in a particular state.
-	GetDomains(models.DomainState) ([]models.Domain, error)
+	GetDomains(models.DomainState) ([]models.PolicySubmission, error)
 	SetStatus(string, models.DomainState) error
-	RemoveDomain(string, models.DomainState) (models.Domain, error)
+	RemoveDomain(string, models.DomainState) (models.PolicySubmission, error)
 	ClearTables() error
 }
 
diff --git a/db/policy.go b/db/policy.go
index 2505deab..7cf18930 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -8,9 +8,9 @@ import (
 	"github.com/EFForg/starttls-backend/models"
 )
 
-func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.Domain, error) {
+func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.PolicySubmission, error) {
 	query := fmt.Sprintf(sqlQuery, "domain, email, data, status, last_updated, queue_weeks")
-	data := models.Domain{}
+	data := models.PolicySubmission{}
 	var rawMXs string
 	err := db.conn.QueryRow(query, args...).Scan(
 		&data.Name, &data.Email, &rawMXs, &data.State, &data.LastUpdated, &data.QueueWeeks)
@@ -21,16 +21,16 @@ func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.
 	return data, err
 }
 
-func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ([]models.Domain, error) {
+func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ([]models.PolicySubmission, error) {
 	query := fmt.Sprintf("SELECT domain, email, data, status, last_updated, queue_weeks FROM domains WHERE %s", condition)
 	rows, err := db.conn.Query(query, args...)
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
-	domains := []models.Domain{}
+	domains := []models.PolicySubmission{}
 	for rows.Next() {
-		var domain models.Domain
+		var domain models.PolicySubmission
 		var rawMXs string
 		if err := rows.Scan(&domain.Name, &domain.Email, &rawMXs, &domain.State, &domain.LastUpdated, &domain.QueueWeeks); err != nil {
 			return nil, err
@@ -47,7 +47,7 @@ func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) (
 // not yet exist in the database, we initialize it with StateUnconfirmed
 // If there is already a domain in the database with StateUnconfirmed, performs
 // an update of the fields.
-func (db *SQLDatabase) PutDomain(domain models.Domain) error {
+func (db *SQLDatabase) PutDomain(domain models.PolicySubmission) error {
 	_, err := db.conn.Exec("INSERT INTO domains(domain, email, data, status, queue_weeks, mta_sts) "+
 		"VALUES($1, $2, $3, $4, $5, $6) "+
 		"ON CONFLICT ON CONSTRAINT domains_pkey DO UPDATE SET email=$2, data=$3, queue_weeks=$5",
@@ -58,18 +58,18 @@ func (db *SQLDatabase) PutDomain(domain models.Domain) error {
 
 // GetDomain retrieves the status and information associated with a particular
 // mailserver domain.
-func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.Domain, error) {
+func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.PolicySubmission, error) {
 	return db.queryDomain("SELECT %s FROM domains WHERE domain=$1 AND status=$2", domain, state)
 }
 
 // GetDomains retrieves all the domains which match a particular state,
 // that are not in MTA_STS mode
-func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.Domain, error) {
+func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.PolicySubmission, error) {
 	return db.queryDomainsWhere("status=$1", state)
 }
 
 // GetMTASTSDomains retrieves domains which wish their policy to be queued with their MTASTS.
-func (db SQLDatabase) GetMTASTSDomains() ([]models.Domain, error) {
+func (db SQLDatabase) GetMTASTSDomains() ([]models.PolicySubmission, error) {
 	return db.queryDomainsWhere("mta_sts=TRUE")
 }
 
@@ -85,7 +85,7 @@ func (db SQLDatabase) SetStatus(domain string, state models.DomainState) error {
 }
 
 // RemoveDomain removes a particular domain and returns it.
-func (db SQLDatabase) RemoveDomain(domain string, state models.DomainState) (models.Domain, error) {
+func (db SQLDatabase) RemoveDomain(domain string, state models.DomainState) (models.PolicySubmission, error) {
 	return db.queryDomain("DELETE FROM domains WHERE domain=$1 AND status=$2 RETURNING %s")
 }
 
diff --git a/db/scripts/init_tables.sql b/db/scripts/init_tables.sql
index 01b7dfb2..ac3425cb 100644
--- a/db/scripts/init_tables.sql
+++ b/db/scripts/init_tables.sql
@@ -48,6 +48,25 @@ CREATE TABLE IF NOT EXISTS blacklisted_emails
     timestamp   TIMESTAMP
 );
 
+CREATE TABLE IF NOT EXISTS pending_policies
+(
+    domain        TEXT NOT NULL PRIMARY KEY,
+    email         TEXT NOT NULL,
+    mta_sts       BOOLEAN DEFAULT FALSE,
+    mxs           TEXT NOT NULL,
+    mode          VARCHAR(255) NOT NULL,
+);
+
+
+CREATE TABLE IF NOT EXISTS policies
+(
+    domain        TEXT NOT NULL PRIMARY KEY,
+    email         TEXT NOT NULL,
+    mta_sts       BOOLEAN DEFAULT FALSE,
+    mxs           TEXT NOT NULL,
+    mode          VARCHAR(255) NOT NULL,
+);
+
 -- Schema change: add "last_updated" timestamp column if it doesn't exist.
 
 ALTER TABLE domains ADD COLUMN IF NOT EXISTS last_updated TIMESTAMP DEFAULT CURRENT_TIMESTAMP;
diff --git a/db/sqldb_test.go b/db/sqldb_test.go
index 6d7df677..4c23086c 100644
--- a/db/sqldb_test.go
+++ b/db/sqldb_test.go
@@ -139,7 +139,7 @@ func TestGetAllScans(t *testing.T) {
 
 func TestPutGetDomain(t *testing.T) {
 	database.ClearTables()
-	data := models.Domain{
+	data := models.PolicySubmission{
 		Name:  "testing.com",
 		Email: "admin@testing.com",
 	}
@@ -161,13 +161,13 @@ func TestPutGetDomain(t *testing.T) {
 
 func TestUpsertDomain(t *testing.T) {
 	database.ClearTables()
-	data := models.Domain{
+	data := models.PolicySubmission{
 		Name:  "testing.com",
 		MXs:   []string{"hello1"},
 		Email: "admin@testing.com",
 	}
 	database.PutDomain(data)
-	err := database.PutDomain(models.Domain{Name: "testing.com", MXs: []string{"hello_darkness_my_old_friend"}, Email: "actual_admin@testing.com"})
+	err := database.PutDomain(models.PolicySubmission{Name: "testing.com", MXs: []string{"hello_darkness_my_old_friend"}, Email: "actual_admin@testing.com"})
 	if err != nil {
 		t.Errorf("PutDomain(%s) failed: %v\n", data.Name, err)
 	}
@@ -214,7 +214,7 @@ func TestPutTokenTwice(t *testing.T) {
 
 func TestLastUpdatedFieldUpdates(t *testing.T) {
 	database.ClearTables()
-	data := models.Domain{
+	data := models.PolicySubmission{
 		Name:  "testing.com",
 		Email: "admin@testing.com",
 		State: models.StateUnconfirmed,
@@ -223,7 +223,7 @@ func TestLastUpdatedFieldUpdates(t *testing.T) {
 	retrievedData, _ := database.GetDomain(data.Name, models.StateUnconfirmed)
 	lastUpdated := retrievedData.LastUpdated
 	data.State = models.StateTesting
-	database.PutDomain(models.Domain{Name: data.Name, Email: "new fone who dis"})
+	database.PutDomain(models.PolicySubmission{Name: data.Name, Email: "new fone who dis"})
 	retrievedData, _ = database.GetDomain(data.Name, models.StateUnconfirmed)
 	if lastUpdated.Equal(retrievedData.LastUpdated) {
 		t.Errorf("Expected last_updated to be updated on change: %v", lastUpdated)
@@ -232,7 +232,7 @@ func TestLastUpdatedFieldUpdates(t *testing.T) {
 
 func TestLastUpdatedFieldDoesntUpdate(t *testing.T) {
 	database.ClearTables()
-	data := models.Domain{
+	data := models.PolicySubmission{
 		Name:  "testing.com",
 		Email: "admin@testing.com",
 		State: models.StateUnconfirmed,
@@ -254,9 +254,9 @@ func TestDomainsToValidate(t *testing.T) {
 	}
 	for domain, queued := range queuedMap {
 		if queued {
-			database.PutDomain(models.Domain{Name: domain, State: models.StateTesting})
+			database.PutDomain(models.PolicySubmission{Name: domain, State: models.StateTesting})
 		} else {
-			database.PutDomain(models.Domain{Name: domain})
+			database.PutDomain(models.PolicySubmission{Name: domain})
 		}
 	}
 	result, err := database.DomainsToValidate()
@@ -272,8 +272,8 @@ func TestDomainsToValidate(t *testing.T) {
 
 func TestHostnamesForDomain(t *testing.T) {
 	database.ClearTables()
-	database.PutDomain(models.Domain{Name: "x", MXs: []string{"x.com", "y.org"}})
-	database.PutDomain(models.Domain{Name: "y"})
+	database.PutDomain(models.PolicySubmission{Name: "x", MXs: []string{"x.com", "y.org"}})
+	database.PutDomain(models.PolicySubmission{Name: "y"})
 	database.SetStatus("x", models.StateTesting)
 	database.SetStatus("y", models.StateTesting)
 	result, err := database.HostnamesForDomain("x")
@@ -477,10 +477,10 @@ func TestGetLocalStats(t *testing.T) {
 
 func TestGetMTASTSDomains(t *testing.T) {
 	database.ClearTables()
-	database.PutDomain(models.Domain{Name: "unicorns"})
-	database.PutDomain(models.Domain{Name: "mta-sts-x", MTASTS: true})
-	database.PutDomain(models.Domain{Name: "mta-sts-y", MTASTS: true})
-	database.PutDomain(models.Domain{Name: "regular"})
+	database.PutDomain(models.PolicySubmission{Name: "unicorns"})
+	database.PutDomain(models.PolicySubmission{Name: "mta-sts-x", MTASTS: true})
+	database.PutDomain(models.PolicySubmission{Name: "mta-sts-y", MTASTS: true})
+	database.PutDomain(models.PolicySubmission{Name: "regular"})
 	domains, err := database.GetMTASTSDomains()
 	if err != nil {
 		t.Fatalf("GetMTASTSDomains() failed: %v", err)
diff --git a/email.go b/email.go
index b02d45f7..d29272a8 100644
--- a/email.go
+++ b/email.go
@@ -88,7 +88,7 @@ func makeEmailConfigFromEnv(database db.Database) (emailConfig, error) {
 	return c, nil
 }
 
-func validationAddress(domain *models.Domain) string {
+func validationAddress(domain *models.PolicySubmission) string {
 	return fmt.Sprintf("postmaster@%s", domain.Name)
 }
 
@@ -99,7 +99,7 @@ func validationEmailText(domain string, contactEmail string, hostnames []string,
 
 // SendValidation sends a validation e-mail for the domain outlined by domainInfo.
 // The validation link is generated using a token.
-func (c emailConfig) SendValidation(domain *models.Domain, token string) error {
+func (c emailConfig) SendValidation(domain *models.PolicySubmission, token string) error {
 	emailContent := validationEmailText(domain.Name, domain.Email, domain.MXs, token,
 		c.website)
 	return c.sendEmail(validationEmailSubject, emailContent, validationAddress(domain))
diff --git a/main_test.go b/main_test.go
index f470ff77..8da8767a 100644
--- a/main_test.go
+++ b/main_test.go
@@ -55,7 +55,7 @@ func (l mockList) HasDomain(domain string) bool {
 // Mock emailer
 type mockEmailer struct{}
 
-func (e mockEmailer) SendValidation(domain *models.Domain, token string) error { return nil }
+func (e mockEmailer) SendValidation(domain *models.PolicySubmission, token string) error { return nil }
 
 // Load env. vars, initialize DB hook, and tests API
 func TestMain(m *testing.M) {
diff --git a/models/policy.go b/models/policy.go
index b12e639b..c6ea422c 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -8,14 +8,8 @@ import (
 	"github.com/EFForg/starttls-backend/checker"
 )
 
-/* Domain represents an email domain's TLS policy.
- *
- * If there's a Domain object for a particular email domain in "Enforce" mode,
- * that email domain's policy is fixed and cannot be changed.
- */
-
-// Domain stores the preload state of a single domain.
-type Domain struct {
+// PolicySubmission represents an email domain's TLS policy submission.
+type PolicySubmission struct {
 	Name         string      `json:"domain"` // Domain that is preloaded
 	Email        string      `json:"-"`      // Contact e-mail for Domain
 	MXs          []string    `json:"mxs"`    // MXs that are valid for this domain
@@ -28,11 +22,11 @@ type Domain struct {
 
 // domainStore is a simple interface for fetching and adding domain objects.
 type domainStore interface {
-	PutDomain(Domain) error
-	GetDomain(string, DomainState) (Domain, error)
-	GetDomains(DomainState) ([]Domain, error)
+	PutDomain(PolicySubmission) error
+	GetDomain(string, DomainState) (PolicySubmission, error)
+	GetDomains(DomainState) ([]PolicySubmission, error)
 	SetStatus(string, DomainState) error
-	RemoveDomain(string, DomainState) (Domain, error)
+	RemoveDomain(string, DomainState) (PolicySubmission, error)
 }
 
 // DomainState represents the state of a single domain.
@@ -56,7 +50,7 @@ type policyList interface {
 // A successful scan should already have been submitted for this domain,
 // and it should not already be on the policy list.
 // Returns (queuability, error message, and most recent scan)
-func (d *Domain) IsQueueable(domains domainStore, scans scanStore, list policyList) (bool, string, Scan) {
+func (d *PolicySubmission) IsQueueable(domains domainStore, scans scanStore, list policyList) (bool, string, Scan) {
 	scan, err := scans.GetLatestScan(d.Name)
 	if err != nil {
 		return false, "We haven't scanned this domain yet. " +
@@ -86,7 +80,7 @@ func (d *Domain) IsQueueable(domains domainStore, scans scanStore, list policyLi
 }
 
 // PopulateFromScan updates a Domain's fields based on a scan of that domain.
-func (d *Domain) PopulateFromScan(scan Scan) {
+func (d *PolicySubmission) PopulateFromScan(scan Scan) {
 	// We should only trust MTA-STS info from a successful MTA-STS check.
 	if d.MTASTS && scan.SupportsMTASTS() {
 		// If the domain's MXs are missing, we can take them from the scan's
@@ -100,7 +94,7 @@ func (d *Domain) PopulateFromScan(scan Scan) {
 
 // InitializeWithToken adds this domain to the given DomainStore and initializes a validation token
 // for the addition. The newly generated Token is returned.
-func (d *Domain) InitializeWithToken(store domainStore, tokens tokenStore) (string, error) {
+func (d *PolicySubmission) InitializeWithToken(store domainStore, tokens tokenStore) (string, error) {
 	if err := store.PutDomain(*d); err != nil {
 		return "", err
 	}
@@ -112,7 +106,7 @@ func (d *Domain) InitializeWithToken(store domainStore, tokens tokenStore) (stri
 }
 
 // PolicyListCheck checks the policy list status of this particular domain.
-func (d *Domain) PolicyListCheck(store domainStore, list policyList) *checker.Result {
+func (d *PolicySubmission) PolicyListCheck(store domainStore, list policyList) *checker.Result {
 	result := checker.Result{Name: checker.PolicyList}
 	if list.HasDomain(d.Name) {
 		return result.Success()
@@ -136,7 +130,7 @@ func (d *Domain) PolicyListCheck(store domainStore, list policyList) *checker.Re
 
 // AsyncPolicyListCheck performs PolicyListCheck asynchronously.
 // domainStore and policyList should be safe for concurrent use.
-func (d Domain) AsyncPolicyListCheck(store domainStore, list policyList) <-chan checker.Result {
+func (d PolicySubmission) AsyncPolicyListCheck(store domainStore, list policyList) <-chan checker.Result {
 	result := make(chan checker.Result)
 	go func() { result <- *d.PolicyListCheck(store, list) }()
 	return result
@@ -146,7 +140,7 @@ func (d Domain) AsyncPolicyListCheck(store domainStore, list policyList) <-chan
 // At any given time, there can only be one domain that's either StateEnforce
 // or StateTesting. If that domain exists in the store, return that one.
 // Otherwise, look for a Domain policy in the unconfirmed state.
-func GetDomain(store domainStore, name string) (Domain, error) {
+func GetDomain(store domainStore, name string) (PolicySubmission, error) {
 	domain, err := store.GetDomain(name, StateEnforce)
 	if err == nil {
 		return domain, nil
diff --git a/models/policy_test.go b/models/policy_test.go
index 0dea7490..6c8b43e4 100644
--- a/models/policy_test.go
+++ b/models/policy_test.go
@@ -9,12 +9,12 @@ import (
 )
 
 type mockDomainStore struct {
-	domain  Domain
-	domains []Domain
+	domain  PolicySubmission
+	domains []PolicySubmission
 	err     error
 }
 
-func (m *mockDomainStore) PutDomain(d Domain) error {
+func (m *mockDomainStore) PutDomain(d PolicySubmission) error {
 	m.domain = d
 	return m.err
 }
@@ -24,7 +24,7 @@ func (m *mockDomainStore) SetStatus(d string, status DomainState) error {
 	return m.err
 }
 
-func (m *mockDomainStore) GetDomain(d string, state DomainState) (Domain, error) {
+func (m *mockDomainStore) GetDomain(d string, state DomainState) (PolicySubmission, error) {
 	domain := m.domain
 	if state != domain.State {
 		return m.domain, errors.New("")
@@ -32,11 +32,11 @@ func (m *mockDomainStore) GetDomain(d string, state DomainState) (Domain, error)
 	return m.domain, nil
 }
 
-func (m *mockDomainStore) GetDomains(_ DomainState) ([]Domain, error) {
+func (m *mockDomainStore) GetDomains(_ DomainState) ([]PolicySubmission, error) {
 	return m.domains, m.err
 }
 
-func (m *mockDomainStore) RemoveDomain(d string, state DomainState) (Domain, error) {
+func (m *mockDomainStore) RemoveDomain(d string, state DomainState) (PolicySubmission, error) {
 	domain := m.domain
 	if state != domain.State {
 		return m.domain, errors.New("")
@@ -59,7 +59,7 @@ func (m mockScanStore) GetLatestScan(string) (Scan, error) { return m.scan, m.er
 
 func TestIsQueueable(t *testing.T) {
 	// With supplied hostnames
-	d := Domain{
+	d := PolicySubmission{
 		Name:  "example.com",
 		Email: "me@example.com",
 		MXs:   []string{".example.com"},
@@ -107,7 +107,7 @@ func TestIsQueueable(t *testing.T) {
 			ok: false, msg: "do not match policy"},
 	}
 	for _, tc := range testCases {
-		domainStore := mockDomainStore{domain: Domain{State: tc.state}}
+		domainStore := mockDomainStore{domain: PolicySubmission{State: tc.state}}
 		ok, msg, _ := d.IsQueueable(&domainStore, mockScanStore{tc.scan, tc.scanErr}, mockList{tc.onList})
 		if ok != tc.ok {
 			t.Error(tc.name)
@@ -117,7 +117,7 @@ func TestIsQueueable(t *testing.T) {
 		}
 	}
 	// With MTA-STS
-	d = Domain{
+	d = PolicySubmission{
 		Name:   "example.com",
 		Email:  "me@example.com",
 		MTASTS: true,
@@ -143,7 +143,7 @@ func TestIsQueueable(t *testing.T) {
 }
 
 func TestPopulateFromScan(t *testing.T) {
-	d := Domain{
+	d := PolicySubmission{
 		Name:   "example.com",
 		Email:  "me@example.com",
 		MTASTS: true,
@@ -177,7 +177,7 @@ func TestPolicyCheck(t *testing.T) {
 		{"Domain not currently in the DB or on the list should return a failure", false, StateUnconfirmed, false, checker.Failure},
 	}
 	for _, tc := range testCases {
-		domainObj := Domain{Name: "example.com", State: tc.state}
+		domainObj := PolicySubmission{Name: "example.com", State: tc.state}
 		var dbErr error
 		if !tc.inDB {
 			dbErr = errors.New("")
@@ -191,7 +191,7 @@ func TestPolicyCheck(t *testing.T) {
 
 func TestInitializeWithToken(t *testing.T) {
 	mockToken := mockTokenStore{domain: "domain", err: nil}
-	domainObj := Domain{Name: "example.com"}
+	domainObj := PolicySubmission{Name: "example.com"}
 	// domainStore returns error
 	_, err := domainObj.InitializeWithToken(&mockDomainStore{domain: domainObj, err: errors.New("")}, &mockToken)
 	if err == nil {
diff --git a/models/token_test.go b/models/token_test.go
index 2ecd568f..b6e93671 100644
--- a/models/token_test.go
+++ b/models/token_test.go
@@ -21,7 +21,7 @@ func (m *mockTokenStore) UseToken(token string) (string, error) {
 }
 
 func TestRedeemToken(t *testing.T) {
-	domains := mockDomainStore{domain: Domain{Name: "anything", State: StateUnconfirmed}, err: nil}
+	domains := mockDomainStore{domain: PolicySubmission{Name: "anything", State: StateUnconfirmed}, err: nil}
 	token := Token{Token: "token"}
 	domain, userErr, dbErr := token.Redeem(&domains, &mockTokenStore{domain: "anything", err: nil})
 	if domain != "anything" || userErr != nil || dbErr != nil {
diff --git a/queue_test.go b/queue_test.go
index 15f92a0c..9a2537e4 100644
--- a/queue_test.go
+++ b/queue_test.go
@@ -129,7 +129,7 @@ func TestBasicQueueWorkflow(t *testing.T) {
 
 	// 2-T. Check to see domain status was initialized to 'unvalidated'
 	domainBody, _ := ioutil.ReadAll(resp.Body)
-	domain := models.Domain{}
+	domain := models.PolicySubmission{}
 	err := json.Unmarshal(domainBody, &APIResponse{Response: &domain})
 	if err != nil {
 		t.Fatalf("Returned invalid JSON object:%v\n", string(domainBody))

From 8ca7acbf94e62fcd298f09e307bbe0589174ef9c Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Thu, 20 Jun 2019 11:50:14 -0700
Subject: [PATCH 03/12] Create new policy model abstraction from domain model

---
 models/policy.go | 153 ++++++++++++++++++-----------------------------
 models/token.go  |  11 ++--
 2 files changed, 62 insertions(+), 102 deletions(-)

diff --git a/models/policy.go b/models/policy.go
index c6ea422c..b9874586 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -2,100 +2,89 @@ package models
 
 import (
 	"fmt"
-	"log"
 	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
+	"github.com/EFForg/starttls-backend/policy"
 )
 
 // PolicySubmission represents an email domain's TLS policy submission.
 type PolicySubmission struct {
-	Name         string      `json:"domain"` // Domain that is preloaded
-	Email        string      `json:"-"`      // Contact e-mail for Domain
-	MXs          []string    `json:"mxs"`    // MXs that are valid for this domain
-	MTASTS       bool        `json:"mta_sts"`
-	State        DomainState `json:"state"`
-	LastUpdated  time.Time   `json:"last_updated"`
-	TestingStart time.Time   `json:"-"`
-	QueueWeeks   int         `json:"queue_weeks"`
+	Name   string `json:"domain"` // Domain that is preloaded
+	Email  string `json:"-"`      // Contact e-mail for Domain
+	MTASTS bool   `json:"mta_sts"`
+	Policy *policy.TLSPolicy
 }
 
-// domainStore is a simple interface for fetching and adding domain objects.
-type domainStore interface {
-	PutDomain(PolicySubmission) error
-	GetDomain(string, DomainState) (PolicySubmission, error)
-	GetDomains(DomainState) ([]PolicySubmission, error)
-	SetStatus(string, DomainState) error
-	RemoveDomain(string, DomainState) (PolicySubmission, error)
+// policyStore is a simple interface for fetching and adding domain objects.
+type policyStore interface {
+	PutOrUpdatePolicy(*PolicySubmission) error
+	GetPolicy(string) (PolicySubmission, error)
+	GetPolicies(bool) ([]PolicySubmission, error)
+	RemovePolicy(string) (PolicySubmission, error)
 }
 
-// DomainState represents the state of a single domain.
-type DomainState string
-
-// Possible values for DomainState
-const (
-	StateUnknown     = "unknown"     // Domain was never submitted, so we don't know.
-	StateUnconfirmed = "unvalidated" // Administrator has not yet confirmed their intention to add the domain.
-	StateTesting     = "queued"      // Queued for addition at next addition date pending continued validation
-	StateFailed      = "failed"      // Requested to be queued, but failed verification.
-	StateEnforce     = "added"       // On the list.
-)
-
 type policyList interface {
 	HasDomain(string) bool
 }
 
-// IsQueueable returns true if a domain can be submitted for validation and
-// queueing to the STARTTLS Everywhere Policy List.
-// A successful scan should already have been submitted for this domain,
-// and it should not already be on the policy list.
-// Returns (queuability, error message, and most recent scan)
-func (d *PolicySubmission) IsQueueable(domains domainStore, scans scanStore, list policyList) (bool, string, Scan) {
+// CanUpdate returns whether we can update the policyStore with this particular
+// Policy Submission. In some cases, you should be able to update the
+// existing policy. In other cases, you shouldn't.
+// TODO: test
+func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
+	oldPolicy, err := policies.GetPolicy(p.Name)
+	// If this policy doesn't exist in the policyStore, we can add it.
+	if err != nil {
+		return true
+	}
+	// If the policies are the same, return true if emails are different.
+	if (oldPolicy.MTASTS && p.MTASTS) || oldPolicy.Policy.HostnamesEqual(p.Policy) {
+		return oldPolicy.Email != p.Email
+	}
+	// If old policy is manual and in testing, we can update it safely.
+	if !oldPolicy.MTASTS && oldPolicy.Policy.Mode == "testing" {
+		return true
+	}
+	return false
+}
+
+// HasValidScan checks whether this policy already has a recent scan as an initial
+// sanity check. This function isn't meant to be bullet-proof since state can change
+// between initial submission and final addition to the list, but we can short-circuit
+// premature failures here on initial submission.
+func (d *PolicySubmission) HasValidScan(scans scanStore) (bool, string) {
 	scan, err := scans.GetLatestScan(d.Name)
 	if err != nil {
 		return false, "We haven't scanned this domain yet. " +
 			"Please use the STARTTLS checker to scan your domain's " +
-			"STARTTLS configuration so we can validate your submission", scan
-	}
-	if scan.Data.Status != 0 {
-		return false, "Domain hasn't passed our STARTTLS security checks", scan
+			"STARTTLS configuration so we can validate your submission"
 	}
-	if list.HasDomain(d.Name) {
-		return false, "Domain is already on the policy list!", scan
+	if scan.Timestamp.Add(time.Minute * 10).Before(time.Now()) {
+		return false, "We haven't scanned this domain recently. " +
+			"Please use the STARTTLS checker to scan your domain's " +
+			"STARTTLS configuration so we can validate your submission"
 	}
-	if _, err := domains.GetDomain(d.Name, StateEnforce); err == nil {
-		return false, "Domain is already on the policy list!", scan
+	if scan.Data.Status != 0 {
+		return false, "Domain hasn't passed our STARTTLS security checks"
 	}
 	// Domains without submitted MTA-STS support must match provided mx patterns.
 	if !d.MTASTS {
 		for _, hostname := range scan.Data.PreferredHostnames {
-			if !checker.PolicyMatches(hostname, d.MXs) {
-				return false, fmt.Sprintf("Hostnames %v do not match policy %v", scan.Data.PreferredHostnames, d.MXs), scan
+			if !checker.PolicyMatches(hostname, d.Policy.MXs) {
+				return false, fmt.Sprintf("Hostnames %v do not match policy %v", scan.Data.PreferredHostnames, d.Policy.MXs)
 			}
 		}
 	} else if !scan.SupportsMTASTS() {
-		return false, "Domain does not correctly implement MTA-STS.", scan
-	}
-	return true, "", scan
-}
-
-// PopulateFromScan updates a Domain's fields based on a scan of that domain.
-func (d *PolicySubmission) PopulateFromScan(scan Scan) {
-	// We should only trust MTA-STS info from a successful MTA-STS check.
-	if d.MTASTS && scan.SupportsMTASTS() {
-		// If the domain's MXs are missing, we can take them from the scan's
-		// PreferredHostnames, which must be a subset of those listed in the
-		// MTA-STS policy file.
-		if len(d.MXs) == 0 {
-			d.MXs = scan.Data.MTASTSResult.MXs
-		}
+		return false, "Domain does not correctly implement MTA-STS."
 	}
+	return true, ""
 }
 
 // InitializeWithToken adds this domain to the given DomainStore and initializes a validation token
 // for the addition. The newly generated Token is returned.
-func (d *PolicySubmission) InitializeWithToken(store domainStore, tokens tokenStore) (string, error) {
-	if err := store.PutDomain(*d); err != nil {
+func (d *PolicySubmission) InitializeWithToken(pending policyStore, tokens tokenStore) (string, error) {
+	if err := pending.PutOrUpdatePolicy(d); err != nil {
 		return "", err
 	}
 	token, err := tokens.PutToken(d.Name)
@@ -106,52 +95,26 @@ func (d *PolicySubmission) InitializeWithToken(store domainStore, tokens tokenSt
 }
 
 // PolicyListCheck checks the policy list status of this particular domain.
-func (d *PolicySubmission) PolicyListCheck(store domainStore, list policyList) *checker.Result {
+func (d *PolicySubmission) PolicyListCheck(pending policyStore, store policyStore, list policyList) *checker.Result {
 	result := checker.Result{Name: checker.PolicyList}
 	if list.HasDomain(d.Name) {
 		return result.Success()
 	}
-	domain, err := GetDomain(store, d.Name)
+	_, err := store.GetPolicy(d.Name)
 	if err != nil {
-		return result.Failure("Domain %s is not on the policy list.", d.Name)
-	}
-	if domain.State == StateEnforce {
-		log.Println("Warning: Domain was StateEnforce in DB but was not found on the policy list.")
-		return result.Success()
-	}
-	if domain.State == StateTesting {
-		return result.Warning("Domain %s is queued to be added to the policy list.", d.Name)
+		return result.Warning("Domain %s should soon be added to the policy list.", d.Name)
 	}
-	if domain.State == StateUnconfirmed {
-		return result.Failure("The policy addition request for %s is waiting on email validation", d.Name)
+	_, err = pending.GetPolicy(d.Name)
+	if err != nil {
+		return result.Failure("The policy submission for %s is waiting on email validation.", d.Name)
 	}
 	return result.Failure("Domain %s is not on the policy list.", d.Name)
 }
 
 // AsyncPolicyListCheck performs PolicyListCheck asynchronously.
 // domainStore and policyList should be safe for concurrent use.
-func (d PolicySubmission) AsyncPolicyListCheck(store domainStore, list policyList) <-chan checker.Result {
+func (d PolicySubmission) AsyncPolicyListCheck(pending policyStore, store policyStore, list policyList) <-chan checker.Result {
 	result := make(chan checker.Result)
-	go func() { result <- *d.PolicyListCheck(store, list) }()
+	go func() { result <- *d.PolicyListCheck(pending, store, list) }()
 	return result
 }
-
-// GetDomain retrieves Domain with the most "important" state.
-// At any given time, there can only be one domain that's either StateEnforce
-// or StateTesting. If that domain exists in the store, return that one.
-// Otherwise, look for a Domain policy in the unconfirmed state.
-func GetDomain(store domainStore, name string) (PolicySubmission, error) {
-	domain, err := store.GetDomain(name, StateEnforce)
-	if err == nil {
-		return domain, nil
-	}
-	domain, err = store.GetDomain(name, StateTesting)
-	if err == nil {
-		return domain, nil
-	}
-	domain, err = store.GetDomain(name, StateUnconfirmed)
-	if err == nil {
-		return domain, nil
-	}
-	return store.GetDomain(name, StateFailed)
-}
diff --git a/models/token.go b/models/token.go
index a4caf7a5..7ae14f00 100644
--- a/models/token.go
+++ b/models/token.go
@@ -18,22 +18,19 @@ type tokenStore interface {
 
 // Redeem redeems this Token, and updates its entry in the associated domain and token
 // database stores. Returns the domain name that this token was generated for.
-func (t *Token) Redeem(store domainStore, tokens tokenStore) (ret string, userErr error, dbErr error) {
+func (t *Token) Redeem(pending policyStore, store policyStore, tokens tokenStore) (ret string, userErr error, dbErr error) {
 	domain, err := tokens.UseToken(t.Token)
 	if err != nil {
 		return domain, err, nil
 	}
-	domainData, err := store.GetDomain(domain, StateUnconfirmed)
+	domainData, err := pending.GetPolicy(domain)
 	if err != nil {
 		return domain, nil, err
 	}
-	domainOnList, err := GetDomain(store, domainData.Name)
+	err = store.PutOrUpdatePolicy(&domainData)
 	if err != nil {
 		return domain, nil, err
 	}
-	if domainOnList.State != StateUnconfirmed {
-		store.RemoveDomain(domainData.Name, domainOnList.State)
-	}
-	err = store.SetStatus(domainData.Name, StateTesting)
+	_, err = pending.RemovePolicy(domain)
 	return domain, nil, err
 }

From 61f2c69ff1a8c8af9a8b284884b8929974b7eafe Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Thu, 20 Jun 2019 15:53:11 -0700
Subject: [PATCH 04/12] Test new model functions

---
 models/policy.go      |  23 +++-
 models/policy_test.go | 275 ++++++++++++++++++++++--------------------
 models/token_test.go  |  51 ++++----
 policy/policy.go      |  24 ++++
 4 files changed, 213 insertions(+), 160 deletions(-)

diff --git a/models/policy.go b/models/policy.go
index b9874586..640ae8d6 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -28,18 +28,32 @@ type policyList interface {
 	HasDomain(string) bool
 }
 
+// TODO: test
+func (p *PolicySubmission) SamePolicy(other PolicySubmission) bool {
+	shallowEqual := p.Name == other.Name && p.MTASTS == other.MTASTS
+	if p.Policy == nil {
+		return shallowEqual && other.Policy == nil
+	}
+	return shallowEqual && p.Policy.Equals(other.Policy)
+}
+
+// TODO: test
+func (p *PolicySubmission) Valid() bool {
+	return p.MTASTS || (p.Policy != nil && len(p.Policy.MXs) > 0)
+}
+
 // CanUpdate returns whether we can update the policyStore with this particular
 // Policy Submission. In some cases, you should be able to update the
 // existing policy. In other cases, you shouldn't.
-// TODO: test
 func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
 	oldPolicy, err := policies.GetPolicy(p.Name)
 	// If this policy doesn't exist in the policyStore, we can add it.
+	// TODO: not to conflate between real errors and "not present" errors.
 	if err != nil {
 		return true
 	}
 	// If the policies are the same, return true if emails are different.
-	if (oldPolicy.MTASTS && p.MTASTS) || oldPolicy.Policy.HostnamesEqual(p.Policy) {
+	if p.SamePolicy(oldPolicy) {
 		return oldPolicy.Email != p.Email
 	}
 	// If old policy is manual and in testing, we can update it safely.
@@ -95,17 +109,18 @@ func (d *PolicySubmission) InitializeWithToken(pending policyStore, tokens token
 }
 
 // PolicyListCheck checks the policy list status of this particular domain.
+// TODO: differentiate between errors and not-in-DB
 func (d *PolicySubmission) PolicyListCheck(pending policyStore, store policyStore, list policyList) *checker.Result {
 	result := checker.Result{Name: checker.PolicyList}
 	if list.HasDomain(d.Name) {
 		return result.Success()
 	}
 	_, err := store.GetPolicy(d.Name)
-	if err != nil {
+	if err == nil {
 		return result.Warning("Domain %s should soon be added to the policy list.", d.Name)
 	}
 	_, err = pending.GetPolicy(d.Name)
-	if err != nil {
+	if err == nil {
 		return result.Failure("The policy submission for %s is waiting on email validation.", d.Name)
 	}
 	return result.Failure("Domain %s is not on the policy list.", d.Name)
diff --git a/models/policy_test.go b/models/policy_test.go
index 6c8b43e4..d67202b4 100644
--- a/models/policy_test.go
+++ b/models/policy_test.go
@@ -2,46 +2,35 @@ package models
 
 import (
 	"errors"
-	"strings"
-	"testing"
+	// "strings"
+	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
+	"github.com/EFForg/starttls-backend/policy"
+	"testing"
 )
 
-type mockDomainStore struct {
-	domain  PolicySubmission
-	domains []PolicySubmission
-	err     error
-}
-
-func (m *mockDomainStore) PutDomain(d PolicySubmission) error {
-	m.domain = d
-	return m.err
+type mockPolicyStore struct {
+	policy   PolicySubmission
+	policies []PolicySubmission
+	err      error
 }
 
-func (m *mockDomainStore) SetStatus(d string, status DomainState) error {
-	m.domain.State = status
+func (m *mockPolicyStore) PutOrUpdatePolicy(p *PolicySubmission) error {
+	m.policy = *p
 	return m.err
 }
 
-func (m *mockDomainStore) GetDomain(d string, state DomainState) (PolicySubmission, error) {
-	domain := m.domain
-	if state != domain.State {
-		return m.domain, errors.New("")
-	}
-	return m.domain, nil
+func (m *mockPolicyStore) GetPolicy(domain string) (PolicySubmission, error) {
+	return m.policy, m.err
 }
 
-func (m *mockDomainStore) GetDomains(_ DomainState) ([]PolicySubmission, error) {
-	return m.domains, m.err
+func (m *mockPolicyStore) GetPolicies(_ bool) ([]PolicySubmission, error) {
+	return m.policies, m.err
 }
 
-func (m *mockDomainStore) RemoveDomain(d string, state DomainState) (PolicySubmission, error) {
-	domain := m.domain
-	if state != domain.State {
-		return m.domain, errors.New("")
-	}
-	return m.domain, nil
+func (m *mockPolicyStore) RemovePolicy(_ string) (PolicySubmission, error) {
+	return m.policy, m.err
 }
 
 type mockList struct {
@@ -57,134 +46,159 @@ type mockScanStore struct {
 
 func (m mockScanStore) GetLatestScan(string) (Scan, error) { return m.scan, m.err }
 
-func TestIsQueueable(t *testing.T) {
-	// With supplied hostnames
-	d := PolicySubmission{
+// Some helper functions to make constructing dummy objects easier
+
+func (p PolicySubmission) withMode(mode string) PolicySubmission {
+	p.Policy.Mode = mode
+	return p
+}
+
+func (p PolicySubmission) withMXs(mxs []string) PolicySubmission {
+	p.Policy.MXs = mxs
+	return p
+}
+
+func (p PolicySubmission) withMTASTS() PolicySubmission {
+	p.MTASTS = true
+	return p
+}
+
+func (p PolicySubmission) withEmail(email string) PolicySubmission {
+	p.Email = email
+	return p
+}
+
+func TestCanUpdate(t *testing.T) {
+	var newP = func() PolicySubmission {
+		return PolicySubmission{Policy: &policy.TLSPolicy{}}
+	}
+	var testCases = []struct {
+		desc      string
+		oldPolicy PolicySubmission // Return from GetPolicy
+		policy    PolicySubmission // Policy to try to insert
+		err       error            // Does GetPolicy return an error?
+		expected  bool
+	}{
+		{"policy not found", newP(), newP(), errors.New("policy not found in DB"), true},
+		{"no update if policies same 1", newP().withMode("testing"), newP().withMode("testing"), nil, false},
+		{"no update if policies same 2", newP().withMode("enforce"), newP().withMode("enforce"), nil, false},
+		{"no update if policies same 3",
+			newP().withMode("enforce").withMTASTS().withMXs([]string{"a", "b", "c"}),
+			newP().withMode("enforce").withMTASTS().withMXs([]string{"a", "c", "b"}),
+			nil, false},
+		{"can upgrade to manual enforce", newP().withMode("testing"), newP().withMode("enforce"), nil, true},
+		{"can't downgrade to enforce", newP().withMode("enforce"), newP().withMode("testing"), nil, false},
+		{"prevent upgrade to enforce with MTA-STS",
+			newP().withMTASTS().withMode("testing"),
+			newP().withMode("enforce"),
+			nil, false},
+		{"no mx changes with MTA-STS, even in testing",
+			newP().withMTASTS().withMode("testing").withMXs([]string{"a", "b"}),
+			newP().withMTASTS().withMode("testing").withMXs([]string{"a", "b", "c"}),
+			nil, false},
+		{"mx can change in testing",
+			newP().withMode("testing").withMXs([]string{"a", "b"}),
+			newP().withMode("testing").withMXs([]string{"a", "b", "c"}),
+			nil, true},
+		{"update email", newP().withEmail("abc").withMode("enforce"), newP().withEmail("a").withMode("enforce"),
+			nil, true},
+	}
+	for _, tc := range testCases {
+		store := mockPolicyStore{policy: tc.oldPolicy, err: tc.err}
+		got := (&tc.policy).CanUpdate(&store)
+		if got != tc.expected {
+			t.Errorf("%s: expected %t but got %t",
+				tc.desc, tc.expected, got)
+		}
+	}
+}
+
+func TestValidScan(t *testing.T) {
+	var newP = PolicySubmission{
 		Name:  "example.com",
 		Email: "me@example.com",
-		MXs:   []string{".example.com"},
-	}
+		Policy: &policy.TLSPolicy{
+			Mode: "testing",
+			MXs:  []string{".example.com"}}}
+
 	goodScan := Scan{
 		Data: checker.DomainResult{
 			PreferredHostnames: []string{"mx1.example.com", "mx2.example.com"},
 			MTASTSResult:       checker.MakeMTASTSResult(),
 		},
+		Timestamp: time.Now()}
+	var withBadMTASTS = func(scan Scan) Scan {
+		scan.Data.MTASTSResult = checker.MakeMTASTSResult()
+		scan.Data.MTASTSResult.Status = checker.Failure
+		return scan
+	}
+	var withTimestamp = func(scan Scan, time time.Time) Scan {
+		scan.Timestamp = time
+		return scan
 	}
 	failedScan := Scan{
 		Data: checker.DomainResult{Status: checker.DomainFailure},
 	}
-	wrongMXsScan := Scan{
-		Data: checker.DomainResult{
-			PreferredHostnames: []string{"mx1.nomatch.example.com"},
-		},
-	}
 	var testCases = []struct {
-		name    string
-		scan    Scan
-		scanErr error
-		state   DomainState
-		onList  bool
-		ok      bool
-		msg     string
+		desc     string
+		mxs      []string
+		mtasts   bool
+		scan     Scan
+		err      error
+		expected bool
 	}{
-		{name: "Unadded domain with passing scan should be queueable",
-			scan: goodScan, scanErr: nil, onList: false,
-			ok: true, msg: ""},
-		{name: "Domain on policy list should not be queueable",
-			scan: goodScan, scanErr: nil, onList: true,
-			ok: false, msg: "already on the policy list"},
-		{name: "Enforced domain should not be queueable",
-			scan: goodScan, scanErr: nil, onList: false, state: StateEnforce,
-			ok: false, msg: "already on the policy list"},
-		{name: "Domain with failing scan should not be queueable",
-			scan: failedScan, scanErr: nil, onList: false,
-			ok: false, msg: "hasn't passed"},
-		{name: "Domain without scan should not be queueable",
-			scan: goodScan, scanErr: errors.New(""), onList: false,
-			ok: false, msg: "haven't scanned"},
-		{name: "Domain with mismatched hostnames should not be queueable",
-			scan: wrongMXsScan, scanErr: nil, onList: false,
-			ok: false, msg: "do not match policy"},
+		{desc: "Unadded domain with recent passing scan should be queueable",
+			mxs: []string{".example.com"}, scan: goodScan, err: nil, expected: true},
+		{desc: "Unadded domain with old passing scan shouldn't be queueable",
+			mxs: []string{".example.com"}, scan: withTimestamp(goodScan, time.Now().Add(time.Duration(-1)*time.Hour)),
+			err: nil, expected: false},
+		{desc: "Domain with passing scan but mismatched hostnames shouldn't be queueable",
+			mxs: []string{"mx1.example.com"}, scan: goodScan, err: nil, expected: false},
+		{desc: "Domain with failing scan shouldn't be queueable", scan: failedScan, err: nil, expected: false},
+		{desc: "Domain without scan shouldn't be queueable", err: errors.New(""), expected: false},
+		{desc: "Domain with MTA-STS should be queueable",
+			mxs: []string{".example.com"}, scan: goodScan, mtasts: true, expected: true},
+		{desc: "Domain with MTA-STS but MTA-STS scan failed shouldn't be queueable",
+			mxs: []string{".example.com"}, scan: withBadMTASTS(goodScan), mtasts: true, expected: false},
 	}
 	for _, tc := range testCases {
-		domainStore := mockDomainStore{domain: PolicySubmission{State: tc.state}}
-		ok, msg, _ := d.IsQueueable(&domainStore, mockScanStore{tc.scan, tc.scanErr}, mockList{tc.onList})
-		if ok != tc.ok {
-			t.Error(tc.name)
-		}
-		if !strings.Contains(msg, tc.msg) {
-			t.Errorf("IsQueueable message should contain %s, got %s", tc.msg, msg)
-		}
-	}
-	// With MTA-STS
-	d = PolicySubmission{
-		Name:   "example.com",
-		Email:  "me@example.com",
-		MTASTS: true,
-	}
-	domainStore := mockDomainStore{err: errors.New("")}
-	ok, msg, _ := d.IsQueueable(&domainStore, mockScanStore{goodScan, nil}, mockList{false})
-	if !ok {
-		t.Error("Unadded domain with passing scan should be queueable, got " + msg)
-	}
-	noMTASTSScan := Scan{
-		Data: checker.DomainResult{
-			MTASTSResult: &checker.MTASTSResult{
-				Result: &checker.Result{
-					Status: checker.Failure,
-				},
-			},
-		},
-	}
-	ok, msg, _ = d.IsQueueable(&domainStore, mockScanStore{noMTASTSScan, nil}, mockList{false})
-	if ok || !strings.Contains(msg, "MTA-STS") {
-		t.Error("Domain without MTA-STS or hostnames should not be queueable, got " + msg)
-	}
-}
-
-func TestPopulateFromScan(t *testing.T) {
-	d := PolicySubmission{
-		Name:   "example.com",
-		Email:  "me@example.com",
-		MTASTS: true,
-	}
-	s := Scan{
-		Data: checker.DomainResult{
-			MTASTSResult: checker.MakeMTASTSResult(),
-		},
-	}
-	s.Data.MTASTSResult.MXs = []string{"mx1.example.com", "mx2.example.com"}
-	d.PopulateFromScan(s)
-	for i, mx := range s.Data.MTASTSResult.MXs {
-		if mx != d.MXs[i] {
-			t.Errorf("Expected MXs to match scan, got %s", d.MXs)
+		store := mockScanStore{tc.scan, tc.err}
+		policy := newP.withMXs(tc.mxs)
+		policy.MTASTS = tc.mtasts
+		got, msg := (&policy).HasValidScan(store)
+		if got != tc.expected {
+			t.Errorf("%s: expected %t but got %t: %s", tc.desc, tc.expected, got, msg)
 		}
 	}
 }
 
 func TestPolicyCheck(t *testing.T) {
 	var testCases = []struct {
-		name     string
-		onList   bool
-		state    DomainState
-		inDB     bool
-		expected checker.Status
+		desc        string
+		onList      bool
+		inDB        bool
+		inPendingDB bool
+		expected    checker.Status
 	}{
-		{"Domain on the list should return success", true, StateEnforce, false, checker.Success},
-		{"Domain in DB as enforce should return success", false, StateEnforce, true, checker.Success},
-		{"Domain queued should return a warning", false, StateTesting, true, checker.Warning},
-		{"Unconfirmed domain should return a failure", false, StateUnconfirmed, true, checker.Failure},
-		{"Domain not currently in the DB or on the list should return a failure", false, StateUnconfirmed, false, checker.Failure},
+		{desc: "Domain on the list should return success", onList: true, expected: checker.Success},
+		{desc: "Domain not on list but in policies DB should return warning", inDB: true, expected: checker.Warning},
+		{desc: "Domain in pending policies DB should return failure", inPendingDB: true, expected: checker.Failure},
+		{desc: "Domain not anywhere should return failure", expected: checker.Failure},
 	}
 	for _, tc := range testCases {
-		domainObj := PolicySubmission{Name: "example.com", State: tc.state}
+		policy := &PolicySubmission{Policy: &policy.TLSPolicy{}}
 		var dbErr error
 		if !tc.inDB {
-			dbErr = errors.New("")
+			dbErr = errors.New("DB err")
+		}
+		var pendingDBErr error
+		if !tc.inPendingDB {
+			pendingDBErr = errors.New("pending err")
 		}
-		result := domainObj.PolicyListCheck(&mockDomainStore{domain: domainObj, err: dbErr}, mockList{tc.onList})
+		result := policy.PolicyListCheck(
+			&mockPolicyStore{err: pendingDBErr}, &mockPolicyStore{err: dbErr}, mockList{tc.onList})
 		if result.Status != tc.expected {
-			t.Error(tc.name)
+			t.Errorf("%s: expected status %d, got result %v", tc.desc, tc.expected, result)
 		}
 	}
 }
@@ -192,19 +206,18 @@ func TestPolicyCheck(t *testing.T) {
 func TestInitializeWithToken(t *testing.T) {
 	mockToken := mockTokenStore{domain: "domain", err: nil}
 	domainObj := PolicySubmission{Name: "example.com"}
-	// domainStore returns error
-	_, err := domainObj.InitializeWithToken(&mockDomainStore{domain: domainObj, err: errors.New("")}, &mockToken)
+	_, err := domainObj.InitializeWithToken(&mockPolicyStore{err: errors.New("")}, &mockToken)
 	if err == nil {
 		t.Error("Expected InitializeWithToken to forward error message from DB")
 	}
 	if mockToken.token != nil {
 		t.Error("Token should not have been set if domain not found")
 	}
-	_, err = domainObj.InitializeWithToken(&mockDomainStore{domain: domainObj}, &mockTokenStore{err: errors.New("")})
+	_, err = domainObj.InitializeWithToken(&mockPolicyStore{policy: domainObj}, &mockTokenStore{err: errors.New("")})
 	if err == nil {
 		t.Error("Expected InitializeWithToken to forward error message from DB")
 	}
-	domainObj.InitializeWithToken(&mockDomainStore{domain: domainObj, err: nil}, &mockToken)
+	domainObj.InitializeWithToken(&mockPolicyStore{policy: domainObj, err: nil}, &mockToken)
 	if mockToken.token == nil {
 		t.Error("Token should have been set for domain")
 	}
diff --git a/models/token_test.go b/models/token_test.go
index b6e93671..9e42cd69 100644
--- a/models/token_test.go
+++ b/models/token_test.go
@@ -1,8 +1,8 @@
 package models
 
 import (
-	"errors"
-	"testing"
+// 	"errors"
+// 	"testing"
 )
 
 type mockTokenStore struct {
@@ -20,26 +20,27 @@ func (m *mockTokenStore) UseToken(token string) (string, error) {
 	return m.domain, m.err
 }
 
-func TestRedeemToken(t *testing.T) {
-	domains := mockDomainStore{domain: PolicySubmission{Name: "anything", State: StateUnconfirmed}, err: nil}
-	token := Token{Token: "token"}
-	domain, userErr, dbErr := token.Redeem(&domains, &mockTokenStore{domain: "anything", err: nil})
-	if domain != "anything" || userErr != nil || dbErr != nil {
-		t.Error("Expected token redeem to succeed")
-	}
-	if domains.domain.State != StateTesting {
-		t.Error("Expected PutDomain to have upgraded domain State")
-	}
-}
-
-func TestRedeemTokenFailures(t *testing.T) {
-	token := Token{Token: "token"}
-	_, userErr, _ := token.Redeem(&mockDomainStore{err: nil}, &mockTokenStore{err: errors.New("")})
-	if userErr == nil {
-		t.Error("Errors reported from the token store should be interpreted as usage error (token already used, or doesn't exist)")
-	}
-	_, _, dbErr := token.Redeem(&mockDomainStore{err: errors.New("")}, &mockTokenStore{err: nil})
-	if dbErr == nil {
-		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
-	}
-}
+// TODO: fix
+// func TestRedeemToken(t *testing.T) {
+// 	domains := mockDomainStore{domain: PolicySubmission{Name: "anything", State: StateUnconfirmed}, err: nil}
+// 	token := Token{Token: "token"}
+// 	domain, userErr, dbErr := token.Redeem(&domains, &mockTokenStore{domain: "anything", err: nil})
+// 	if domain != "anything" || userErr != nil || dbErr != nil {
+// 		t.Error("Expected token redeem to succeed")
+// 	}
+// 	if domains.domain.State != StateTesting {
+// 		t.Error("Expected PutDomain to have upgraded domain State")
+// 	}
+// }
+//
+// func TestRedeemTokenFailures(t *testing.T) {
+// 	token := Token{Token: "token"}
+// 	_, userErr, _ := token.Redeem(&mockDomainStore{err: nil}, &mockTokenStore{err: errors.New("")})
+// 	if userErr == nil {
+// 		t.Error("Errors reported from the token store should be interpreted as usage error (token already used, or doesn't exist)")
+// 	}
+// 	_, _, dbErr := token.Redeem(&mockDomainStore{err: errors.New("")}, &mockTokenStore{err: nil})
+// 	if dbErr == nil {
+// 		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
+// 	}
+// }
diff --git a/policy/policy.go b/policy/policy.go
index 32758748..e5a9fd5c 100644
--- a/policy/policy.go
+++ b/policy/policy.go
@@ -30,6 +30,30 @@ type List struct {
 	Policies      map[string]TLSPolicy `json:"policies"`
 }
 
+func (p *TLSPolicy) Equals(other *TLSPolicy) bool {
+	if other == nil {
+		return false
+	}
+	return p.Mode == other.Mode && p.hostnamesEqual(other)
+}
+
+// Assumption: Every string is unique in the MXs list.
+func (p *TLSPolicy) hostnamesEqual(other *TLSPolicy) bool {
+	if len(p.MXs) != len(other.MXs) {
+		return false
+	}
+	set := make(map[string]bool)
+	for _, mx := range p.MXs {
+		set[mx] = true
+	}
+	for _, mx := range other.MXs {
+		if !set[mx] {
+			return false
+		}
+	}
+	return true
+}
+
 // Add adds a particular domain's policy to the list.
 func (l *List) Add(domain string, policy TLSPolicy) {
 	l.Policies[domain] = policy

From 846da9c5a6af5b147c2a78099025c64b15a8589a Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Thu, 20 Jun 2019 11:50:52 -0700
Subject: [PATCH 05/12] Plumbing and test migrations for database access to new
 domain model

---
 db/README.md               |   1 +
 db/db.go                   |   8 --
 db/policy.go               | 121 +++++++++++++++----------------
 db/scripts/init_tables.sql |   4 +-
 db/sqldb.go                |  13 +++-
 db/sqldb_test.go           | 145 +++++++++++--------------------------
 6 files changed, 114 insertions(+), 178 deletions(-)
 create mode 100644 db/README.md

diff --git a/db/README.md b/db/README.md
new file mode 100644
index 00000000..9af97e40
--- /dev/null
+++ b/db/README.md
@@ -0,0 +1 @@
+# Database structure
diff --git a/db/db.go b/db/db.go
index cfdbbc04..befb87bd 100644
--- a/db/db.go
+++ b/db/db.go
@@ -39,14 +39,6 @@ type Database interface {
 	PutLocalStats(time.Time) (checker.AggregatedScan, error)
 	// Gets counts per day of hosts supporting MTA-STS for a given source.
 	GetStats(string) (stats.Series, error)
-	// Upserts domain state.
-	PutDomain(models.PolicySubmission) error
-	// Retrieves state of a domain
-	GetDomain(string, models.DomainState) (models.PolicySubmission, error)
-	// Retrieves all domains in a particular state.
-	GetDomains(models.DomainState) ([]models.PolicySubmission, error)
-	SetStatus(string, models.DomainState) error
-	RemoveDomain(string, models.DomainState) (models.PolicySubmission, error)
 	ClearTables() error
 }
 
diff --git a/db/policy.go b/db/policy.go
index 7cf18930..0efc824f 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -1,99 +1,92 @@
 package db
 
 import (
+	"database/sql"
 	"fmt"
 	"strings"
-	"time"
 
 	"github.com/EFForg/starttls-backend/models"
+	"github.com/EFForg/starttls-backend/policy"
 )
 
-func (db SQLDatabase) queryDomain(sqlQuery string, args ...interface{}) (models.PolicySubmission, error) {
-	query := fmt.Sprintf(sqlQuery, "domain, email, data, status, last_updated, queue_weeks")
-	data := models.PolicySubmission{}
+type PolicyDB struct {
+	tableName string
+	conn      *sql.DB
+	locked    bool
+}
+
+func (p *PolicyDB) formQuery(query string) string {
+	return fmt.Sprintf(query, p.tableName, "domain, email, mta_sts, mxs, mode")
+}
+
+type scanner interface {
+	Scan(dest ...interface{}) error
+}
+
+func (p *PolicyDB) scanPolicy(result scanner) (models.PolicySubmission, error) {
+	data := models.PolicySubmission{Policy: new(policy.TLSPolicy)}
 	var rawMXs string
-	err := db.conn.QueryRow(query, args...).Scan(
-		&data.Name, &data.Email, &rawMXs, &data.State, &data.LastUpdated, &data.QueueWeeks)
-	data.MXs = strings.Split(rawMXs, ",")
-	if len(rawMXs) == 0 {
-		data.MXs = []string{}
-	}
+	err := result.Scan(
+		&data.Name, &data.Email,
+		&data.MTASTS, &rawMXs, &data.Policy.Mode)
+	data.Policy.MXs = strings.Split(rawMXs, ",")
 	return data, err
 }
 
-func (db SQLDatabase) queryDomainsWhere(condition string, args ...interface{}) ([]models.PolicySubmission, error) {
-	query := fmt.Sprintf("SELECT domain, email, data, status, last_updated, queue_weeks FROM domains WHERE %s", condition)
-	rows, err := db.conn.Query(query, args...)
+func (p *PolicyDB) GetPolicies(mtasts bool) ([]models.PolicySubmission, error) {
+	rows, err := p.conn.Query(p.formQuery(
+		"SELECT %[2]s FROM %[1]s WHERE mta_sts=$1"), mtasts)
 	if err != nil {
 		return nil, err
 	}
 	defer rows.Close()
-	domains := []models.PolicySubmission{}
+	policies := []models.PolicySubmission{}
 	for rows.Next() {
-		var domain models.PolicySubmission
-		var rawMXs string
-		if err := rows.Scan(&domain.Name, &domain.Email, &rawMXs, &domain.State, &domain.LastUpdated, &domain.QueueWeeks); err != nil {
+		policy, err := p.scanPolicy(rows)
+		if err != nil {
 			return nil, err
 		}
-		domain.MXs = strings.Split(rawMXs, ",")
-		domains = append(domains, domain)
+		policies = append(policies, policy)
 	}
-	return domains, nil
-}
-
-// =============== models.DomainStore impl ===============
-
-// PutDomain inserts a particular domain into the database. If the domain does
-// not yet exist in the database, we initialize it with StateUnconfirmed
-// If there is already a domain in the database with StateUnconfirmed, performs
-// an update of the fields.
-func (db *SQLDatabase) PutDomain(domain models.PolicySubmission) error {
-	_, err := db.conn.Exec("INSERT INTO domains(domain, email, data, status, queue_weeks, mta_sts) "+
-		"VALUES($1, $2, $3, $4, $5, $6) "+
-		"ON CONFLICT ON CONSTRAINT domains_pkey DO UPDATE SET email=$2, data=$3, queue_weeks=$5",
-		domain.Name, domain.Email, strings.Join(domain.MXs[:], ","),
-		models.StateUnconfirmed, domain.QueueWeeks, domain.MTASTS)
-	return err
-}
-
-// GetDomain retrieves the status and information associated with a particular
-// mailserver domain.
-func (db SQLDatabase) GetDomain(domain string, state models.DomainState) (models.PolicySubmission, error) {
-	return db.queryDomain("SELECT %s FROM domains WHERE domain=$1 AND status=$2", domain, state)
+	return policies, nil
 }
 
-// GetDomains retrieves all the domains which match a particular state,
-// that are not in MTA_STS mode
-func (db SQLDatabase) GetDomains(state models.DomainState) ([]models.PolicySubmission, error) {
-	return db.queryDomainsWhere("status=$1", state)
+func (p *PolicyDB) GetPolicy(domainName string) (models.PolicySubmission, error) {
+	row := p.conn.QueryRow(p.formQuery(
+		"SELECT %[2]s FROM %[1]s WHERE domain=$1"), domainName)
+	return p.scanPolicy(row)
 }
 
-// GetMTASTSDomains retrieves domains which wish their policy to be queued with their MTASTS.
-func (db SQLDatabase) GetMTASTSDomains() ([]models.PolicySubmission, error) {
-	return db.queryDomainsWhere("mta_sts=TRUE")
+func (p *PolicyDB) RemovePolicy(domainName string) (models.PolicySubmission, error) {
+	row := p.conn.QueryRow(p.formQuery(
+		"DELETE FROM %[1]s WHERE domain=$1 RETURNING %[2]s"), domainName)
+	return p.scanPolicy(row)
 }
 
-// SetStatus sets the status of a particular domain object to |state|.
-func (db SQLDatabase) SetStatus(domain string, state models.DomainState) error {
-	var testingStart time.Time
-	if state == models.StateTesting {
-		testingStart = time.Now()
+func (p *PolicyDB) PutOrUpdatePolicy(ps *models.PolicySubmission) error {
+	if p.locked && !ps.CanUpdate(p) {
+		return fmt.Errorf("can't update policy in restricted table")
 	}
-	_, err := db.conn.Exec("UPDATE domains SET status = $1, testing_start = $2 WHERE domain=$3",
-		state, testingStart, domain)
+	if p.locked && ps.Policy == nil {
+		return fmt.Errorf("can't degrade policy in restricted table")
+	}
+	if ps.Policy == nil {
+		ps.Policy = &policy.TLSPolicy{MXs: []string{}, Mode: ""}
+	}
+	_, err := p.conn.Exec(p.formQuery(
+		"INSERT INTO %[1]s(%[2]s) VALUES($1, $2, $3, $4, $5) "+
+			"ON CONFLICT (domain) DO UPDATE SET "+
+			"email=$2, mta_sts=$3, mxs=$4, mode=$5"),
+		ps.Name, ps.Email, ps.MTASTS,
+		strings.Join(ps.Policy.MXs[:], ","), ps.Policy.Mode)
 	return err
 }
 
-// RemoveDomain removes a particular domain and returns it.
-func (db SQLDatabase) RemoveDomain(domain string, state models.DomainState) (models.PolicySubmission, error) {
-	return db.queryDomain("DELETE FROM domains WHERE domain=$1 AND status=$2 RETURNING %s")
-}
-
 // DomainsToValidate [interface Validator] retrieves domains from the
 // DB whose policies should be validated.
 func (db SQLDatabase) DomainsToValidate() ([]string, error) {
 	domains := []string{}
-	data, err := db.GetDomains(models.StateTesting)
+	data, err := db.PendingPolicies.GetPolicies(false)
 	if err != nil {
 		return domains, err
 	}
@@ -106,12 +99,12 @@ func (db SQLDatabase) DomainsToValidate() ([]string, error) {
 // HostnamesForDomain [interface Validator] retrieves the hostname policy for
 // a particular domain.
 func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) {
-	data, err := db.GetDomain(domain, models.StateEnforce)
+	data, err := db.Policies.GetPolicy(domain)
 	if err != nil {
-		data, err = db.GetDomain(domain, models.StateTesting)
+		data, err = db.PendingPolicies.GetPolicy(domain)
 	}
 	if err != nil {
 		return []string{}, err
 	}
-	return data.MXs, nil
+	return data.Policy.MXs, nil
 }
diff --git a/db/scripts/init_tables.sql b/db/scripts/init_tables.sql
index ac3425cb..a7b54e71 100644
--- a/db/scripts/init_tables.sql
+++ b/db/scripts/init_tables.sql
@@ -54,7 +54,7 @@ CREATE TABLE IF NOT EXISTS pending_policies
     email         TEXT NOT NULL,
     mta_sts       BOOLEAN DEFAULT FALSE,
     mxs           TEXT NOT NULL,
-    mode          VARCHAR(255) NOT NULL,
+    mode          VARCHAR(255) NOT NULL
 );
 
 
@@ -64,7 +64,7 @@ CREATE TABLE IF NOT EXISTS policies
     email         TEXT NOT NULL,
     mta_sts       BOOLEAN DEFAULT FALSE,
     mxs           TEXT NOT NULL,
-    mode          VARCHAR(255) NOT NULL,
+    mode          VARCHAR(255) NOT NULL
 );
 
 -- Schema change: add "last_updated" timestamp column if it doesn't exist.
diff --git a/db/sqldb.go b/db/sqldb.go
index b22f1492..6ecea8f2 100644
--- a/db/sqldb.go
+++ b/db/sqldb.go
@@ -22,8 +22,10 @@ const sqlTimeFormat = "2006-01-02 15:04:05"
 
 // SQLDatabase is a Database interface backed by postgresql.
 type SQLDatabase struct {
-	cfg  Config  // Configuration to define the DB connection.
-	conn *sql.DB // The database connection.
+	cfg             Config  // Configuration to define the DB connection.
+	conn            *sql.DB // The database connection.
+	PendingPolicies *PolicyDB
+	Policies        *PolicyDB
 }
 
 func getConnectionString(cfg Config) string {
@@ -45,7 +47,10 @@ func InitSQLDatabase(cfg Config) (*SQLDatabase, error) {
 	if err != nil {
 		return nil, err
 	}
-	return &SQLDatabase{cfg: cfg, conn: conn}, nil
+	return &SQLDatabase{cfg: cfg, conn: conn,
+		PendingPolicies: &PolicyDB{tableName: "pending_policies", conn: conn, locked: false},
+		Policies:        &PolicyDB{tableName: "policies", conn: conn, locked: false},
+	}, nil
 }
 
 // TOKEN DB FUNCTIONS
@@ -251,6 +256,8 @@ func (db SQLDatabase) ClearTables() error {
 		fmt.Sprintf("DELETE FROM %s", "hostname_scans"),
 		fmt.Sprintf("DELETE FROM %s", "blacklisted_emails"),
 		fmt.Sprintf("DELETE FROM %s", "aggregated_scans"),
+		fmt.Sprintf("DELETE FROM %s", "policies"),
+		fmt.Sprintf("DELETE FROM %s", "pending_policies"),
 		fmt.Sprintf("ALTER SEQUENCE %s_id_seq RESTART WITH 1", db.cfg.DbScanTable),
 	})
 }
diff --git a/db/sqldb_test.go b/db/sqldb_test.go
index 4c23086c..88b4eaeb 100644
--- a/db/sqldb_test.go
+++ b/db/sqldb_test.go
@@ -3,13 +3,13 @@ package db_test
 import (
 	"log"
 	"os"
-	"strings"
 	"testing"
 	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
 	"github.com/EFForg/starttls-backend/db"
 	"github.com/EFForg/starttls-backend/models"
+	"github.com/EFForg/starttls-backend/policy"
 	"github.com/joho/godotenv"
 )
 
@@ -143,44 +143,41 @@ func TestPutGetDomain(t *testing.T) {
 		Name:  "testing.com",
 		Email: "admin@testing.com",
 	}
-	err := database.PutDomain(data)
+	err := database.Policies.PutOrUpdatePolicy(&data)
 	if err != nil {
 		t.Errorf("PutDomain failed: %v\n", err)
 	}
-	retrievedData, err := database.GetDomain(data.Name, models.StateUnconfirmed)
+	retrievedData, err := database.Policies.GetPolicy(data.Name)
 	if err != nil {
 		t.Errorf("GetDomain(%s) failed: %v\n", data.Name, err)
 	}
 	if retrievedData.Name != data.Name {
 		t.Errorf("Somehow, GetDomain retrieved the wrong object?")
 	}
-	if retrievedData.State != models.StateUnconfirmed {
-		t.Errorf("Default state should be 'Unconfirmed'")
-	}
 }
 
 func TestUpsertDomain(t *testing.T) {
 	database.ClearTables()
-	data := models.PolicySubmission{
-		Name:  "testing.com",
-		MXs:   []string{"hello1"},
-		Email: "admin@testing.com",
+	var getPolicy = func(email string, mx string) *models.PolicySubmission {
+		return &models.PolicySubmission{
+			Name: "testing.com",
+			Policy: &policy.TLSPolicy{
+				MXs: []string{mx},
+			},
+			Email: email,
+		}
 	}
-	database.PutDomain(data)
-	err := database.PutDomain(models.PolicySubmission{Name: "testing.com", MXs: []string{"hello_darkness_my_old_friend"}, Email: "actual_admin@testing.com"})
+	database.Policies.PutOrUpdatePolicy(getPolicy("admin@testing.com", "hello1"))
+	err := database.Policies.PutOrUpdatePolicy(getPolicy("actual_admin@testing.com", "hello_darkness_my_old_friend"))
 	if err != nil {
-		t.Errorf("PutDomain(%s) failed: %v\n", data.Name, err)
+		t.Errorf("PutDomain(%s) failed: %v\n", "testing.com", err)
 	}
-	retrievedData, err := database.GetDomain(data.Name, models.StateUnconfirmed)
-	if retrievedData.MXs[0] != "hello_darkness_my_old_friend" || retrievedData.Email != "actual_admin@testing.com" {
+	retrievedData, err := database.Policies.GetPolicy("testing.com")
+	if retrievedData.Policy.MXs[0] != "hello_darkness_my_old_friend" || retrievedData.Email != "actual_admin@testing.com" {
 		t.Errorf("Email and MXs should have been rewritten: %v\n", retrievedData)
 	}
 }
 
-func TestDomainSetStatus(t *testing.T) {
-	// TODO
-}
-
 func TestPutUseToken(t *testing.T) {
 	database.ClearTables()
 	data, err := database.PutToken("testing.com")
@@ -212,51 +209,16 @@ func TestPutTokenTwice(t *testing.T) {
 	}
 }
 
-func TestLastUpdatedFieldUpdates(t *testing.T) {
-	database.ClearTables()
-	data := models.PolicySubmission{
-		Name:  "testing.com",
-		Email: "admin@testing.com",
-		State: models.StateUnconfirmed,
-	}
-	database.PutDomain(data)
-	retrievedData, _ := database.GetDomain(data.Name, models.StateUnconfirmed)
-	lastUpdated := retrievedData.LastUpdated
-	data.State = models.StateTesting
-	database.PutDomain(models.PolicySubmission{Name: data.Name, Email: "new fone who dis"})
-	retrievedData, _ = database.GetDomain(data.Name, models.StateUnconfirmed)
-	if lastUpdated.Equal(retrievedData.LastUpdated) {
-		t.Errorf("Expected last_updated to be updated on change: %v", lastUpdated)
-	}
-}
-
-func TestLastUpdatedFieldDoesntUpdate(t *testing.T) {
-	database.ClearTables()
-	data := models.PolicySubmission{
-		Name:  "testing.com",
-		Email: "admin@testing.com",
-		State: models.StateUnconfirmed,
-	}
-	database.PutDomain(data)
-	retrievedData, _ := database.GetDomain(data.Name, models.StateUnconfirmed)
-	lastUpdated := retrievedData.LastUpdated
-	database.PutDomain(data)
-	retrievedData, _ = database.GetDomain(data.Name, models.StateUnconfirmed)
-	if !lastUpdated.Equal(retrievedData.LastUpdated) {
-		t.Errorf("Expected last_updated to stay the same if no changes were made")
-	}
-}
-
 func TestDomainsToValidate(t *testing.T) {
 	database.ClearTables()
-	queuedMap := map[string]bool{
+	mtastsMap := map[string]bool{
 		"a": false, "b": true, "c": false, "d": true,
 	}
-	for domain, queued := range queuedMap {
-		if queued {
-			database.PutDomain(models.PolicySubmission{Name: domain, State: models.StateTesting})
+	for domain, mtasts := range mtastsMap {
+		if mtasts {
+			database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: domain, MTASTS: true})
 		} else {
-			database.PutDomain(models.PolicySubmission{Name: domain})
+			database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: domain})
 		}
 	}
 	result, err := database.DomainsToValidate()
@@ -264,33 +226,34 @@ func TestDomainsToValidate(t *testing.T) {
 		t.Fatalf("DomainsToValidate failed: %v\n", err)
 	}
 	for _, domain := range result {
-		if !queuedMap[domain] {
+		if !mtastsMap[domain] {
 			t.Errorf("Did not expect %s to be returned", domain)
 		}
 	}
 }
 
-func TestHostnamesForDomain(t *testing.T) {
-	database.ClearTables()
-	database.PutDomain(models.PolicySubmission{Name: "x", MXs: []string{"x.com", "y.org"}})
-	database.PutDomain(models.PolicySubmission{Name: "y"})
-	database.SetStatus("x", models.StateTesting)
-	database.SetStatus("y", models.StateTesting)
-	result, err := database.HostnamesForDomain("x")
-	if err != nil {
-		t.Fatalf("HostnamesForDomain failed: %v\n", err)
-	}
-	if len(result) != 2 || result[0] != "x.com" || result[1] != "y.org" {
-		t.Errorf("Expected two hostnames, x.com and y.org\n")
-	}
-	result, err = database.HostnamesForDomain("y")
-	if err != nil {
-		t.Fatalf("HostnamesForDomain failed: %v\n", err)
-	}
-	if len(result) > 0 {
-		t.Errorf("Expected no hostnames to be returned, got %s\n", result[0])
-	}
-}
+// TODO: fix test
+// func TestHostnamesForDomain(t *testing.T) {
+// 	database.ClearTables()
+// 	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "x",
+// 		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{"x.com", "y.org"}}})
+// 	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "y",
+// 		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{}}})
+// 	result, err := database.HostnamesForDomain("x")
+// 	if err != nil {
+// 		t.Fatalf("HostnamesForDomain failed: %v\n", err)
+// 	}
+// 	if len(result) != 2 || result[0] != "x.com" || result[1] != "y.org" {
+// 		t.Errorf("Expected two hostnames, x.com and y.org\n")
+// 	}
+// 	result, err = database.HostnamesForDomain("y")
+// 	if err != nil {
+// 		t.Fatalf("HostnamesForDomain failed: %v\n", err)
+// 	}
+// 	if len(result) > 0 {
+// 		t.Errorf("Expected no hostnames to be returned, got %s\n", result[0])
+// 	}
+// }
 
 func TestPutAndIsBlacklistedEmail(t *testing.T) {
 	database.ClearTables()
@@ -474,23 +437,3 @@ func TestGetLocalStats(t *testing.T) {
 		}
 	}
 }
-
-func TestGetMTASTSDomains(t *testing.T) {
-	database.ClearTables()
-	database.PutDomain(models.PolicySubmission{Name: "unicorns"})
-	database.PutDomain(models.PolicySubmission{Name: "mta-sts-x", MTASTS: true})
-	database.PutDomain(models.PolicySubmission{Name: "mta-sts-y", MTASTS: true})
-	database.PutDomain(models.PolicySubmission{Name: "regular"})
-	domains, err := database.GetMTASTSDomains()
-	if err != nil {
-		t.Fatalf("GetMTASTSDomains() failed: %v", err)
-	}
-	if len(domains) != 2 {
-		t.Errorf("Expected GetMTASTSDomains() to return 2 elements")
-	}
-	for _, domain := range domains {
-		if !strings.HasPrefix(domain.Name, "mta-sts") {
-			t.Errorf("GetMTASTSDomains returned %s when it wasn't supposed to", domain.Name)
-		}
-	}
-}

From f8e1a14d6e033a7fde5567aaac54f93844bed540 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Thu, 20 Jun 2019 11:51:30 -0700
Subject: [PATCH 06/12] Replace Domain usage with Policy abstraction

---
 api.go   | 66 +++++++++++++++++++++-----------------------------------
 email.go |  8 +++----
 2 files changed, 28 insertions(+), 46 deletions(-)

diff --git a/api.go b/api.go
index 98ca0a2b..3f6cb5b4 100644
--- a/api.go
+++ b/api.go
@@ -41,7 +41,7 @@ type checkPerformer func(API, string) (checker.DomainResult, error)
 // Any POST request accepts either URL query parameters or data value parameters,
 // and prefers the latter if both are present.
 type API struct {
-	Database    db.Database
+	Database    *db.SQLDatabase
 	CheckDomain checkPerformer
 	List        PolicyList
 	DontScan    map[string]bool
@@ -90,7 +90,7 @@ func (api *API) wrapper(handler apiHandler) func(w http.ResponseWriter, r *http.
 }
 
 func defaultCheck(api API, domain string) (checker.DomainResult, error) {
-	policyChan := models.PolicySubmission{Name: domain}.AsyncPolicyListCheck(api.Database, api.List)
+	policyChan := models.PolicySubmission{Name: domain}.AsyncPolicyListCheck(api.Database.PendingPolicies, api.Database.Policies, api.List)
 	c := checker.Checker{
 		Cache: &checker.ScanCache{
 			ScanStore:  api.Database,
@@ -171,32 +171,26 @@ func (api API) Scan(r *http.Request) APIResponse {
 // MaxHostnames is the maximum number of hostnames that can be specified for a single domain's TLS policy.
 const MaxHostnames = 8
 
-// Extracts relevant parameters from http.Request for a POST to /api/queue
-// TODO: also validate hostnames as FQDNs.
+// Extracts relevant parameters from http.Request for a POST to /api/queue into PolicySubmission
+// If MTASTS is set, doesn't try to extract hostnames. Otherwise, expects between 1 and MaxHostnames
+// valid hostnames to be given in |r|.
 func getDomainParams(r *http.Request) (models.PolicySubmission, error) {
 	name, err := getASCIIDomain(r)
 	if err != nil {
 		return models.PolicySubmission{}, err
 	}
+	email, err := getParam("email", r)
+	if err != nil {
+		email = validationAddress(name)
+	}
 	mtasts := r.FormValue("mta-sts")
 	domain := models.PolicySubmission{
 		Name:   name,
+		Email:  email,
 		MTASTS: mtasts == "on",
-		State:  models.StateUnconfirmed,
-	}
-	email, err := getParam("email", r)
-	if err == nil {
-		domain.Email = email
-	} else {
-		domain.Email = validationAddress(&domain)
 	}
-	queueWeeks, err := getInt("weeks", r, 4, 52, 4)
-	if err != nil {
-		return domain, err
-	}
-	domain.QueueWeeks = queueWeeks
-
-	if mtasts != "on" {
+	if !domain.MTASTS {
+		p := policy.TLSPolicy{Mode: "testing", MXs: make([]string, 0)}
 		for _, hostname := range r.PostForm["hostnames"] {
 			if len(hostname) == 0 {
 				continue
@@ -204,14 +198,15 @@ func getDomainParams(r *http.Request) (models.PolicySubmission, error) {
 			if !validDomainName(strings.TrimPrefix(hostname, ".")) {
 				return domain, fmt.Errorf("Hostname %s is invalid", hostname)
 			}
-			domain.MXs = append(domain.MXs, hostname)
+			p.MXs = append(p.MXs, hostname)
 		}
-		if len(domain.MXs) == 0 {
-			return domain, fmt.Errorf("No MX hostnames supplied for domain %s", domain.Name)
+		if len(p.MXs) == 0 {
+			return domain, fmt.Errorf("No MX hostnames supplied for domain %s", name)
 		}
-		if len(domain.MXs) > MaxHostnames {
+		if len(p.MXs) > MaxHostnames {
 			return domain, fmt.Errorf("No more than 8 MX hostnames are permitted")
 		}
+		domain.Policy = &p
 	}
 	return domain, nil
 }
@@ -233,12 +228,14 @@ func (api API) Queue(r *http.Request) APIResponse {
 		if err != nil {
 			return badRequest(err.Error())
 		}
-		ok, msg, scan := domain.IsQueueable(api.Database, api.Database, api.List)
+		if !domain.CanUpdate(api.Database.Policies) {
+			return badRequest("existing submission can't be updated")
+		}
+		ok, msg := domain.HasValidScan(api.Database)
 		if !ok {
 			return badRequest(msg)
 		}
-		domain.PopulateFromScan(scan)
-		token, err := domain.InitializeWithToken(api.Database, api.Database)
+		token, err := domain.InitializeWithToken(api.Database.PendingPolicies, api.Database)
 		if err != nil {
 			return serverError(err.Error())
 		}
@@ -251,23 +248,8 @@ func (api API) Queue(r *http.Request) APIResponse {
 			Response:   fmt.Sprintf("Thank you for submitting your domain. Please check postmaster@%s to validate that you control the domain.", domain.Name),
 		}
 	}
-	// GET: Retrieve domain status from queue
-	if r.Method == http.MethodGet {
-		domainName, err := getASCIIDomain(r)
-		if err != nil {
-			return badRequest(err.Error())
-		}
-		domainObj, err := models.GetDomain(api.Database, domainName)
-		if err != nil {
-			return APIResponse{StatusCode: http.StatusNotFound, Message: err.Error()}
-		}
-		return APIResponse{
-			StatusCode: http.StatusOK,
-			Response:   domainObj,
-		}
-	}
 	return APIResponse{StatusCode: http.StatusMethodNotAllowed,
-		Message: "/api/queue only accepts POST and GET requests"}
+		Message: "/api/queue only accepts POST requests"}
 }
 
 // Validate handles requests to /api/validate
@@ -284,7 +266,7 @@ func (api API) Validate(r *http.Request) APIResponse {
 			Message: "/api/validate only accepts POST requests"}
 	}
 	tokenData := models.Token{Token: token}
-	domain, userErr, dbErr := tokenData.Redeem(api.Database, api.Database)
+	domain, userErr, dbErr := tokenData.Redeem(api.Database.PendingPolicies, api.Database.Policies, api.Database)
 	if userErr != nil {
 		return badRequest(userErr.Error())
 	}
diff --git a/email.go b/email.go
index d29272a8..d96d9dec 100644
--- a/email.go
+++ b/email.go
@@ -88,8 +88,8 @@ func makeEmailConfigFromEnv(database db.Database) (emailConfig, error) {
 	return c, nil
 }
 
-func validationAddress(domain *models.PolicySubmission) string {
-	return fmt.Sprintf("postmaster@%s", domain.Name)
+func validationAddress(name string) string {
+	return fmt.Sprintf("postmaster@%s", name)
 }
 
 func validationEmailText(domain string, contactEmail string, hostnames []string, token string, website string) string {
@@ -100,9 +100,9 @@ func validationEmailText(domain string, contactEmail string, hostnames []string,
 // SendValidation sends a validation e-mail for the domain outlined by domainInfo.
 // The validation link is generated using a token.
 func (c emailConfig) SendValidation(domain *models.PolicySubmission, token string) error {
-	emailContent := validationEmailText(domain.Name, domain.Email, domain.MXs, token,
+	emailContent := validationEmailText(domain.Name, domain.Email, domain.Policy.MXs, token,
 		c.website)
-	return c.sendEmail(validationEmailSubject, emailContent, validationAddress(domain))
+	return c.sendEmail(validationEmailSubject, emailContent, validationAddress(domain.Name))
 }
 
 func (c emailConfig) sendEmail(subject string, body string, address string) error {

From 6ab6609666c7368b6eebdd317a2e47fdedae2247 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 11:59:56 -0700
Subject: [PATCH 07/12] fix HostnamesForDomain functionality and test

---
 db/policy.go     |  9 +++------
 db/sqldb_test.go | 40 ++++++++++++++++++----------------------
 2 files changed, 21 insertions(+), 28 deletions(-)

diff --git a/db/policy.go b/db/policy.go
index 0efc824f..ae2a62ff 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -83,7 +83,7 @@ func (p *PolicyDB) PutOrUpdatePolicy(ps *models.PolicySubmission) error {
 }
 
 // DomainsToValidate [interface Validator] retrieves domains from the
-// DB whose policies should be validated.
+// DB whose policies should be validated-- all Pending policies.
 func (db SQLDatabase) DomainsToValidate() ([]string, error) {
 	domains := []string{}
 	data, err := db.PendingPolicies.GetPolicies(false)
@@ -97,12 +97,9 @@ func (db SQLDatabase) DomainsToValidate() ([]string, error) {
 }
 
 // HostnamesForDomain [interface Validator] retrieves the hostname policy for
-// a particular domain.
+// a particular domain in Pending.
 func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) {
-	data, err := db.Policies.GetPolicy(domain)
-	if err != nil {
-		data, err = db.PendingPolicies.GetPolicy(domain)
-	}
+	data, err := db.PendingPolicies.GetPolicy(domain)
 	if err != nil {
 		return []string{}, err
 	}
diff --git a/db/sqldb_test.go b/db/sqldb_test.go
index 88b4eaeb..48abe3d4 100644
--- a/db/sqldb_test.go
+++ b/db/sqldb_test.go
@@ -232,28 +232,24 @@ func TestDomainsToValidate(t *testing.T) {
 	}
 }
 
-// TODO: fix test
-// func TestHostnamesForDomain(t *testing.T) {
-// 	database.ClearTables()
-// 	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "x",
-// 		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{"x.com", "y.org"}}})
-// 	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "y",
-// 		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{}}})
-// 	result, err := database.HostnamesForDomain("x")
-// 	if err != nil {
-// 		t.Fatalf("HostnamesForDomain failed: %v\n", err)
-// 	}
-// 	if len(result) != 2 || result[0] != "x.com" || result[1] != "y.org" {
-// 		t.Errorf("Expected two hostnames, x.com and y.org\n")
-// 	}
-// 	result, err = database.HostnamesForDomain("y")
-// 	if err != nil {
-// 		t.Fatalf("HostnamesForDomain failed: %v\n", err)
-// 	}
-// 	if len(result) > 0 {
-// 		t.Errorf("Expected no hostnames to be returned, got %s\n", result[0])
-// 	}
-// }
+func TestHostnamesForDomain(t *testing.T) {
+	database.ClearTables()
+	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "x",
+		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{"x.com", "y.org"}}})
+	database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "y",
+		Policy: &policy.TLSPolicy{Mode: "testing", MXs: []string{}}})
+	result, err := database.HostnamesForDomain("x")
+	if err != nil {
+		t.Fatalf("HostnamesForDomain failed: %v\n", err)
+	}
+	if len(result) != 2 || result[0] != "x.com" || result[1] != "y.org" {
+		t.Errorf("Expected two hostnames, x.com and y.org\n")
+	}
+	result, err = database.HostnamesForDomain("y")
+	if err == nil {
+		t.Errorf("HostnamesForDomain should fail for y\n")
+	}
+}
 
 func TestPutAndIsBlacklistedEmail(t *testing.T) {
 	database.ClearTables()

From 3a40121692baea1b22be18c84680642708772224 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 12:18:30 -0700
Subject: [PATCH 08/12] Fix remaining tests and lint

---
 db/policy.go          | 14 ++++++++---
 db/sqldb.go           |  4 ++--
 models/policy.go      | 44 +++++++++++++++-------------------
 models/policy_test.go | 38 ++++++++++++++++++++++++++++-
 models/token_test.go  | 56 +++++++++++++++++++++++--------------------
 policy/policy.go      |  1 +
 queue_test.go         | 55 ++++++++++--------------------------------
 7 files changed, 112 insertions(+), 100 deletions(-)

diff --git a/db/policy.go b/db/policy.go
index ae2a62ff..2a3b22bb 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -9,10 +9,11 @@ import (
 	"github.com/EFForg/starttls-backend/policy"
 )
 
+// PolicyDB is a database of PolicySubmissions.
 type PolicyDB struct {
 	tableName string
 	conn      *sql.DB
-	locked    bool
+	strict    bool
 }
 
 func (p *PolicyDB) formQuery(query string) string {
@@ -33,6 +34,8 @@ func (p *PolicyDB) scanPolicy(result scanner) (models.PolicySubmission, error) {
 	return data, err
 }
 
+// GetPolicies returns a list of policy submissions that match
+// the mtasts status given.
 func (p *PolicyDB) GetPolicies(mtasts bool) ([]models.PolicySubmission, error) {
 	rows, err := p.conn.Query(p.formQuery(
 		"SELECT %[2]s FROM %[1]s WHERE mta_sts=$1"), mtasts)
@@ -51,23 +54,28 @@ func (p *PolicyDB) GetPolicies(mtasts bool) ([]models.PolicySubmission, error) {
 	return policies, nil
 }
 
+// GetPolicy returns the policy submission for the given domain.
 func (p *PolicyDB) GetPolicy(domainName string) (models.PolicySubmission, error) {
 	row := p.conn.QueryRow(p.formQuery(
 		"SELECT %[2]s FROM %[1]s WHERE domain=$1"), domainName)
 	return p.scanPolicy(row)
 }
 
+// RemovePolicy removes the policy submission with the given domain from
+// the database.
 func (p *PolicyDB) RemovePolicy(domainName string) (models.PolicySubmission, error) {
 	row := p.conn.QueryRow(p.formQuery(
 		"DELETE FROM %[1]s WHERE domain=$1 RETURNING %[2]s"), domainName)
 	return p.scanPolicy(row)
 }
 
+// PutOrUpdatePolicy upserts the given policy into the data store, if
+// CanUpdate passes.
 func (p *PolicyDB) PutOrUpdatePolicy(ps *models.PolicySubmission) error {
-	if p.locked && !ps.CanUpdate(p) {
+	if p.strict && !ps.CanUpdate(p) {
 		return fmt.Errorf("can't update policy in restricted table")
 	}
-	if p.locked && ps.Policy == nil {
+	if p.strict && ps.Policy == nil {
 		return fmt.Errorf("can't degrade policy in restricted table")
 	}
 	if ps.Policy == nil {
diff --git a/db/sqldb.go b/db/sqldb.go
index 6ecea8f2..e67985be 100644
--- a/db/sqldb.go
+++ b/db/sqldb.go
@@ -48,8 +48,8 @@ func InitSQLDatabase(cfg Config) (*SQLDatabase, error) {
 		return nil, err
 	}
 	return &SQLDatabase{cfg: cfg, conn: conn,
-		PendingPolicies: &PolicyDB{tableName: "pending_policies", conn: conn, locked: false},
-		Policies:        &PolicyDB{tableName: "policies", conn: conn, locked: false},
+		PendingPolicies: &PolicyDB{tableName: "pending_policies", conn: conn, strict: false},
+		Policies:        &PolicyDB{tableName: "policies", conn: conn, strict: false},
 	}, nil
 }
 
diff --git a/models/policy.go b/models/policy.go
index 640ae8d6..ece8b02f 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -28,8 +28,7 @@ type policyList interface {
 	HasDomain(string) bool
 }
 
-// TODO: test
-func (p *PolicySubmission) SamePolicy(other PolicySubmission) bool {
+func (p *PolicySubmission) samePolicy(other PolicySubmission) bool {
 	shallowEqual := p.Name == other.Name && p.MTASTS == other.MTASTS
 	if p.Policy == nil {
 		return shallowEqual && other.Policy == nil
@@ -37,11 +36,6 @@ func (p *PolicySubmission) SamePolicy(other PolicySubmission) bool {
 	return shallowEqual && p.Policy.Equals(other.Policy)
 }
 
-// TODO: test
-func (p *PolicySubmission) Valid() bool {
-	return p.MTASTS || (p.Policy != nil && len(p.Policy.MXs) > 0)
-}
-
 // CanUpdate returns whether we can update the policyStore with this particular
 // Policy Submission. In some cases, you should be able to update the
 // existing policy. In other cases, you shouldn't.
@@ -53,7 +47,7 @@ func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
 		return true
 	}
 	// If the policies are the same, return true if emails are different.
-	if p.SamePolicy(oldPolicy) {
+	if p.samePolicy(oldPolicy) {
 		return oldPolicy.Email != p.Email
 	}
 	// If old policy is manual and in testing, we can update it safely.
@@ -67,8 +61,8 @@ func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
 // sanity check. This function isn't meant to be bullet-proof since state can change
 // between initial submission and final addition to the list, but we can short-circuit
 // premature failures here on initial submission.
-func (d *PolicySubmission) HasValidScan(scans scanStore) (bool, string) {
-	scan, err := scans.GetLatestScan(d.Name)
+func (p *PolicySubmission) HasValidScan(scans scanStore) (bool, string) {
+	scan, err := scans.GetLatestScan(p.Name)
 	if err != nil {
 		return false, "We haven't scanned this domain yet. " +
 			"Please use the STARTTLS checker to scan your domain's " +
@@ -83,10 +77,10 @@ func (d *PolicySubmission) HasValidScan(scans scanStore) (bool, string) {
 		return false, "Domain hasn't passed our STARTTLS security checks"
 	}
 	// Domains without submitted MTA-STS support must match provided mx patterns.
-	if !d.MTASTS {
+	if !p.MTASTS {
 		for _, hostname := range scan.Data.PreferredHostnames {
-			if !checker.PolicyMatches(hostname, d.Policy.MXs) {
-				return false, fmt.Sprintf("Hostnames %v do not match policy %v", scan.Data.PreferredHostnames, d.Policy.MXs)
+			if !checker.PolicyMatches(hostname, p.Policy.MXs) {
+				return false, fmt.Sprintf("Hostnames %v do not match policy %v", scan.Data.PreferredHostnames, p.Policy.MXs)
 			}
 		}
 	} else if !scan.SupportsMTASTS() {
@@ -97,11 +91,11 @@ func (d *PolicySubmission) HasValidScan(scans scanStore) (bool, string) {
 
 // InitializeWithToken adds this domain to the given DomainStore and initializes a validation token
 // for the addition. The newly generated Token is returned.
-func (d *PolicySubmission) InitializeWithToken(pending policyStore, tokens tokenStore) (string, error) {
-	if err := pending.PutOrUpdatePolicy(d); err != nil {
+func (p *PolicySubmission) InitializeWithToken(pending policyStore, tokens tokenStore) (string, error) {
+	if err := pending.PutOrUpdatePolicy(p); err != nil {
 		return "", err
 	}
-	token, err := tokens.PutToken(d.Name)
+	token, err := tokens.PutToken(p.Name)
 	if err != nil {
 		return "", err
 	}
@@ -110,26 +104,26 @@ func (d *PolicySubmission) InitializeWithToken(pending policyStore, tokens token
 
 // PolicyListCheck checks the policy list status of this particular domain.
 // TODO: differentiate between errors and not-in-DB
-func (d *PolicySubmission) PolicyListCheck(pending policyStore, store policyStore, list policyList) *checker.Result {
+func (p *PolicySubmission) PolicyListCheck(pending policyStore, store policyStore, list policyList) *checker.Result {
 	result := checker.Result{Name: checker.PolicyList}
-	if list.HasDomain(d.Name) {
+	if list.HasDomain(p.Name) {
 		return result.Success()
 	}
-	_, err := store.GetPolicy(d.Name)
+	_, err := store.GetPolicy(p.Name)
 	if err == nil {
-		return result.Warning("Domain %s should soon be added to the policy list.", d.Name)
+		return result.Warning("Domain %s should soon be added to the policy list.", p.Name)
 	}
-	_, err = pending.GetPolicy(d.Name)
+	_, err = pending.GetPolicy(p.Name)
 	if err == nil {
-		return result.Failure("The policy submission for %s is waiting on email validation.", d.Name)
+		return result.Failure("The policy submission for %s is waiting on email validation.", p.Name)
 	}
-	return result.Failure("Domain %s is not on the policy list.", d.Name)
+	return result.Failure("Domain %s is not on the policy list.", p.Name)
 }
 
 // AsyncPolicyListCheck performs PolicyListCheck asynchronously.
 // domainStore and policyList should be safe for concurrent use.
-func (d PolicySubmission) AsyncPolicyListCheck(pending policyStore, store policyStore, list policyList) <-chan checker.Result {
+func (p PolicySubmission) AsyncPolicyListCheck(pending policyStore, store policyStore, list policyList) <-chan checker.Result {
 	result := make(chan checker.Result)
-	go func() { result <- *d.PolicyListCheck(pending, store, list) }()
+	go func() { result <- *p.PolicyListCheck(pending, store, list) }()
 	return result
 }
diff --git a/models/policy_test.go b/models/policy_test.go
index d67202b4..874981f0 100644
--- a/models/policy_test.go
+++ b/models/policy_test.go
@@ -30,7 +30,9 @@ func (m *mockPolicyStore) GetPolicies(_ bool) ([]PolicySubmission, error) {
 }
 
 func (m *mockPolicyStore) RemovePolicy(_ string) (PolicySubmission, error) {
-	return m.policy, m.err
+	policy := m.policy
+	m.policy.Name = "-removed-"
+	return policy, m.err
 }
 
 type mockList struct {
@@ -68,6 +70,40 @@ func (p PolicySubmission) withEmail(email string) PolicySubmission {
 	return p
 }
 
+func TestSamePolicy(t *testing.T) {
+	empty := PolicySubmission{}
+	var initialized = func() PolicySubmission {
+		return PolicySubmission{Policy: &policy.TLSPolicy{}}
+	}
+	var testCases = []struct {
+		desc     string
+		policy   PolicySubmission
+		other    PolicySubmission
+		expected bool
+	}{
+		{"Empty structs equal", empty, empty, true},
+		{"Names unequal", PolicySubmission{Name: "hello"}, PolicySubmission{Name: "nope"}, false},
+		{"MTASTS structs equal", empty.withMTASTS(), empty.withMTASTS(), true},
+		{"Modes not equal", initialized().withMode("testing"), initialized().withMode("enforce"), false},
+		{"Modes equal", initialized().withMode("testing"), initialized().withMode("testing"), true},
+		{"MXs equal",
+			initialized().withMXs([]string{"a", "b", "c"}),
+			initialized().withMXs([]string{"b", "c", "a"}), true},
+		{"MXs not equal",
+			initialized().withMXs([]string{"a", "b"}),
+			initialized().withMXs([]string{"b", "c", "a"}), false},
+		{"MXs equal but mode unequal",
+			initialized().withMode("enforce").withMXs([]string{"a", "b", "c"}),
+			initialized().withMode("testing").withMXs([]string{"b", "c", "a"}), false},
+	}
+	for _, tc := range testCases {
+		got := (&tc.policy).samePolicy(tc.other)
+		if got != tc.expected {
+			t.Errorf("%s: expected %t, got %t", tc.desc, tc.expected, got)
+		}
+	}
+}
+
 func TestCanUpdate(t *testing.T) {
 	var newP = func() PolicySubmission {
 		return PolicySubmission{Policy: &policy.TLSPolicy{}}
diff --git a/models/token_test.go b/models/token_test.go
index 9e42cd69..cc64e813 100644
--- a/models/token_test.go
+++ b/models/token_test.go
@@ -1,8 +1,8 @@
 package models
 
 import (
-// 	"errors"
-// 	"testing"
+	"errors"
+	"testing"
 )
 
 type mockTokenStore struct {
@@ -20,27 +20,31 @@ func (m *mockTokenStore) UseToken(token string) (string, error) {
 	return m.domain, m.err
 }
 
-// TODO: fix
-// func TestRedeemToken(t *testing.T) {
-// 	domains := mockDomainStore{domain: PolicySubmission{Name: "anything", State: StateUnconfirmed}, err: nil}
-// 	token := Token{Token: "token"}
-// 	domain, userErr, dbErr := token.Redeem(&domains, &mockTokenStore{domain: "anything", err: nil})
-// 	if domain != "anything" || userErr != nil || dbErr != nil {
-// 		t.Error("Expected token redeem to succeed")
-// 	}
-// 	if domains.domain.State != StateTesting {
-// 		t.Error("Expected PutDomain to have upgraded domain State")
-// 	}
-// }
-//
-// func TestRedeemTokenFailures(t *testing.T) {
-// 	token := Token{Token: "token"}
-// 	_, userErr, _ := token.Redeem(&mockDomainStore{err: nil}, &mockTokenStore{err: errors.New("")})
-// 	if userErr == nil {
-// 		t.Error("Errors reported from the token store should be interpreted as usage error (token already used, or doesn't exist)")
-// 	}
-// 	_, _, dbErr := token.Redeem(&mockDomainStore{err: errors.New("")}, &mockTokenStore{err: nil})
-// 	if dbErr == nil {
-// 		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
-// 	}
-// }
+func TestRedeemToken(t *testing.T) {
+	pending := mockPolicyStore{policy: PolicySubmission{Name: "anything"}, err: nil}
+	store := mockPolicyStore{}
+	token := Token{Token: "token"}
+	domain, userErr, dbErr := token.Redeem(&pending, &store, &mockTokenStore{domain: "anything", err: nil})
+	if domain != "anything" || userErr != nil || dbErr != nil {
+		t.Error("Expected token redeem to succeed")
+	}
+	if store.policy.Name != "anything" && pending.policy.Name != "-removed-" {
+		t.Error("Expected PutDomain to have upgraded domain State")
+	}
+}
+
+func TestRedeemTokenFailures(t *testing.T) {
+	token := Token{Token: "token"}
+	_, userErr, _ := token.Redeem(&mockPolicyStore{}, &mockPolicyStore{}, &mockTokenStore{err: errors.New("")})
+	if userErr == nil {
+		t.Error("Errors reported from the token store should be interpreted as usage error (token already used, or doesn't exist)")
+	}
+	_, _, dbErr := token.Redeem(&mockPolicyStore{}, &mockPolicyStore{err: errors.New("")}, &mockTokenStore{})
+	if dbErr == nil {
+		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
+	}
+	_, _, dbErr = token.Redeem(&mockPolicyStore{err: errors.New("")}, &mockPolicyStore{}, &mockTokenStore{})
+	if dbErr == nil {
+		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
+	}
+}
diff --git a/policy/policy.go b/policy/policy.go
index e5a9fd5c..b592c068 100644
--- a/policy/policy.go
+++ b/policy/policy.go
@@ -30,6 +30,7 @@ type List struct {
 	Policies      map[string]TLSPolicy `json:"policies"`
 }
 
+// Equals tests equality between this policy and another.
 func (p *TLSPolicy) Equals(other *TLSPolicy) bool {
 	if other == nil {
 		return false
diff --git a/queue_test.go b/queue_test.go
index 9a2537e4..e5a6e762 100644
--- a/queue_test.go
+++ b/queue_test.go
@@ -8,8 +8,6 @@ import (
 	"net/url"
 	"strings"
 	"testing"
-
-	"github.com/EFForg/starttls-backend/models"
 )
 
 func validQueueData(scan bool) url.Values {
@@ -78,34 +76,6 @@ func TestQueueDomainHidesToken(t *testing.T) {
 	}
 }
 
-func TestQueueDomainQueueWeeks(t *testing.T) {
-	defer teardown()
-
-	requestData := validQueueData(true)
-	requestData.Set("weeks", "50")
-	http.PostForm(server.URL+"/api/queue", requestData)
-	resp, _ := http.Get(server.URL + "/api/queue?domain=" + requestData.Get("domain"))
-
-	responseBody, _ := ioutil.ReadAll(resp.Body)
-	if !bytes.Contains(responseBody, []byte("50")) {
-		t.Errorf("Queueing domain should set weeks field properly")
-	}
-}
-
-func TestQueueDomainInvalidWeeks(t *testing.T) {
-	defer teardown()
-
-	requestData := validQueueData(true)
-	invalidWeeks := []string{"53", "3", "0", "-1", "abc", "5.5"}
-	for _, week := range invalidWeeks {
-		requestData.Set("weeks", week)
-		resp, _ := http.PostForm(server.URL+"/api/queue", requestData)
-		if resp.StatusCode != http.StatusBadRequest {
-			t.Fatalf("Expected POST to api/queue to fail with weeks=%s.", week)
-		}
-	}
-}
-
 // Tests basic queuing workflow.
 // Requests domain to be queued, and validates corresponding e-mail token.
 // Domain status should then be updated to "queued".
@@ -127,20 +97,14 @@ func TestBasicQueueWorkflow(t *testing.T) {
 	queueDomainGetPath := server.URL + "/api/queue?domain=" + queueDomainPostData.Get("domain")
 	resp, _ = http.Get(queueDomainGetPath)
 
-	// 2-T. Check to see domain status was initialized to 'unvalidated'
-	domainBody, _ := ioutil.ReadAll(resp.Body)
-	domain := models.PolicySubmission{}
-	err := json.Unmarshal(domainBody, &APIResponse{Response: &domain})
+	// 2-T. Check to see domain is in pending
+	domain, err := api.Database.PendingPolicies.GetPolicy("example.com")
 	if err != nil {
-		t.Fatalf("Returned invalid JSON object:%v\n", string(domainBody))
+		t.Errorf("Queued domain should be in pending")
 	}
-	if domain.State != "unvalidated" {
-		t.Fatalf("Initial state for domains should be 'unvalidated'")
-	}
-	if len(domain.MXs) != 1 {
+	if len(domain.Policy.MXs) != 1 {
 		t.Fatalf("Domain should have loaded one hostname into policy")
 	}
-
 	// 3. Validate domain token
 	token, err := api.Database.GetTokenByDomain(queueDomainPostData.Get("domain"))
 	if err != nil {
@@ -154,7 +118,7 @@ func TestBasicQueueWorkflow(t *testing.T) {
 	}
 
 	// 3-T. Ensure response body contains domain name
-	domainBody, _ = ioutil.ReadAll(resp.Body)
+	domainBody, _ := ioutil.ReadAll(resp.Body)
 	var responseObj map[string]interface{}
 	err = json.Unmarshal(domainBody, &responseObj)
 	if err != nil {
@@ -179,8 +143,13 @@ func TestBasicQueueWorkflow(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Returned invalid JSON object:%v\n", string(domainBody))
 	}
-	if domain.State != "queued" {
-		t.Fatalf("Token validation should have automatically queued domain")
+	_, err = api.Database.Policies.GetPolicy(domain.Name)
+	if err != nil {
+		t.Errorf("Token validation should have automatically queued domain")
+	}
+	_, err = api.Database.PendingPolicies.GetPolicy(domain.Name)
+	if err == nil {
+		t.Errorf("Token validation should have removed domain from pending")
 	}
 }
 

From 1316c648291e3011a52a69678f5ad93ad12325e1 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 16:13:06 -0700
Subject: [PATCH 09/12] Disambiguate between database error vs. no entry found

---
 db/policy.go          | 13 ++++++++++---
 db/sqldb_test.go      | 35 +++++++++++++++++++++++++++++++----
 models/policy.go      | 26 +++++++++++++++++---------
 models/policy_test.go | 38 ++++++++++++++++----------------------
 models/token.go       |  4 ++--
 models/token_test.go  |  9 +++++----
 queue_test.go         | 12 ++++++------
 7 files changed, 87 insertions(+), 50 deletions(-)

diff --git a/db/policy.go b/db/policy.go
index 2a3b22bb..78e42c18 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -55,10 +55,14 @@ func (p *PolicyDB) GetPolicies(mtasts bool) ([]models.PolicySubmission, error) {
 }
 
 // GetPolicy returns the policy submission for the given domain.
-func (p *PolicyDB) GetPolicy(domainName string) (models.PolicySubmission, error) {
+func (p *PolicyDB) GetPolicy(domainName string) (models.PolicySubmission, bool, error) {
 	row := p.conn.QueryRow(p.formQuery(
 		"SELECT %[2]s FROM %[1]s WHERE domain=$1"), domainName)
-	return p.scanPolicy(row)
+	result, err := p.scanPolicy(row)
+	if err == sql.ErrNoRows {
+		return result, false, nil
+	}
+	return result, true, err
 }
 
 // RemovePolicy removes the policy submission with the given domain from
@@ -107,7 +111,10 @@ func (db SQLDatabase) DomainsToValidate() ([]string, error) {
 // HostnamesForDomain [interface Validator] retrieves the hostname policy for
 // a particular domain in Pending.
 func (db SQLDatabase) HostnamesForDomain(domain string) ([]string, error) {
-	data, err := db.PendingPolicies.GetPolicy(domain)
+	data, ok, err := db.PendingPolicies.GetPolicy(domain)
+	if !ok {
+		err = fmt.Errorf("domain %s not in database", domain)
+	}
 	if err != nil {
 		return []string{}, err
 	}
diff --git a/db/sqldb_test.go b/db/sqldb_test.go
index 48abe3d4..e0d412c4 100644
--- a/db/sqldb_test.go
+++ b/db/sqldb_test.go
@@ -137,6 +137,30 @@ func TestGetAllScans(t *testing.T) {
 	}
 }
 
+func TestGetNonexistentPolicy(t *testing.T) {
+	database.ClearTables()
+	_, ok, err := database.Policies.GetPolicy("fake")
+	if err != nil || ok {
+		t.Errorf("Expected nothing to return and ok to be false.")
+	}
+}
+
+func TestGetPoliciesEmpty(t *testing.T) {
+	database.ClearTables()
+	_, err := database.Policies.GetPolicies(false)
+	if err != nil {
+		t.Errorf("Get policies failed: %v", err)
+	}
+	err = database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "abcde", MTASTS: true})
+	policies, err := database.Policies.GetPolicies(false)
+	if len(policies) > 0 {
+		t.Errorf("Should not have returned any policies.")
+	}
+	if err != nil {
+		t.Errorf("Get policies failed: %v", err)
+	}
+}
+
 func TestPutGetDomain(t *testing.T) {
 	database.ClearTables()
 	data := models.PolicySubmission{
@@ -147,9 +171,9 @@ func TestPutGetDomain(t *testing.T) {
 	if err != nil {
 		t.Errorf("PutDomain failed: %v\n", err)
 	}
-	retrievedData, err := database.Policies.GetPolicy(data.Name)
-	if err != nil {
-		t.Errorf("GetDomain(%s) failed: %v\n", data.Name, err)
+	retrievedData, ok, err := database.Policies.GetPolicy(data.Name)
+	if !ok || err != nil {
+		t.Fatalf("GetDomain(%s) failed: %v\n", data.Name, err)
 	}
 	if retrievedData.Name != data.Name {
 		t.Errorf("Somehow, GetDomain retrieved the wrong object?")
@@ -172,7 +196,10 @@ func TestUpsertDomain(t *testing.T) {
 	if err != nil {
 		t.Errorf("PutDomain(%s) failed: %v\n", "testing.com", err)
 	}
-	retrievedData, err := database.Policies.GetPolicy("testing.com")
+	retrievedData, ok, err := database.Policies.GetPolicy("testing.com")
+	if !ok || err != nil {
+		t.Fatalf("GetPolicy failed: %v", err)
+	}
 	if retrievedData.Policy.MXs[0] != "hello_darkness_my_old_friend" || retrievedData.Email != "actual_admin@testing.com" {
 		t.Errorf("Email and MXs should have been rewritten: %v\n", retrievedData)
 	}
diff --git a/models/policy.go b/models/policy.go
index ece8b02f..0ba7abc5 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -19,7 +19,7 @@ type PolicySubmission struct {
 // policyStore is a simple interface for fetching and adding domain objects.
 type policyStore interface {
 	PutOrUpdatePolicy(*PolicySubmission) error
-	GetPolicy(string) (PolicySubmission, error)
+	GetPolicy(string) (PolicySubmission, bool, error)
 	GetPolicies(bool) ([]PolicySubmission, error)
 	RemovePolicy(string) (PolicySubmission, error)
 }
@@ -40,12 +40,15 @@ func (p *PolicySubmission) samePolicy(other PolicySubmission) bool {
 // Policy Submission. In some cases, you should be able to update the
 // existing policy. In other cases, you shouldn't.
 func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
-	oldPolicy, err := policies.GetPolicy(p.Name)
+	oldPolicy, ok, err := policies.GetPolicy(p.Name)
 	// If this policy doesn't exist in the policyStore, we can add it.
-	// TODO: not to conflate between real errors and "not present" errors.
-	if err != nil {
+	if !ok {
 		return true
 	}
+	// If something messed up, we can't add it.
+	if err != nil {
+		return false
+	}
 	// If the policies are the same, return true if emails are different.
 	if p.samePolicy(oldPolicy) {
 		return oldPolicy.Email != p.Email
@@ -103,20 +106,25 @@ func (p *PolicySubmission) InitializeWithToken(pending policyStore, tokens token
 }
 
 // PolicyListCheck checks the policy list status of this particular domain.
-// TODO: differentiate between errors and not-in-DB
 func (p *PolicySubmission) PolicyListCheck(pending policyStore, store policyStore, list policyList) *checker.Result {
 	result := checker.Result{Name: checker.PolicyList}
 	if list.HasDomain(p.Name) {
 		return result.Success()
 	}
-	_, err := store.GetPolicy(p.Name)
-	if err == nil {
+	_, ok, err := store.GetPolicy(p.Name)
+	if ok {
 		return result.Warning("Domain %s should soon be added to the policy list.", p.Name)
 	}
-	_, err = pending.GetPolicy(p.Name)
-	if err == nil {
+	if err != nil {
+		return result.Error("Error retrieving domain from database.")
+	}
+	_, ok, err = pending.GetPolicy(p.Name)
+	if ok {
 		return result.Failure("The policy submission for %s is waiting on email validation.", p.Name)
 	}
+	if err != nil {
+		return result.Error("Error retrieving domain from database.")
+	}
 	return result.Failure("Domain %s is not on the policy list.", p.Name)
 }
 
diff --git a/models/policy_test.go b/models/policy_test.go
index 874981f0..eae913c8 100644
--- a/models/policy_test.go
+++ b/models/policy_test.go
@@ -12,6 +12,7 @@ import (
 
 type mockPolicyStore struct {
 	policy   PolicySubmission
+	ok       bool
 	policies []PolicySubmission
 	err      error
 }
@@ -21,8 +22,8 @@ func (m *mockPolicyStore) PutOrUpdatePolicy(p *PolicySubmission) error {
 	return m.err
 }
 
-func (m *mockPolicyStore) GetPolicy(domain string) (PolicySubmission, error) {
-	return m.policy, m.err
+func (m *mockPolicyStore) GetPolicy(domain string) (PolicySubmission, bool, error) {
+	return m.policy, m.ok, m.err
 }
 
 func (m *mockPolicyStore) GetPolicies(_ bool) ([]PolicySubmission, error) {
@@ -112,35 +113,36 @@ func TestCanUpdate(t *testing.T) {
 		desc      string
 		oldPolicy PolicySubmission // Return from GetPolicy
 		policy    PolicySubmission // Policy to try to insert
+		ok        bool             // Does GetPolicy return OK?
 		err       error            // Does GetPolicy return an error?
 		expected  bool
 	}{
-		{"policy not found", newP(), newP(), errors.New("policy not found in DB"), true},
-		{"no update if policies same 1", newP().withMode("testing"), newP().withMode("testing"), nil, false},
-		{"no update if policies same 2", newP().withMode("enforce"), newP().withMode("enforce"), nil, false},
+		{"policy not found", newP(), newP(), false, nil, true},
+		{"no update if policies same 1", newP().withMode("testing"), newP().withMode("testing"), true, nil, false},
+		{"no update if policies same 2", newP().withMode("enforce"), newP().withMode("enforce"), true, nil, false},
 		{"no update if policies same 3",
 			newP().withMode("enforce").withMTASTS().withMXs([]string{"a", "b", "c"}),
 			newP().withMode("enforce").withMTASTS().withMXs([]string{"a", "c", "b"}),
-			nil, false},
-		{"can upgrade to manual enforce", newP().withMode("testing"), newP().withMode("enforce"), nil, true},
-		{"can't downgrade to enforce", newP().withMode("enforce"), newP().withMode("testing"), nil, false},
+			true, nil, false},
+		{"can upgrade to manual enforce", newP().withMode("testing"), newP().withMode("enforce"), true, nil, true},
+		{"can't downgrade to enforce", newP().withMode("enforce"), newP().withMode("testing"), true, nil, false},
 		{"prevent upgrade to enforce with MTA-STS",
 			newP().withMTASTS().withMode("testing"),
 			newP().withMode("enforce"),
-			nil, false},
+			true, nil, false},
 		{"no mx changes with MTA-STS, even in testing",
 			newP().withMTASTS().withMode("testing").withMXs([]string{"a", "b"}),
 			newP().withMTASTS().withMode("testing").withMXs([]string{"a", "b", "c"}),
-			nil, false},
+			true, nil, false},
 		{"mx can change in testing",
 			newP().withMode("testing").withMXs([]string{"a", "b"}),
 			newP().withMode("testing").withMXs([]string{"a", "b", "c"}),
-			nil, true},
+			true, nil, true},
 		{"update email", newP().withEmail("abc").withMode("enforce"), newP().withEmail("a").withMode("enforce"),
-			nil, true},
+			true, nil, true},
 	}
 	for _, tc := range testCases {
-		store := mockPolicyStore{policy: tc.oldPolicy, err: tc.err}
+		store := mockPolicyStore{policy: tc.oldPolicy, err: tc.err, ok: tc.ok}
 		got := (&tc.policy).CanUpdate(&store)
 		if got != tc.expected {
 			t.Errorf("%s: expected %t but got %t",
@@ -223,16 +225,8 @@ func TestPolicyCheck(t *testing.T) {
 	}
 	for _, tc := range testCases {
 		policy := &PolicySubmission{Policy: &policy.TLSPolicy{}}
-		var dbErr error
-		if !tc.inDB {
-			dbErr = errors.New("DB err")
-		}
-		var pendingDBErr error
-		if !tc.inPendingDB {
-			pendingDBErr = errors.New("pending err")
-		}
 		result := policy.PolicyListCheck(
-			&mockPolicyStore{err: pendingDBErr}, &mockPolicyStore{err: dbErr}, mockList{tc.onList})
+			&mockPolicyStore{ok: tc.inPendingDB}, &mockPolicyStore{ok: tc.inDB}, mockList{tc.onList})
 		if result.Status != tc.expected {
 			t.Errorf("%s: expected status %d, got result %v", tc.desc, tc.expected, result)
 		}
diff --git a/models/token.go b/models/token.go
index 7ae14f00..fc375aa7 100644
--- a/models/token.go
+++ b/models/token.go
@@ -23,8 +23,8 @@ func (t *Token) Redeem(pending policyStore, store policyStore, tokens tokenStore
 	if err != nil {
 		return domain, err, nil
 	}
-	domainData, err := pending.GetPolicy(domain)
-	if err != nil {
+	domainData, ok, err := pending.GetPolicy(domain)
+	if !ok || err != nil {
 		return domain, nil, err
 	}
 	err = store.PutOrUpdatePolicy(&domainData)
diff --git a/models/token_test.go b/models/token_test.go
index cc64e813..9f822ad2 100644
--- a/models/token_test.go
+++ b/models/token_test.go
@@ -21,7 +21,7 @@ func (m *mockTokenStore) UseToken(token string) (string, error) {
 }
 
 func TestRedeemToken(t *testing.T) {
-	pending := mockPolicyStore{policy: PolicySubmission{Name: "anything"}, err: nil}
+	pending := mockPolicyStore{policy: PolicySubmission{Name: "anything"}, err: nil, ok: true}
 	store := mockPolicyStore{}
 	token := Token{Token: "token"}
 	domain, userErr, dbErr := token.Redeem(&pending, &store, &mockTokenStore{domain: "anything", err: nil})
@@ -34,16 +34,17 @@ func TestRedeemToken(t *testing.T) {
 }
 
 func TestRedeemTokenFailures(t *testing.T) {
+	emptyStore := &mockPolicyStore{ok: true}
 	token := Token{Token: "token"}
-	_, userErr, _ := token.Redeem(&mockPolicyStore{}, &mockPolicyStore{}, &mockTokenStore{err: errors.New("")})
+	_, userErr, _ := token.Redeem(emptyStore, emptyStore, &mockTokenStore{err: errors.New("")})
 	if userErr == nil {
 		t.Error("Errors reported from the token store should be interpreted as usage error (token already used, or doesn't exist)")
 	}
-	_, _, dbErr := token.Redeem(&mockPolicyStore{}, &mockPolicyStore{err: errors.New("")}, &mockTokenStore{})
+	_, _, dbErr := token.Redeem(emptyStore, &mockPolicyStore{err: errors.New("")}, &mockTokenStore{})
 	if dbErr == nil {
 		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
 	}
-	_, _, dbErr = token.Redeem(&mockPolicyStore{err: errors.New("")}, &mockPolicyStore{}, &mockTokenStore{})
+	_, _, dbErr = token.Redeem(&mockPolicyStore{err: errors.New("")}, emptyStore, &mockTokenStore{})
 	if dbErr == nil {
 		t.Error("Errors reported from the domain store should be interpreted as a hard failure")
 	}
diff --git a/queue_test.go b/queue_test.go
index e5a6e762..9202d64d 100644
--- a/queue_test.go
+++ b/queue_test.go
@@ -98,8 +98,8 @@ func TestBasicQueueWorkflow(t *testing.T) {
 	resp, _ = http.Get(queueDomainGetPath)
 
 	// 2-T. Check to see domain is in pending
-	domain, err := api.Database.PendingPolicies.GetPolicy("example.com")
-	if err != nil {
+	domain, ok, err := api.Database.PendingPolicies.GetPolicy("example.com")
+	if err != nil || !ok {
 		t.Errorf("Queued domain should be in pending")
 	}
 	if len(domain.Policy.MXs) != 1 {
@@ -143,12 +143,12 @@ func TestBasicQueueWorkflow(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Returned invalid JSON object:%v\n", string(domainBody))
 	}
-	_, err = api.Database.Policies.GetPolicy(domain.Name)
-	if err != nil {
+	_, ok, err = api.Database.Policies.GetPolicy(domain.Name)
+	if err != nil || !ok {
 		t.Errorf("Token validation should have automatically queued domain")
 	}
-	_, err = api.Database.PendingPolicies.GetPolicy(domain.Name)
-	if err == nil {
+	_, ok, err = api.Database.PendingPolicies.GetPolicy(domain.Name)
+	if ok {
 		t.Errorf("Token validation should have removed domain from pending")
 	}
 }

From 5e3cf87b1b066518df37c6ccaddd9d3d285b2483 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 16:26:49 -0700
Subject: [PATCH 10/12] Test more cases in models/policy

---
 models/policy.go      |  8 ++++----
 models/policy_test.go | 21 +++++++++++++--------
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/models/policy.go b/models/policy.go
index 0ba7abc5..0962a738 100644
--- a/models/policy.go
+++ b/models/policy.go
@@ -41,14 +41,14 @@ func (p *PolicySubmission) samePolicy(other PolicySubmission) bool {
 // existing policy. In other cases, you shouldn't.
 func (p *PolicySubmission) CanUpdate(policies policyStore) bool {
 	oldPolicy, ok, err := policies.GetPolicy(p.Name)
-	// If this policy doesn't exist in the policyStore, we can add it.
-	if !ok {
-		return true
-	}
 	// If something messed up, we can't add it.
 	if err != nil {
 		return false
 	}
+	// If this policy doesn't exist in the policyStore, we can add it.
+	if !ok {
+		return true
+	}
 	// If the policies are the same, return true if emails are different.
 	if p.samePolicy(oldPolicy) {
 		return oldPolicy.Email != p.Email
diff --git a/models/policy_test.go b/models/policy_test.go
index eae913c8..72b9d7e5 100644
--- a/models/policy_test.go
+++ b/models/policy_test.go
@@ -118,6 +118,7 @@ func TestCanUpdate(t *testing.T) {
 		expected  bool
 	}{
 		{"policy not found", newP(), newP(), false, nil, true},
+		{"DB throws error", newP(), newP(), false, errors.New("Oh no"), false},
 		{"no update if policies same 1", newP().withMode("testing"), newP().withMode("testing"), true, nil, false},
 		{"no update if policies same 2", newP().withMode("enforce"), newP().withMode("enforce"), true, nil, false},
 		{"no update if policies same 3",
@@ -175,8 +176,8 @@ func TestValidScan(t *testing.T) {
 		return scan
 	}
 	failedScan := Scan{
-		Data: checker.DomainResult{Status: checker.DomainFailure},
-	}
+		Data:      checker.DomainResult{Status: checker.DomainFailure},
+		Timestamp: time.Now()}
 	var testCases = []struct {
 		desc     string
 		mxs      []string
@@ -212,21 +213,25 @@ func TestValidScan(t *testing.T) {
 
 func TestPolicyCheck(t *testing.T) {
 	var testCases = []struct {
-		desc        string
-		onList      bool
-		inDB        bool
-		inPendingDB bool
-		expected    checker.Status
+		desc         string
+		onList       bool
+		inDB         bool
+		errDB        error
+		errPendingDB error
+		inPendingDB  bool
+		expected     checker.Status
 	}{
 		{desc: "Domain on the list should return success", onList: true, expected: checker.Success},
 		{desc: "Domain not on list but in policies DB should return warning", inDB: true, expected: checker.Warning},
+		{desc: "DB error should surface", errDB: errors.New(""), expected: checker.Error},
+		{desc: "Pending DB error should surface", errPendingDB: errors.New(""), expected: checker.Error},
 		{desc: "Domain in pending policies DB should return failure", inPendingDB: true, expected: checker.Failure},
 		{desc: "Domain not anywhere should return failure", expected: checker.Failure},
 	}
 	for _, tc := range testCases {
 		policy := &PolicySubmission{Policy: &policy.TLSPolicy{}}
 		result := policy.PolicyListCheck(
-			&mockPolicyStore{ok: tc.inPendingDB}, &mockPolicyStore{ok: tc.inDB}, mockList{tc.onList})
+			&mockPolicyStore{err: tc.errPendingDB, ok: tc.inPendingDB}, &mockPolicyStore{err: tc.errDB, ok: tc.inDB}, mockList{tc.onList})
 		if result.Status != tc.expected {
 			t.Errorf("%s: expected status %d, got result %v", tc.desc, tc.expected, result)
 		}

From 71bd03a6cb16d38a67c5b84c2ce12f903d02e9f3 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 18:03:53 -0700
Subject: [PATCH 11/12] Ported basic functionality from other branch

---
 db/policy.go           |  4 +--
 main.go                | 19 +++++++++-----
 validator/validator.go | 59 ++++++++++++++++++++++++------------------
 3 files changed, 49 insertions(+), 33 deletions(-)

diff --git a/db/policy.go b/db/policy.go
index 78e42c18..335ead02 100644
--- a/db/policy.go
+++ b/db/policy.go
@@ -96,9 +96,9 @@ func (p *PolicyDB) PutOrUpdatePolicy(ps *models.PolicySubmission) error {
 
 // DomainsToValidate [interface Validator] retrieves domains from the
 // DB whose policies should be validated-- all Pending policies.
-func (db SQLDatabase) DomainsToValidate() ([]string, error) {
+func (p *PolicyDB) DomainsToValidate() ([]string, error) {
 	domains := []string{}
-	data, err := db.PendingPolicies.GetPolicies(false)
+	data, err := p.GetPolicies(true)
 	if err != nil {
 		return domains, err
 	}
diff --git a/main.go b/main.go
index e38b5f76..56435c1a 100644
--- a/main.go
+++ b/main.go
@@ -114,14 +114,21 @@ func main() {
 		Emailer:     emailConfig,
 	}
 	api.parseTemplates()
-	if os.Getenv("VALIDATE_LIST") == "1" {
-		log.Println("[Starting list validator]")
-		go validator.ValidateRegularly("Live policy list", list, 24*time.Hour)
-	}
+	// if os.Getenv("VALIDATE_LIST") == "1" {
+	// 	log.Println("[Starting list validator]")
+	// 	go validator.ValidateRegularly("Live policy list", list, 24*time.Hour)
+	// }
 	if os.Getenv("VALIDATE_QUEUED") == "1" {
-		log.Println("[Starting queued validator]")
-		go validator.ValidateRegularly("Testing domains", db, 24*time.Hour)
+		v := validator.Validator{
+			Name:     "testing and enforced domains",
+			Store:    db.Policies,
+			Interval: 24 * time.Hour,
+		}
+		go v.Run()
+		// log.Println("[Starting queued validator]")
+		// 	go validator.ValidateRegularly("MTA-STS domains", db.Policies, 24*time.Hour)
 	}
+	// go validator.ValidateRegularly("MTA-STS domains", db.Policies, 24*time.Hour)
 	go stats.UpdateRegularly(db, time.Hour)
 	ServePublicEndpoints(&api, &cfg)
 }
diff --git a/validator/validator.go b/validator/validator.go
index 87d0b606..ce270df0 100644
--- a/validator/validator.go
+++ b/validator/validator.go
@@ -6,6 +6,8 @@ import (
 	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
+	"github.com/EFForg/starttls-backend/models"
+	"github.com/EFForg/starttls-backend/policy"
 	"github.com/getsentry/raven-go"
 )
 
@@ -14,7 +16,7 @@ import (
 // expected hostnames).
 type DomainPolicyStore interface {
 	DomainsToValidate() ([]string, error)
-	HostnamesForDomain(string) ([]string, error)
+	GetPolicy(string) (models.PolicySubmission, bool, error)
 }
 
 // Called with failure by defaault.
@@ -28,7 +30,7 @@ func reportToSentry(name string, domain string, result checker.DomainResult) {
 		result)
 }
 
-type checkPerformer func(string, []string) checker.DomainResult
+type checkPerformer func(models.PolicySubmission) checker.DomainResult
 type resultCallback func(string, string, checker.DomainResult)
 
 // Validator runs checks regularly against domain policies. This structure
@@ -47,18 +49,37 @@ type Validator struct {
 	OnFailure resultCallback
 	// OnSuccess: optional. Called when a particular policy validation succeeds.
 	OnSuccess resultCallback
-	// checkPerformer: performs the check.
-	checkPerformer checkPerformer
+	// CheckPerformer: performs the check.
+	CheckPerformer checkPerformer
 }
 
-func (v *Validator) checkPolicy(domain string, hostnames []string) checker.DomainResult {
-	if v.checkPerformer == nil {
-		c := checker.Checker{
-			Cache: checker.MakeSimpleCache(time.Hour),
+func resultMTASTSToPolicy(r *checker.MTASTSResult) *policy.TLSPolicy {
+	return &policy.TLSPolicy{Mode: r.Mode, MXs: r.MXs}
+}
+
+func getMTASTSUpdater(update func(*models.PolicySubmission) error) checkPerformer {
+	c := checker.Checker{Cache: checker.MakeSimpleCache(time.Hour)}
+	return func(p models.PolicySubmission) checker.DomainResult {
+		if p.MTASTS {
+			result := c.CheckDomain(p.Name, []string{})
+			if !p.Policy.Equals(resultMTASTSToPolicy(result.MTASTSResult)) {
+				if err := update(&p); err != nil {
+					reportToSentry(fmt.Sprintf("couldn't update policy in DB: %v", err), p.Name, result)
+				}
+			}
+		}
+		return c.CheckDomain(p.Name, p.Policy.MXs)
+	}
+}
+
+func (v *Validator) checkPolicy(p *models.PolicySubmission) checker.DomainResult {
+	if v.CheckPerformer == nil {
+		c := checker.Checker{Cache: checker.MakeSimpleCache(time.Hour)}
+		v.CheckPerformer = func(policy models.PolicySubmission) checker.DomainResult {
+			return c.CheckDomain(p.Name, p.Policy.MXs)
 		}
-		v.checkPerformer = c.CheckDomain
 	}
-	return v.checkPerformer(domain, hostnames)
+	return v.CheckPerformer(*p)
 }
 
 func (v *Validator) interval() time.Duration {
@@ -93,12 +114,12 @@ func (v *Validator) Run() {
 			continue
 		}
 		for _, domain := range domains {
-			hostnames, err := v.Store.HostnamesForDomain(domain)
-			if err != nil {
+			policy, ok, err := v.Store.GetPolicy(domain)
+			if err != nil || !ok {
 				log.Printf("[%s validator] Could not retrieve policy for domain %s: %v", v.Name, domain, err)
 				continue
 			}
-			result := v.checkPolicy(domain, hostnames)
+			result := v.checkPolicy(&policy)
 			if result.Status != 0 {
 				log.Printf("[%s validator] %s failed; sending report", v.Name, domain)
 				v.policyFailed(v.Name, domain, result)
@@ -108,15 +129,3 @@ func (v *Validator) Run() {
 		}
 	}
 }
-
-// ValidateRegularly regularly runs checker.CheckDomain against a Domain-
-// Hostname map. Interval specifies the interval to wait between each run.
-// Failures are reported to Sentry.
-func ValidateRegularly(name string, store DomainPolicyStore, interval time.Duration) {
-	v := Validator{
-		Name:     name,
-		Store:    store,
-		Interval: interval,
-	}
-	v.Run()
-}

From ec01f3138d9ba5e2efa4e1ec01bd143b669ae485 Mon Sep 17 00:00:00 2001
From: sydneyli <sydney@eff.org>
Date: Fri, 21 Jun 2019 18:08:12 -0700
Subject: [PATCH 12/12] Fix existing tests

---
 db/sqldb_test.go            | 23 -----------------------
 validator/validator_test.go | 16 +++++++++-------
 2 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/db/sqldb_test.go b/db/sqldb_test.go
index e0d412c4..7deff9f7 100644
--- a/db/sqldb_test.go
+++ b/db/sqldb_test.go
@@ -236,29 +236,6 @@ func TestPutTokenTwice(t *testing.T) {
 	}
 }
 
-func TestDomainsToValidate(t *testing.T) {
-	database.ClearTables()
-	mtastsMap := map[string]bool{
-		"a": false, "b": true, "c": false, "d": true,
-	}
-	for domain, mtasts := range mtastsMap {
-		if mtasts {
-			database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: domain, MTASTS: true})
-		} else {
-			database.Policies.PutOrUpdatePolicy(&models.PolicySubmission{Name: domain})
-		}
-	}
-	result, err := database.DomainsToValidate()
-	if err != nil {
-		t.Fatalf("DomainsToValidate failed: %v\n", err)
-	}
-	for _, domain := range result {
-		if !mtastsMap[domain] {
-			t.Errorf("Did not expect %s to be returned", domain)
-		}
-	}
-}
-
 func TestHostnamesForDomain(t *testing.T) {
 	database.ClearTables()
 	database.PendingPolicies.PutOrUpdatePolicy(&models.PolicySubmission{Name: "x",
diff --git a/validator/validator_test.go b/validator/validator_test.go
index eec55a9f..8da4084f 100644
--- a/validator/validator_test.go
+++ b/validator/validator_test.go
@@ -5,6 +5,8 @@ import (
 	"time"
 
 	"github.com/EFForg/starttls-backend/checker"
+	"github.com/EFForg/starttls-backend/models"
+	"github.com/EFForg/starttls-backend/policy"
 )
 
 type mockDomainPolicyStore struct {
@@ -19,21 +21,21 @@ func (m mockDomainPolicyStore) DomainsToValidate() ([]string, error) {
 	return domains, nil
 }
 
-func (m mockDomainPolicyStore) HostnamesForDomain(domain string) ([]string, error) {
-	return m.hostnames[domain], nil
+func (m mockDomainPolicyStore) GetPolicy(domain string) (models.PolicySubmission, bool, error) {
+	return models.PolicySubmission{Name: domain, Policy: &policy.TLSPolicy{Mode: "testing", MXs: m.hostnames[domain]}}, true, nil
 }
 
 func noop(_ string, _ string, _ checker.DomainResult) {}
 
 func TestRegularValidationValidates(t *testing.T) {
 	called := make(chan bool)
-	fakeChecker := func(domain string, hostnames []string) checker.DomainResult {
+	fakeChecker := func(_ models.PolicySubmission) checker.DomainResult {
 		called <- true
 		return checker.DomainResult{}
 	}
 	mock := mockDomainPolicyStore{
 		hostnames: map[string][]string{"a": []string{"hostname"}}}
-	v := Validator{Store: mock, Interval: 100 * time.Millisecond, checkPerformer: fakeChecker, OnFailure: noop}
+	v := Validator{Store: mock, Interval: 100 * time.Millisecond, CheckPerformer: fakeChecker, OnFailure: noop}
 	go v.Run()
 
 	select {
@@ -46,8 +48,8 @@ func TestRegularValidationValidates(t *testing.T) {
 
 func TestRegularValidationReportsErrors(t *testing.T) {
 	reports := make(chan string)
-	fakeChecker := func(domain string, hostnames []string) checker.DomainResult {
-		if domain == "fail" || domain == "error" {
+	fakeChecker := func(p models.PolicySubmission) checker.DomainResult {
+		if p.Name == "fail" || p.Name == "error" {
 			return checker.DomainResult{Status: 5}
 		}
 		return checker.DomainResult{Status: 0}
@@ -64,7 +66,7 @@ func TestRegularValidationReportsErrors(t *testing.T) {
 			"fail":   []string{"hostname"},
 			"error":  []string{"hostname"},
 			"normal": []string{"hostname"}}}
-	v := Validator{Store: mock, Interval: 100 * time.Millisecond, checkPerformer: fakeChecker,
+	v := Validator{Store: mock, Interval: 100 * time.Millisecond, CheckPerformer: fakeChecker,
 		OnFailure: fakeReporter, OnSuccess: fakeSuccessReporter,
 	}
 	go v.Run()