Merge pull request #19 from tapglue/user-uniqueness

Move uniqueness constraints on user to service level

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)
 }