Do atomic account updates through AccountsUpdate class
The AccountsUpdate class should become the single class that updates
accounts. At the moment it always updates the accounts in ReviewDb, but
in future it will update accounts in NoteDb.
Change-Id: Iebde73b60a6c91c2acad83f3123c22ce2376c7ed
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
index 0f109f2..e40fd2c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountsUpdate.java
@@ -31,6 +31,7 @@
import com.google.inject.Singleton;
import java.io.IOException;
import java.sql.Timestamp;
+import java.util.function.Consumer;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
@@ -150,6 +151,26 @@
db.accounts().update(ImmutableSet.of(account));
}
+ /**
+ * Gets the account and updates it atomically.
+ *
+ * @param db ReviewDb
+ * @param accountId ID of the account
+ * @param consumer consumer to update the account, only invoked if the account exists
+ * @return the updated account, {@code null} if the account doesn't exist
+ * @throws OrmException if updating the account fails
+ */
+ public Account atomicUpdate(ReviewDb db, Account.Id accountId, Consumer<Account> consumer)
+ throws OrmException {
+ return db.accounts()
+ .atomicUpdate(
+ accountId,
+ a -> {
+ consumer.accept(a);
+ return a;
+ });
+ }
+
/** Deletes the account. */
public void delete(ReviewDb db, Account account) throws OrmException, IOException {
db.accounts().delete(ImmutableSet.of(account));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
index d013120..26893e2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
@@ -25,7 +25,6 @@
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;
@@ -39,13 +38,18 @@
public static class Input {}
private final Provider<ReviewDb> dbProvider;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountCache byIdCache;
private final Provider<IdentifiedUser> self;
@Inject
DeleteActive(
- Provider<ReviewDb> dbProvider, AccountCache byIdCache, Provider<IdentifiedUser> self) {
+ Provider<ReviewDb> dbProvider,
+ AccountsUpdate.Server accountsUpdate,
+ AccountCache byIdCache,
+ Provider<IdentifiedUser> self) {
this.dbProvider = dbProvider;
+ this.accountsUpdate = accountsUpdate;
this.byIdCache = byIdCache;
this.self = self;
}
@@ -58,30 +62,26 @@
}
AtomicBoolean alreadyInactive = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
rsrc.getUser().getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- if (!a.isActive()) {
- alreadyInactive.set(true);
- } else {
- a.setActive(false);
- }
- return a;
+ a -> {
+ if (!a.isActive()) {
+ alreadyInactive.set(true);
+ } else {
+ a.setActive(false);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
if (alreadyInactive.get()) {
throw new ResourceConflictException("account not active");
}
- byIdCache.evict(a.getId());
+ byIdCache.evict(account.getId());
return Response.none();
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
index 4c525c2..2eb0176 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
@@ -22,7 +22,6 @@
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;
@@ -36,11 +35,14 @@
public static class Input {}
private final Provider<ReviewDb> dbProvider;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountCache byIdCache;
@Inject
- PutActive(Provider<ReviewDb> dbProvider, AccountCache byIdCache) {
+ PutActive(
+ Provider<ReviewDb> dbProvider, AccountsUpdate.Server accountsUpdate, AccountCache byIdCache) {
this.dbProvider = dbProvider;
+ this.accountsUpdate = accountsUpdate;
this.byIdCache = byIdCache;
}
@@ -48,27 +50,23 @@
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException {
AtomicBoolean alreadyActive = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
rsrc.getUser().getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- if (a.isActive()) {
- alreadyActive.set(true);
- } else {
- a.setActive(true);
- }
- return a;
+ a -> {
+ if (a.isActive()) {
+ alreadyActive.set(true);
+ } else {
+ a.setActive(true);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
+ byIdCache.evict(account.getId());
return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
index 7a2868e..e4aa232 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
@@ -30,7 +30,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -47,6 +46,7 @@
private final Realm realm;
private final PermissionBackend permissionBackend;
private final Provider<ReviewDb> dbProvider;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountCache byIdCache;
@Inject
@@ -55,11 +55,13 @@
Realm realm,
PermissionBackend permissionBackend,
Provider<ReviewDb> dbProvider,
+ AccountsUpdate.Server accountsUpdate,
AccountCache byIdCache) {
this.self = self;
this.realm = realm;
this.permissionBackend = permissionBackend;
this.dbProvider = dbProvider;
+ this.accountsUpdate = accountsUpdate;
this.byIdCache = byIdCache;
}
@@ -84,25 +86,16 @@
}
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) {
+ Account account =
+ accountsUpdate
+ .create()
+ .atomicUpdate(dbProvider.get(), user.getAccountId(), a -> a.setFullName(newName));
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
- return Strings.isNullOrEmpty(a.getFullName())
- ? Response.<String>none()
- : Response.ok(a.getFullName());
+ byIdCache.evict(account.getId());
+ return Strings.isNullOrEmpty(account.getFullName())
+ ? Response.none()
+ : Response.ok(account.getFullName());
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
index 4941cc8..cd562b6 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
@@ -26,7 +26,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -41,6 +40,7 @@
private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountCache byIdCache;
@Inject
@@ -48,10 +48,12 @@
Provider<CurrentUser> self,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
+ AccountsUpdate.Server accountsUpdate,
AccountCache byIdCache) {
this.self = self;
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
+ this.accountsUpdate = accountsUpdate;
this.byIdCache = byIdCache;
}
@@ -68,27 +70,23 @@
public Response<String> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, OrmException, IOException {
AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
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;
+ a -> {
+ if (email.equals(a.getPreferredEmail())) {
+ alreadyPreferred.set(true);
+ } else {
+ a.setPreferredEmail(email);
}
});
- if (a == null) {
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
+ byIdCache.evict(account.getId());
return alreadyPreferred.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
index 73a720b..c3ffd4c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
@@ -28,7 +28,6 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -50,6 +49,7 @@
private final Provider<CurrentUser> self;
private final Provider<ReviewDb> dbProvider;
private final PermissionBackend permissionBackend;
+ private final AccountsUpdate.Server accountsUpdate;
private final AccountCache byIdCache;
@Inject
@@ -57,10 +57,12 @@
Provider<CurrentUser> self,
Provider<ReviewDb> dbProvider,
PermissionBackend permissionBackend,
+ AccountsUpdate.Server accountsUpdate,
AccountCache byIdCache) {
this.self = self;
this.dbProvider = dbProvider;
this.permissionBackend = permissionBackend;
+ this.accountsUpdate = accountsUpdate;
this.byIdCache = byIdCache;
}
@@ -81,23 +83,19 @@
}
String newStatus = input.status;
- Account a =
- dbProvider
- .get()
- .accounts()
+ Account account =
+ accountsUpdate
+ .create()
.atomicUpdate(
+ dbProvider.get(),
user.getAccountId(),
- new AtomicUpdate<Account>() {
- @Override
- public Account update(Account a) {
- a.setStatus(Strings.nullToEmpty(newStatus));
- return a;
- }
- });
- if (a == null) {
+ a -> a.setStatus(Strings.nullToEmpty(newStatus)));
+ if (account == null) {
throw new ResourceNotFoundException("account not found");
}
- byIdCache.evict(a.getId());
- return Strings.isNullOrEmpty(a.getStatus()) ? Response.none() : Response.ok(a.getStatus());
+ byIdCache.evict(account.getId());
+ return Strings.isNullOrEmpty(account.getStatus())
+ ? Response.none()
+ : Response.ok(account.getStatus());
}
}
