Skip to content
This repository has been archived by the owner on Jul 12, 2023. It is now read-only.

Commit

Permalink
Require region codes are globally unique
Browse files Browse the repository at this point in the history
This is required for the ENX redirector, but we've also always made the
assumption that these need to be globally unique.

Note that this requires changing the column to citext (for
case-insensitive matching) and to NULL out the default of '', since ''
is a duplicate of ''.

It also handles any existing duplicates by appending -N to them before
applying the database-level unique constraint.
  • Loading branch information
sethvargo committed Sep 21, 2020
1 parent 099283b commit bf22392
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 6 deletions.
4 changes: 3 additions & 1 deletion cmd/server/assets/realmadmin/_form_general.html
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,12 @@
<small class="form-text text-muted">
The realm name is displayed on the realm selection page and in the header
when selected. Choose a descriptive name that your team will recognize.
This value must be globally unique in the system.
</small>
</div>

<div class="form-label-group">
<input type="text" name="region_code" id="region-code" class="form-control{{if $realm.ErrorsFor "regionCode"}} is-invalid{{end}}"
<input type="text" name="region_code" id="region-code" class="form-control text-uppercase{{if $realm.ErrorsFor "regionCode"}} is-invalid{{end}}"
value="{{$realm.RegionCode}}" placeholder="Region code" />
<label for="region-code">Region code</label>
{{if $realm.ErrorsFor "regionCode"}}
Expand All @@ -41,6 +42,7 @@
<a href="https://en.wikipedia.org/wiki/List_of_ISO_3166_country_codes">ISO
3166-1 country codes and ISO 3166-2 subdivision codes</a> where
applicable. For example, Washington state would be <code>US-WA</code>.
This value must globally unique in the system.
</small>
</div>

Expand Down
11 changes: 6 additions & 5 deletions pkg/controller/realmadmin/settings.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"
"fmt"
"net/http"
"strings"
"time"

"github.com/google/exposure-notifications-verification-server/pkg/controller"
Expand Down Expand Up @@ -109,13 +110,13 @@ func (c *Controller) HandleSettings() http.Handler {
}

// General
if v := form.Name; v != "" {
if v := strings.TrimSpace(form.Name); v != "" {
realm.Name = v
}
if v := form.RegionCode; v != "" {
realm.RegionCode = v
if v := strings.TrimSpace(form.RegionCode); v != "" {
realm.RegionCode = strings.ToUpper(v)
}
if v := form.WelcomeMessage; v != "" {
if v := strings.TrimSpace(form.WelcomeMessage); v != "" {
realm.WelcomeMessage = v
}

Expand All @@ -138,7 +139,7 @@ func (c *Controller) HandleSettings() http.Handler {
if v := form.LongCodeDurationHours; v > 0 {
realm.LongCodeDuration.Duration = time.Duration(v) * time.Minute
}
if v := form.SMSTextTemplate; v != "" {
if v := strings.TrimSpace(form.SMSTextTemplate); v != "" {
realm.SMSTextTemplate = v
}

Expand Down
78 changes: 78 additions & 0 deletions pkg/database/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,84 @@ func (db *Database) getMigrations(ctx context.Context) *gormigrate.Gormigrate {
return nil
},
},
{
ID: "00049-MakeRegionCodeUnique",
Migrate: func(tx *gorm.DB) error {
// Make any existing empty string region codes to NULL. Without this,
// the new unique constraint will fail.
if err := tx.Exec("UPDATE realms SET region_code = NULL WHERE TRIM(region_code) = ''").Error; err != nil {
return err
}

// Make all region codes uppercase.
if err := tx.Exec("UPDATE realms SET region_code = UPPER(region_code) WHERE region_code IS NOT NULL").Error; err != nil {
return err
}

// Find any existing duplicate region codes - this could be combined
// into a much large SQL statement with the next thing, but I'm
// optimizing for readability here.
var dupRegionCodes []string
if err := tx.Model(&Realm{}).
Unscoped().
Select("UPPER(region_code) AS region_code").
Where("region_code IS NOT NULL").
Group("region_code").
Having("COUNT(*) > 1").
Pluck("region_code", &dupRegionCodes).
Error; err != nil {
return err
}

// Update any duplicate regions to not be duplicate anymore.
for _, dupRegionCode := range dupRegionCodes {
logger.Warn("de-duplicating region code %q", dupRegionCode)

// I call this the "Microsoft method". For each duplicate realm,
// append -N, starting with 1. If there are 3 realms with the region
// code "PA", their new values will be "PA", "PA-1", and "PA-2"
// respectively.
sql := `
UPDATE
realms
SET region_code = CONCAT(realms.region_code, '-', (z-1)::text)
FROM (
SELECT
id,
region_code,
ROW_NUMBER() OVER (ORDER BY id ASC) AS z
FROM realms
WHERE UPPER(region_code) = UPPER($1)
) AS sq
WHERE realms.id = sq.id AND sq.z > 1`
if err := tx.Exec(sql, dupRegionCode).Error; err != nil {
return err
}
}

sqls := []string{
// There's already a runtime constraint and validation on names, this
// is just an extra layer of protection at the database layer.
"ALTER TABLE realms ALTER COLUMN name SET NOT NULL",

// Make region code case insensitive and unique.
"ALTER TABLE realms ALTER COLUMN region_code TYPE CITEXT",
"ALTER TABLE realms ALTER COLUMN region_code DROP DEFAULT",
"ALTER TABLE realms ALTER COLUMN region_code DROP NOT NULL",
"ALTER TABLE realms ADD CONSTRAINT uix_realms_region_code UNIQUE (region_code)",
}

for _, sql := range sqls {
if err := tx.Exec(sql).Error; err != nil {
return err
}
}
return nil
},
Rollback: func(tx *gorm.DB) error {
return nil
},
},
})
}

Expand Down

0 comments on commit bf22392

Please sign in to comment.