Move uniqueness constraints on user to service level

While it originally thought out as a business level constraint it can be
enforced to more effect on the service level. As we know not only have
exact matches but also make sure that emails no matter in what spelling
won't be duplicated.

core/user.go

@@ -34,14 +34,6 @@ func UserCreate(
 			return nil, err
 		}
 
-		if err := userConstrainEmail(users, currentApp, u.Email); err != nil {
-			return nil, err
-		}
-
-		if err := userConstrainUsername(users, currentApp, u.Username); err != nil {
-			return nil, err
-		}
-
 		if err := u.Validate(); err != nil {
 			return nil, wrapError(ErrInvalidEntity, "%s", err)
 		}
@@ -436,24 +428,10 @@ func UserUpdate(
 			new.Password = old.Password
 		}
 
-		if old.Email != new.Email {
-			err := userConstrainEmail(users, currentApp, new.Email)
-			if err != nil {
-				return nil, err
-			}
-		}
-
 		if new.Private == nil {
 			new.Private = old.Private
 		}
 
-		if old.Username != new.Username {
-			err := userConstrainUsername(users, currentApp, new.Username)
-			if err != nil {
-				return nil, err
-			}
-		}
-
 		u, err := users.Put(currentApp.Namespace(), new)
 		if err != nil {
 			return nil, err
@@ -756,30 +734,6 @@ func passwordSecure(pw string) (string, error) {
 	return base64.StdEncoding.EncodeToString([]byte(enc)), nil
 }
 
-func userConstrainEmail(
-	users user.Service,
-	currentApp *app.App,
-	email string,
-) error {
-	if email != "" {
-		us, err := users.Query(currentApp.Namespace(), user.QueryOptions{
-			Enabled: &defaultEnabled,
-			Emails: []string{
-				email,
-			},
-		})
-		if err != nil {
-			return err
-		}
-
-		if len(us) > 0 {
-			return wrapError(ErrInvalidEntity, "email in use")
-		}
-	}
-
-	return nil
-}
-
 func userConstrainPrivate(origin Origin, private *user.Private) error {
 	if !origin.IsBackend() && private != nil {
 		return wrapError(
@@ -790,27 +744,3 @@ func userConstrainPrivate(origin Origin, private *user.Private) error {
 
 	return nil
 }
-
-func userConstrainUsername(
-	users user.Service,
-	currentApp *app.App,
-	username string,
-) error {
-	if username != "" {
-		us, err := users.Query(currentApp.Namespace(), user.QueryOptions{
-			Enabled: &defaultEnabled,
-			Usernames: []string{
-				username,
-			},
-		})
-		if err != nil {
-			return err
-		}
-
-		if len(us) > 0 {
-			return wrapError(ErrInvalidEntity, "username in use")
-		}
-	}
-
-	return nil
-}

platform/pg/pg.go

@@ -15,16 +15,24 @@ const MetaNamespace = "tg"
 // TimeFormat can be used to extract and store time in a reproducible way.
 const TimeFormat = "2006-01-02 15:04:05.000000 UTC"
 
+// URLTest can be used for consistent local testing.
 const URLTest = "postgres://%[email protected]:5432/tapglue_test?sslmode=disable&connect_timeout=5"
 
 const (
+	codeDuplicateKeyViolation = "23505"
+	codeRelationNotFound      = "42P01"
+
 	fmtClause = "\nAND "
 	fmtWHERE  = "WHERE\n%s"
 )
 
 // ErrRelationNotFound is returned as equivalent to the Postgres error.
 var ErrRelationNotFound = errors.New("relation not found")
 
+// ErrNotUnique indicates that the attempted update violates a unique constrain
+// on a table.
+var ErrNotUnique = errors.New("entity not unique")
+
 // To ensure idempotence we want to create the index only if it doesn't exist,
 // while this feature is about to hit Postgres in 9.5 it is not yet available.
 // We fallback to a conditional create taken from:
@@ -53,6 +61,11 @@ func GuardIndex(namespace, index, query string) string {
 	)
 }
 
+// IsNotUnique indicates if err is ErrNotUnique.
+func IsNotUnique(err error) bool {
+	return err == ErrNotUnique
+}
+
 // IsRelationNotFound indicates if err is ErrRelationNotFound.
 func IsRelationNotFound(err error) bool {
 	return err == ErrRelationNotFound
@@ -61,8 +74,13 @@ func IsRelationNotFound(err error) bool {
 // WrapError check the given error if it indicates that the relation wasn't
 // present, otherwise returns the original error.
 func WrapError(err error) error {
-	if err, ok := err.(*pq.Error); ok && err.Code == "42P01" {
-		return ErrRelationNotFound
+	if err, ok := err.(*pq.Error); ok {
+		switch err.Code {
+		case codeDuplicateKeyViolation:
+			return ErrNotUnique
+		case codeRelationNotFound:
+			return ErrRelationNotFound
+		}
 	}
 
 	return err

platform/pg/setup.sql

@@ -1,5 +1,6 @@
-CREATE EXTENSION fuzzystrmatch;
-CREATE EXTENSION postgis;
-CREATE EXTENSION postgis_topology;
-CREATE EXTENSION postgis_tiger_geocoder;
-CREATE EXTENSION pg_trgm;
+CREATE EXTENSION IF NOT EXISTS citext;
+CREATE EXTENSION IF NOT EXISTS fuzzystrmatch;
+CREATE EXTENSION IF NOT EXISTS postgis;
+CREATE EXTENSION IF NOT EXISTS postgis_topology;
+CREATE EXTENSION IF NOT EXISTS postgis_tiger_geocoder;
+CREATE EXTENSION IF NOT EXISTS pg_trgm;

service/user/error.go

@@ -11,6 +11,7 @@ const errFmt = "%s: %s"
 var (
 	ErrInvalidUser = errors.New("invalid user")
 	ErrNotFound    = errors.New("user not found")
+	ErrNotUnique   = errors.New("user information are not unique")
 )
 
 // Error wraps common User errors.
@@ -33,6 +34,10 @@ func IsNotFound(err error) bool {
 	return unwrapError(err) == ErrNotFound
 }
 
+func IsNotUnique(err error) bool {
+	return unwrapError(err) == ErrNotUnique
+}
+
 func unwrapError(err error) error {
 	switch e := err.(type) {
 	case *Error:

service/user/helper_test.go

@@ -239,6 +239,44 @@ func testServicePut(t *testing.T, p prepareFunc) {
 	}
 }
 
+func testServicePutEmailUnique(t *testing.T, p prepareFunc) {
+	var (
+		namespace = "service_put_email"
+		service   = p(t, namespace)
+		user      = testUser()
+		lowerCase = "[email protected]"
+		mixedCase = "[email protected]"
+	)
+
+	user.Email = lowerCase
+
+	_, err := service.Put(namespace, user)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	us, err := service.Query(namespace, QueryOptions{
+		Emails: []string{
+			mixedCase,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if have, want := len(us), 1; have != want {
+		t.Errorf("have %v, want %v", have, want)
+	}
+
+	second := testUser()
+	second.Email = mixedCase
+
+	_, err = service.Put(namespace, second)
+	if have, want := err, ErrNotUnique; !IsNotUnique(err) {
+		t.Errorf("have %v, want %v\n%#v", have, want, err)
+	}
+}
+
 func testServicePutLastRead(t *testing.T, p prepareFunc) {
 	var (
 		namespace = "service_put_last_read"
@@ -279,6 +317,44 @@ func testServicePutLastRead(t *testing.T, p prepareFunc) {
 	}
 }
 
+func testServicePutUsernameUnique(t *testing.T, p prepareFunc) {
+	var (
+		namespace = "service_put_email"
+		service   = p(t, namespace)
+		user      = testUser()
+		lowerCase = "xla1234"
+		mixedCase = "XlA1234"
+	)
+
+	user.Username = lowerCase
+
+	_, err := service.Put(namespace, user)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	us, err := service.Query(namespace, QueryOptions{
+		Usernames: []string{
+			mixedCase,
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if have, want := len(us), 1; have != want {
+		t.Errorf("have %v, want %v", have, want)
+	}
+
+	second := testUser()
+	second.Username = mixedCase
+
+	_, err = service.Put(namespace, second)
+	if have, want := err, ErrNotUnique; !IsNotUnique(err) {
+		t.Errorf("have %v, want %v\n%#v", have, want, err)
+	}
+}
+
 func testServiceQuery(t *testing.T, p prepareFunc) {
 	var (
 		customID  = generate.RandomString(12)
@@ -443,6 +519,7 @@ func testUser() *User {
 		),
 		Enabled:  true,
 		Password: generate.RandomString(8),
+		Username: generate.RandomString(8),
 	}
 }
 

service/user/postgres.go

@@ -35,11 +35,11 @@ const (
 	pgClauseBefore    = `(json_data->>'id')::BIGINT > ?`
 	pgClauseCustomIDs = `(json_data->>'custom_id')::TEXT IN (?)`
 	pgClauseDeleted   = `(json_data->>'deleted')::BOOL = ?::BOOL`
-	pgClauseEmail     = `(json_data->>'email')::TEXT IN (?)`
+	pgClauseEmail     = `(json_data->>'email')::CITEXT IN (?)`
 	pgClauseEnabled   = `(json_data->>'enabled')::BOOL = ?::BOOL`
 	pgClauseIDs       = `(json_data->>'id')::BIGINT IN (?)`
 	pgClauseSocialIDs = `(json_data->'social_ids'->>'%s')::TEXT IN (?)`
-	pgClauseUsernames = `(json_data->>'user_name')::TEXT IN (?)`
+	pgClauseUsernames = `(json_data->>'user_name')::CITEXT IN (?)`
 
 	pgClauseSearchEmail     = `(json_data->>'email')::TEXT ILIKE '%%%s%%'`
 	pgClauseSearchFirstname = `(json_data->>'first_name')::TEXT ILIKE '%%%s%%'`
@@ -64,10 +64,10 @@ const (
 	pgDropTable = `DROP TABLE IF EXISTS %s.users`
 
 	pgIndexEmail = `
-		CREATE INDEX
+		CREATE UNIQUE INDEX
 			%s
 		ON
-			%s.users(((json_data->>'email')::TEXT))
+			%s.users(((json_data->>'email')::CITEXT))
 		WHERE
 			(json_data->>'enabled')::BOOL = true`
 	pgIndexID = `
@@ -92,10 +92,10 @@ const (
 		WHERE
 			(json_data->>'enabled')::BOOL = true`
 	pgIndexUsername = `
-		CREATE INDEX
+		CREATE UNIQUE INDEX
 			%s
 		ON
-			%s.users(((json_data->>'user_name')::TEXT))
+			%s.users(((json_data->>'user_name')::CITEXT))
 		WHERE
 			(json_data->>'enabled')::BOOL = true`
 )
@@ -182,6 +182,10 @@ func (s *pgService) Put(ns string, user *User) (*User, error) {
 
 			_, err = s.db.Exec(wrapNamespace(query, ns), params...)
 		}
+
+		if pg.IsNotUnique(pg.WrapError(err)) {
+			return nil, ErrNotUnique
+		}
 	}
 
 	return user, err

service/user/postgres_test.go

@@ -22,10 +22,18 @@ func TestPostgresPut(t *testing.T) {
 	testServicePut(t, preparePostgres)
 }
 
+func TestPostgresPutEmailUnique(t *testing.T) {
+	testServicePutEmailUnique(t, preparePostgres)
+}
+
 func TestPostgresPutLastRead(t *testing.T) {
 	testServicePutLastRead(t, preparePostgres)
 }
 
+func TestPostgresPutUsernameUnique(t *testing.T) {
+	testServicePutUsernameUnique(t, preparePostgres)
+}
+
 func TestPostgresQuery(t *testing.T) {
 	testServiceQuery(t, preparePostgres)
 }