NoteDbUpdateManager: Default checkExpectedState to true

This was always the intention, as it's the safest approach.
Unfortunately, since this was dead code, flipping the flag found a bug
in the logic.

We don't want to check whether the expected state matches using the
ChainedReceiveCommands, which includes the results of any updates
we're about to apply--of course that won't match. Instead, reach in
and grab the underlying RepoRefCache. This continues avoiding an extra
read, since it uses the same cached ref value. Using a cached value
here should still be safe as the ReceiveCommand itself will fail if
the ref was updated in the meantime.

Change-Id: I8bc6b1706f27c31e2ca335910040fe083bf146e8
This commit is contained in:
Dave Borowitz
2016-06-20 16:00:18 -04:00
parent a519e8d195
commit ab598d6c8d
2 changed files with 7 additions and 3 deletions

View File

@@ -49,6 +49,10 @@ public class ChainedReceiveCommands implements RefCache {
this.refCache = checkNotNull(refCache);
}
public RepoRefCache getRepoRefCache() {
return refCache;
}
public boolean isEmpty() {
return commands.isEmpty();
}

View File

@@ -180,7 +180,7 @@ public class NoteDbUpdateManager {
private OpenRepo changeRepo;
private OpenRepo allUsersRepo;
private Map<Change.Id, StagedResult> staged;
private boolean checkExpectedState;
private boolean checkExpectedState = true;
@AssistedInject
NoteDbUpdateManager(GitRepositoryManager repoManager,
@@ -486,7 +486,7 @@ public class NoteDbUpdateManager {
continue;
}
if (!expectedState.isChangeUpToDate(changeRepo.cmds)) {
if (!expectedState.isChangeUpToDate(changeRepo.cmds.getRepoRefCache())) {
throw new OrmConcurrencyException(String.format(
"cannot apply NoteDb updates for change %s;"
+ " change meta ref does not match %s",
@@ -504,7 +504,7 @@ public class NoteDbUpdateManager {
Account.Id accountId = u.getAccountId();
if (!expectedState.areDraftsUpToDate(
allUsersRepo.cmds, accountId)) {
allUsersRepo.cmds.getRepoRefCache(), accountId)) {
throw new OrmConcurrencyException(String.format(
"cannot apply NoteDb updates for change %s;"
+ " draft ref for account %s does not match %s",