opendev/gerrit
Code Issues Proposed changes

Use AtomicUpdate to update account table

Browse Source 
Otherwise updates may fail if two requests for the same account are
executed in parallel.

Bug: Issue 5721
Change-Id: Ic1a8a17d19982ff7a487ebe8aefe6a9f29baca1f
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-03-10 08:36:58 +01:00
parent 7c5103310b
commit 0457988491
5 changed files with 99 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
View File

@@ -25,12 +25,13 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.DeleteActive.Input;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicBoolean;
@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
@Singleton
@@ -52,18 +53,34 @@ public class DeleteActive implements RestModifyView<AccountResource, Input> {
@Override
public Response<?> apply(AccountResource rsrc, Input input)
throws RestApiException, OrmException, IOException {
Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
if (!a.isActive()) {
throw new ResourceConflictException("account not active");
}
if (self.get() == rsrc.getUser()) {
throw new ResourceConflictException("cannot deactivate own account");
}
a.setActive(false);
dbProvider.get().accounts().update(Collections.singleton(a));
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
Account a =
dbProvider
.get()
.accounts()
.atomicUpdate(
rsrc.getUser().getAccountId(),
new AtomicUpdate<Account>() {
@Override
public Account update(Account a) {
if (!a.isActive()) {
alreadyInactive.set(true);
} else {
a.setActive(false);
}
return a;
}
});
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
if (alreadyInactive.get()) {
throw new ResourceConflictException("account not active");
}
byIdCache.evict(a.getId());
return Response.none();
}

27
gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
View File

@@ -22,12 +22,14 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.PutActive.Input;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicBoolean;
@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
@Singleton
@@ -46,16 +48,29 @@ public class PutActive implements RestModifyView<AccountResource, Input> {
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException {
Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
AtomicBoolean alreadyActive = new AtomicBoolean(false);
Account a =
dbProvider
.get()
.accounts()
.atomicUpdate(
rsrc.getUser().getAccountId(),
new AtomicUpdate<Account>() {
@Override
public Account update(Account a) {
if (a.isActive()) {
alreadyActive.set(true);
} else {
a.setActive(true);
}
return a;
}
});
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
if (a.isActive()) {
return Response.ok("");
}
a.setActive(true);
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Response.created("");
return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}

19
gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
View File

@@ -27,12 +27,12 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutName.Input;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
@Singleton
public class PutName implements RestModifyView<AccountResource, Input> {
@@ -77,12 +77,23 @@ public class PutName implements RestModifyView<AccountResource, Input> {
throw new MethodNotAllowedException("realm does not allow editing name");
}
Account a = dbProvider.get().accounts().get(user.getAccountId());
String newName = input.name;
Account a =
dbProvider
.get()
.accounts()
.atomicUpdate(
user.getAccountId(),
new AtomicUpdate<Account>() {
@Override
public Account update(Account a) {
a.setFullName(newName);
return a;
}
});
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
a.setFullName(input.name);
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Strings.isNullOrEmpty(a.getFullName())
? Response.<String>none()

27
gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
View File

@@ -23,12 +23,14 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutPreferred.Input;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.atomic.AtomicBoolean;
@Singleton
public class PutPreferred implements RestModifyView<AccountResource.Email, Input> {
@@ -56,16 +58,29 @@ public class PutPreferred implements RestModifyView<AccountResource.Email, Input
public Response<String> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, OrmException, IOException {
Account a = dbProvider.get().accounts().get(user.getAccountId());
AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
Account a =
dbProvider
.get()
.accounts()
.atomicUpdate(
user.getAccountId(),
new AtomicUpdate<Account>() {
@Override
public Account update(Account a) {
if (email.equals(a.getPreferredEmail())) {
alreadyPreferred.set(true);
} else {
a.setPreferredEmail(email);
}
return a;
}
});
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
if (email.equals(a.getPreferredEmail())) {
return Response.ok("");
}
a.setPreferredEmail(email);
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Response.created("");
return alreadyPreferred.get() ? Response.ok("") : Response.created("");
}
}

19
gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
View File

@@ -25,12 +25,12 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutStatus.Input;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
@Singleton
public class PutStatus implements RestModifyView<AccountResource, Input> {
@@ -70,12 +70,23 @@ public class PutStatus implements RestModifyView<AccountResource, Input> {
input = new Input();
}
Account a = dbProvider.get().accounts().get(user.getAccountId());
String newStatus = input.status;
Account a =
dbProvider
.get()
.accounts()
.atomicUpdate(
user.getAccountId(),
new AtomicUpdate<Account>() {
@Override
public Account update(Account a) {
a.setStatus(Strings.nullToEmpty(newStatus));
return a;
}
});
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
a.setStatus(Strings.nullToEmpty(input.status));
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Strings.isNullOrEmpty(a.getStatus()) ? Response.none() : Response.ok(a.getStatus());
}