Commit 4c9a8aa6 authored by Andrei Mihu's avatar Andrei Mihu
Browse files

Handle errors on group join operations when the user is already a member of the group.

parent 1871ba19

CHANGELOG.md

+1 −0
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ The format is based on [keep a changelog](http://keepachangelog.com) and this pr 
### Fixed

- Storage write batches now correctly abort when any query in the batch fails.

- Rank cache correctly calculates record expiry times.

- Return correct response to group join operations when the user is already a member of the group.



## [2.4.2] - 2019-03-25

### Added

server/console_user.go

+2 −2
Original line number Diff line number Diff line
@@ -222,7 +222,7 @@ func (s *ConsoleServer) ListUsers(ctx context.Context, in *console.ListUsersRequ 
func countUsers(ctx context.Context, logger *zap.Logger, db *sql.DB) int32 {

	var count sql.NullInt64

	if err := db.QueryRowContext(ctx, "SELECT reltuples::BIGINT FROM pg_class WHERE relname = 'users'").Scan(&count); err != nil {

		logger.Warn("Error counting storage objects.", zap.Error(err))

		logger.Warn("Error counting users.", zap.Error(err))

		if err == context.Canceled {

			// If the context was cancelled do not attempt the full count.

			return 0
@@ -235,7 +235,7 @@ func countUsers(ctx context.Context, logger *zap.Logger, db *sql.DB) int32 { 


	// If the fast count failed, returned NULL, or returned 0 try a full count.

	if err := db.QueryRowContext(ctx, "SELECT count(id) FROM users").Scan(&count); err != nil {

		logger.Warn("Error counting storage objects.", zap.Error(err))

		logger.Warn("Error counting users.", zap.Error(err))

	}

	return int32(count.Int64)

}

server/core_group.go

+8 −5
Original line number Diff line number Diff line
@@ -369,11 +369,10 @@ WHERE (id = $1) AND (disable_time = '1970-01-01 00:00:00 UTC')` 
	}



	if err = crdb.ExecuteInTx(ctx, tx, func() error {

		_, err = groupAddUser(ctx, db, tx, uuid.Must(uuid.FromString(group.Id)), userID, state)

		if err != nil {

		if _, err = groupAddUser(ctx, db, tx, uuid.Must(uuid.FromString(group.Id)), userID, state); err != nil {

			if e, ok := err.(*pq.Error); ok && e.Code == dbErrorUniqueViolation {

				logger.Info("Could not add user to group as relationship already exists.", zap.String("group_id", groupID.String()), zap.String("user_id", userID.String()))

				return nil // completed successfully

				return e

			}



			logger.Debug("Could not add user to group.", zap.String("group_id", groupID.String()), zap.String("user_id", userID.String()))
@@ -381,14 +380,18 @@ WHERE (id = $1) AND (disable_time = '1970-01-01 00:00:00 UTC')` 
		}



		query = "UPDATE groups SET edge_count = edge_count + 1, update_time = now() WHERE id = $1::UUID AND edge_count+1 <= max_count"

		_, err := tx.ExecContext(ctx, query, groupID)

		if err != nil {

		if _, err = tx.ExecContext(ctx, query, groupID); err != nil {

			logger.Debug("Could not update group edge_count.", zap.String("group_id", groupID.String()), zap.String("user_id", userID.String()))

			return err

		}



		return nil

	}); err != nil {

		if e, ok := err.(*pq.Error); ok && e.Code == dbErrorUniqueViolation {

			// No-op, user was already in group.

			return nil

		}



		logger.Error("Error joining group.", zap.Error(err))

		return err

	}