Add extension point for storing reviewed flags

At the moment the reviewed flags are stored in ReviewDb, but to fully
migrate to NoteDb they must be moved to a different storage. Different
options how to implement this storage were discussed on the Gerrit
mailing list [1,2]. Standalone Gerrit servers can benefit from a
simple storage in a local H2 database, while multi-master
installations must care to replicate this data between the master
nodes. To support both cases well this extension point makes the store
implementation pluggable. Gerrit itself implements the extension
point, but plugins can replace this implementation. For now the
implementation of this extension point in Gerrit just wraps the access
to the ReviewDb, but in a follow-up change another storage and a data
migration will be implemented.

[1] https://groups.google.com/forum/#!msg/repo-discuss/KhXKMKTJNMs/Tq3XaB8wCgAJ
[2] https://groups.google.com/d/msg/repo-discuss/KhXKMKTJNMs/hHoCa1_5CwAJ

Change-Id: Iaaacd9f0af977ccd505b5ad541f4a616afa214a4
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin 2016-06-14 11:50:58 +02:00
parent 764af4402c
commit da17bc33c2
11 changed files with 302 additions and 70 deletions

View File

@ -2121,6 +2121,31 @@ public class S3LargeFileRepository extends S3Repository {
} }
---- ----
[[account-patch-review-store]]
== AccountPatchReviewStore
The AccountPatchReviewStore is used to store reviewed flags on changes.
A reviewed flag is a tuple of (patch set ID, file, account ID) and
records whether the user has reviewed a file in a patch set. Each user
can easily have thousands of reviewed flags and the number of reviewed
flags is growing without bound. The store must be able handle this data
volume efficiently.
Gerrit implements this extension point, but plugins may bind another
implementation, e.g. one that supports multi-master.
----
DynamicItem.bind(binder(), AccountPatchReviewStore.class)
.to(MultiMasterAccountPatchReviewStore.class);
...
public class MultiMasterAccountPatchReviewStore
implements AccountPatchReviewStore {
...
}
----
[[documentation]] [[documentation]]
== Documentation == Documentation

View File

@ -50,6 +50,7 @@ import com.google.gerrit.pgm.util.SiteProgram;
import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.account.InternalAccountDirectory;
import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
import com.google.gerrit.server.change.AccountPatchReviewStoreImpl;
import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.change.ChangeCleanupRunner;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.AuthConfigModule;
@ -342,6 +343,7 @@ public class Daemon extends SiteProgram {
modules.add(new WorkQueue.Module()); modules.add(new WorkQueue.Module());
modules.add(new ChangeHookRunner.Module()); modules.add(new ChangeHookRunner.Module());
modules.add(new EventBroker.Module()); modules.add(new EventBroker.Module());
modules.add(new AccountPatchReviewStoreImpl.Module());
modules.add(new ReceiveCommitsExecutorModule()); modules.add(new ReceiveCommitsExecutorModule());
modules.add(new DiffExecutorModule()); modules.add(new DiffExecutorModule());
modules.add(new MimeUtil2Module()); modules.add(new MimeUtil2Module());

View File

@ -0,0 +1,93 @@
// Copyright (C) 2016 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gwtorm.server.OrmException;
import java.util.Collection;
/**
* Store for reviewed flags on changes.
*
* A reviewed flag is a tuple of (patch set ID, file, account ID) and records
* whether the user has reviewed a file in a patch set. Each user can easily
* have thousands of reviewed flags and the number of reviewed flags is growing
* without bound. The store must be able handle this data volume efficiently.
*
* For a multi-master setup the store must replicate the data between the
* masters.
*/
public interface AccountPatchReviewStore {
/**
* Marks the given file in the given patch set as reviewed by the given user.
*
* @param psId patch set ID
* @param accountId account ID of the user
* @param path file path
* @return {@code true} if the reviewed flag was updated, {@code false} if the
* reviewed flag was already set
* @throws OrmException thrown if updating the reviewed flag failed
*/
boolean markReviewed(PatchSet.Id psId, Account.Id accountId, String path)
throws OrmException;
/**
* Marks the given files in the given patch set as reviewed by the given user.
*
* @param psId patch set ID
* @param accountId account ID of the user
* @param paths file paths
* @throws OrmException thrown if updating the reviewed flag failed
*/
void markReviewed(PatchSet.Id psId, Account.Id accountId,
Collection<String> paths) throws OrmException;
/**
* Clears the reviewed flag for the given file in the given patch set for the
* given user.
*
* @param psId patch set ID
* @param accountId account ID of the user
* @param path file path
* @throws OrmException thrown if clearing the reviewed flag failed
*/
void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path)
throws OrmException;
/**
* Clears the reviewed flags for all files in the given patch set for all
* users.
*
* @param psId patch set ID
* @throws OrmException thrown if clearing the reviewed flags failed
*/
void clearReviewed(PatchSet.Id psId) throws OrmException;
/**
* Returns the paths of all files in the given patch set the have been
* reviewed by the given user.
*
* @param psId patch set ID
* @param accountId account ID of the user
* @return the paths of all files in the given patch set the have been
* reviewed by the given user
* @throws OrmException thrown if accessing the reviewed flags failed
*/
Collection<String> findReviewed(PatchSet.Id psId, Account.Id accountId)
throws OrmException;
}

View File

@ -0,0 +1,132 @@
// Copyright (C) 2016 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.change;
import com.google.common.base.Function;
import com.google.common.collect.Collections2;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountPatchReview;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.Collection;
import java.util.Collections;
import javax.inject.Singleton;
@Singleton
public class AccountPatchReviewStoreImpl implements AccountPatchReviewStore {
private final Provider<ReviewDb> dbProvider;
public static class Module extends AbstractModule {
@Override
protected void configure() {
DynamicItem.itemOf(binder(), AccountPatchReviewStore.class);
DynamicItem.bind(binder(), AccountPatchReviewStore.class)
.to(AccountPatchReviewStoreImpl.class);
}
}
@Inject
AccountPatchReviewStoreImpl(Provider<ReviewDb> dbProvider) {
this.dbProvider = dbProvider;
}
@Override
public boolean markReviewed(PatchSet.Id psId, Account.Id accountId,
String path) throws OrmException {
ReviewDb db = dbProvider.get();
AccountPatchReview apr = getExisting(db, psId, path, accountId);
if (apr != null) {
return false;
}
try {
db.accountPatchReviews().insert(Collections.singleton(
new AccountPatchReview(new Patch.Key(psId, path), accountId)));
return true;
} catch (OrmDuplicateKeyException e) {
// Ignored
return false;
}
}
@Override
public void markReviewed(final PatchSet.Id psId, final Account.Id accountId,
final Collection<String> paths) throws OrmException {
if (paths == null || paths.isEmpty()) {
return;
} else if (paths.size() == 1) {
markReviewed(psId, accountId, Iterables.getOnlyElement(paths));
return;
}
paths.removeAll(findReviewed(psId, accountId));
if (paths.isEmpty()) {
return;
}
dbProvider.get().accountPatchReviews().insert(Collections2.transform(paths,
new Function<String, AccountPatchReview>() {
@Override
public AccountPatchReview apply(String path) {
return new AccountPatchReview(new Patch.Key(psId, path), accountId);
}
}));
}
@Override
public void clearReviewed(PatchSet.Id psId, Account.Id accountId, String path)
throws OrmException {
ReviewDb db = dbProvider.get();
AccountPatchReview apr = getExisting(db, psId, path, accountId);
if (apr != null) {
db.accountPatchReviews().delete(Collections.singleton(apr));
}
}
@Override
public void clearReviewed(PatchSet.Id psId) throws OrmException {
dbProvider.get().accountPatchReviews()
.delete(dbProvider.get().accountPatchReviews().byPatchSet(psId));
}
@Override
public Collection<String> findReviewed(PatchSet.Id psId, Account.Id accountId)
throws OrmException {
return Collections2.transform(dbProvider.get().accountPatchReviews()
.byReviewer(accountId, psId).toList(),
new Function<AccountPatchReview, String>() {
@Override
public String apply(AccountPatchReview apr) {
return apr.getKey().getPatchKey().getFileName();
}
});
}
private static AccountPatchReview getExisting(ReviewDb db, PatchSet.Id psId,
String path, Account.Id accountId) throws OrmException {
AccountPatchReview.Key key =
new AccountPatchReview.Key(new Patch.Key(psId, path), accountId);
return db.accountPatchReviews().get(key);
}
}

View File

@ -33,6 +33,7 @@ import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.FixInput; import com.google.gerrit.extensions.api.changes.FixInput;
import com.google.gerrit.extensions.common.ProblemInfo; import com.google.gerrit.extensions.common.ProblemInfo;
import com.google.gerrit.extensions.common.ProblemInfo.Status; import com.google.gerrit.extensions.common.ProblemInfo.Status;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
@ -125,6 +126,7 @@ public class ConsistencyChecker {
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final ChangeUpdate.Factory changeUpdateFactory; private final ChangeUpdate.Factory changeUpdateFactory;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private FixInput fix; private FixInput fix;
private Change change; private Change change;
@ -151,7 +153,8 @@ public class ConsistencyChecker {
ChangeIndexer indexer, ChangeIndexer indexer,
ChangeControl.GenericFactory changeControlFactory, ChangeControl.GenericFactory changeControlFactory,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
ChangeUpdate.Factory changeUpdateFactory) { ChangeUpdate.Factory changeUpdateFactory,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore) {
this.db = db; this.db = db;
this.notesMigration = notesMigration; this.notesMigration = notesMigration;
this.repoManager = repoManager; this.repoManager = repoManager;
@ -165,6 +168,7 @@ public class ConsistencyChecker {
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.changeUpdateFactory = changeUpdateFactory; this.changeUpdateFactory = changeUpdateFactory;
this.accountPatchReviewStore = accountPatchReviewStore;
reset(); reset();
} }
@ -619,8 +623,7 @@ public class ConsistencyChecker {
// Delete dangling primary key references. Don't delete ChangeMessages, // Delete dangling primary key references. Don't delete ChangeMessages,
// which don't use patch sets as a primary key, and may provide useful // which don't use patch sets as a primary key, and may provide useful
// historical information. // historical information.
db.accountPatchReviews().delete( accountPatchReviewStore.get().clearReviewed(psId);
db.accountPatchReviews().byPatchSet(psId));
db.patchSetApprovals().delete( db.patchSetApprovals().delete(
db.patchSetApprovals().byPatchSet(psId)); db.patchSetApprovals().byPatchSet(psId));
db.patchComments().delete( db.patchComments().delete(

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -65,6 +66,7 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final StarredChangesUtil starredChangesUtil; private final StarredChangesUtil starredChangesUtil;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private final boolean allowDrafts; private final boolean allowDrafts;
private Change.Id id; private Change.Id id;
@ -72,9 +74,11 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
@Inject @Inject
DeleteDraftChangeOp(PatchSetUtil psUtil, DeleteDraftChangeOp(PatchSetUtil psUtil,
StarredChangesUtil starredChangesUtil, StarredChangesUtil starredChangesUtil,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
@GerritServerConfig Config cfg) { @GerritServerConfig Config cfg) {
this.psUtil = psUtil; this.psUtil = psUtil;
this.starredChangesUtil = starredChangesUtil; this.starredChangesUtil = starredChangesUtil;
this.accountPatchReviewStore = accountPatchReviewStore;
this.allowDrafts = allowDrafts(cfg); this.allowDrafts = allowDrafts(cfg);
} }
@ -105,8 +109,7 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
throw new ResourceConflictException("Cannot delete draft change " + id throw new ResourceConflictException("Cannot delete draft change " + id
+ ": patch set " + ps.getPatchSetId() + " is not a draft"); + ": patch set " + ps.getPatchSetId() + " is not a draft");
} }
db.accountPatchReviews().delete( accountPatchReviewStore.get().clearReviewed(ps.getId());
db.accountPatchReviews().byPatchSet(ps.getId()));
} }
// Only delete from ReviewDb here; deletion from NoteDb is handled in // Only delete from ReviewDb here; deletion from NoteDb is handled in

View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.MethodNotAllowedException; import com.google.gerrit.extensions.restapi.MethodNotAllowedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -59,6 +60,7 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final Provider<DeleteDraftChangeOp> deleteChangeOpProvider; private final Provider<DeleteDraftChangeOp> deleteChangeOpProvider;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
private final boolean allowDrafts; private final boolean allowDrafts;
@Inject @Inject
@ -67,12 +69,14 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
PatchSetUtil psUtil, PatchSetUtil psUtil,
Provider<DeleteDraftChangeOp> deleteChangeOpProvider, Provider<DeleteDraftChangeOp> deleteChangeOpProvider,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore,
@GerritServerConfig Config cfg) { @GerritServerConfig Config cfg) {
this.db = db; this.db = db;
this.updateFactory = updateFactory; this.updateFactory = updateFactory;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.psUtil = psUtil; this.psUtil = psUtil;
this.deleteChangeOpProvider = deleteChangeOpProvider; this.deleteChangeOpProvider = deleteChangeOpProvider;
this.accountPatchReviewStore = accountPatchReviewStore;
this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true); this.allowDrafts = cfg.getBoolean("change", "allowDrafts", true);
} }
@ -141,8 +145,8 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
// automatically filtered out when patch sets are deleted. // automatically filtered out when patch sets are deleted.
psUtil.delete(ctx.getDb(), ctx.getUpdate(patchSet.getId()), patchSet); psUtil.delete(ctx.getDb(), ctx.getUpdate(patchSet.getId()), patchSet);
accountPatchReviewStore.get().clearReviewed(psId);
ReviewDb db = DeleteDraftChangeOp.unwrap(ctx.getDb()); ReviewDb db = DeleteDraftChangeOp.unwrap(ctx.getDb());
db.accountPatchReviews().delete(db.accountPatchReviews().byPatchSet(psId));
db.changeMessages().delete(db.changeMessages().byPatchSet(psId)); db.changeMessages().delete(db.changeMessages().byPatchSet(psId));
db.patchComments().delete(db.patchComments().byPatchSet(psId)); db.patchComments().delete(db.patchComments().byPatchSet(psId));
db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(psId)); db.patchSetApprovals().delete(db.patchSetApprovals().byPatchSet(psId));

View File

@ -17,6 +17,7 @@ package com.google.gerrit.server.change;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Sets; import com.google.common.collect.Sets;
import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.FileInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
@ -28,8 +29,6 @@ import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountPatchReview;
import com.google.gerrit.reviewdb.client.Patch;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
@ -109,6 +108,7 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final PatchListCache patchListCache; private final PatchListCache patchListCache;
private final PatchSetUtil psUtil; private final PatchSetUtil psUtil;
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
@Inject @Inject
ListFiles(Provider<ReviewDb> db, ListFiles(Provider<ReviewDb> db,
@ -117,7 +117,8 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
Revisions revisions, Revisions revisions,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
PatchListCache patchListCache, PatchListCache patchListCache,
PatchSetUtil psUtil) { PatchSetUtil psUtil,
DynamicItem<AccountPatchReviewStore> accountPatchReviewStore) {
this.db = db; this.db = db;
this.self = self; this.self = self;
this.fileInfoJson = fileInfoJson; this.fileInfoJson = fileInfoJson;
@ -125,6 +126,7 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
this.gitManager = gitManager; this.gitManager = gitManager;
this.patchListCache = patchListCache; this.patchListCache = patchListCache;
this.psUtil = psUtil; this.psUtil = psUtil;
this.accountPatchReviewStore = accountPatchReviewStore;
} }
public ListFiles setReviewed(boolean r) { public ListFiles setReviewed(boolean r) {
@ -202,7 +204,7 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
} }
} }
private List<String> reviewed(RevisionResource resource) private Collection<String> reviewed(RevisionResource resource)
throws AuthException, OrmException { throws AuthException, OrmException {
CurrentUser user = self.get(); CurrentUser user = self.get();
if (!(user.isIdentifiedUser())) { if (!(user.isIdentifiedUser())) {
@ -210,11 +212,13 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
} }
Account.Id userId = user.getAccountId(); Account.Id userId = user.getAccountId();
List<String> r = scan(userId, resource.getPatchSet().getId()); Collection<String> r = accountPatchReviewStore.get()
.findReviewed(resource.getPatchSet().getId(), userId);
if (r.isEmpty() && 1 < resource.getPatchSet().getPatchSetId()) { if (r.isEmpty() && 1 < resource.getPatchSet().getPatchSetId()) {
for (PatchSet ps : reversePatchSets(resource)) { for (PatchSet ps : reversePatchSets(resource)) {
List<String> o = scan(userId, ps.getId()); Collection<String> o =
accountPatchReviewStore.get().findReviewed(ps.getId(), userId);
if (!o.isEmpty()) { if (!o.isEmpty()) {
try { try {
r = copy(Sets.newHashSet(o), ps.getId(), resource, userId); r = copy(Sets.newHashSet(o), ps.getId(), resource, userId);
@ -229,16 +233,6 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
return r; return r;
} }
private List<String> scan(Account.Id userId, PatchSet.Id psId)
throws OrmException {
List<String> r = new ArrayList<>();
for (AccountPatchReview w : db.get().accountPatchReviews()
.byReviewer(userId, psId)) {
r.add(w.getKey().getPatchKey().getFileName());
}
return r;
}
private List<PatchSet> reversePatchSets(RevisionResource resource) private List<PatchSet> reversePatchSets(RevisionResource resource)
throws OrmException { throws OrmException {
Collection<PatchSet> patchSets = Collection<PatchSet> patchSets =
@ -266,7 +260,6 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
resource.getPatchSet()); resource.getPatchSet());
int sz = paths.size(); int sz = paths.size();
List<AccountPatchReview> inserts = Lists.newArrayListWithCapacity(sz);
List<String> pathList = Lists.newArrayListWithCapacity(sz); List<String> pathList = Lists.newArrayListWithCapacity(sz);
tw.setFilter(PathFilterGroup.createFromStrings(paths)); tw.setFilter(PathFilterGroup.createFromStrings(paths));
@ -291,11 +284,6 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
&& paths.contains(path)) { && paths.contains(path)) {
// File exists in previously reviewed oldList and in curList. // File exists in previously reviewed oldList and in curList.
// File content is identical. // File content is identical.
inserts.add(new AccountPatchReview(
new Patch.Key(
resource.getPatchSet().getId(),
path),
userId));
pathList.add(path); pathList.add(path);
} else if (op >= 0 && cp >= 0 } else if (op >= 0 && cp >= 0
&& tw.getRawMode(o) == 0 && tw.getRawMode(c) == 0 && tw.getRawMode(o) == 0 && tw.getRawMode(c) == 0
@ -305,15 +293,11 @@ public class Files implements ChildCollection<RevisionResource, FileResource> {
// File was deleted in previously reviewed oldList and curList. // File was deleted in previously reviewed oldList and curList.
// File exists in ancestor of oldList and curList. // File exists in ancestor of oldList and curList.
// File content is identical in ancestors. // File content is identical in ancestors.
inserts.add(new AccountPatchReview(
new Patch.Key(
resource.getPatchSet().getId(),
path),
userId));
pathList.add(path); pathList.add(path);
} }
} }
db.get().accountPatchReviews().insert(inserts); accountPatchReviewStore.get()
.markReviewed(resource.getPatchSet().getId(), userId, pathList);
return pathList; return pathList;
} }
} }

View File

@ -14,44 +14,33 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.AccountPatchReview;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmDuplicateKeyException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.util.Collections;
public class Reviewed { public class Reviewed {
public static class Input { public static class Input {
} }
@Singleton @Singleton
public static class PutReviewed implements RestModifyView<FileResource, Input> { public static class PutReviewed
private final Provider<ReviewDb> dbProvider; implements RestModifyView<FileResource, Input> {
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
@Inject @Inject
PutReviewed(Provider<ReviewDb> dbProvider) { PutReviewed(DynamicItem<AccountPatchReviewStore> accountPatchReviewStore) {
this.dbProvider = dbProvider; this.accountPatchReviewStore = accountPatchReviewStore;
} }
@Override @Override
public Response<String> apply(FileResource resource, Input input) public Response<String> apply(FileResource resource, Input input)
throws OrmException { throws OrmException {
ReviewDb db = dbProvider.get(); if (accountPatchReviewStore.get().markReviewed(
AccountPatchReview apr = getExisting(db, resource); resource.getPatchKey().getParentKey(), resource.getAccountId(),
if (apr == null) { resource.getPatchKey().getFileName())) {
try {
db.accountPatchReviews().insert(
Collections.singleton(new AccountPatchReview(resource.getPatchKey(),
resource.getAccountId())));
} catch (OrmDuplicateKeyException e) {
return Response.ok("");
}
return Response.created(""); return Response.created("");
} }
return Response.ok(""); return Response.ok("");
@ -59,33 +48,26 @@ public class Reviewed {
} }
@Singleton @Singleton
public static class DeleteReviewed implements RestModifyView<FileResource, Input> { public static class DeleteReviewed
private final Provider<ReviewDb> dbProvider; implements RestModifyView<FileResource, Input> {
private final DynamicItem<AccountPatchReviewStore> accountPatchReviewStore;
@Inject @Inject
DeleteReviewed(Provider<ReviewDb> dbProvider) { DeleteReviewed(
this.dbProvider = dbProvider; DynamicItem<AccountPatchReviewStore> accountPatchReviewStore) {
this.accountPatchReviewStore = accountPatchReviewStore;
} }
@Override @Override
public Response<?> apply(FileResource resource, Input input) public Response<?> apply(FileResource resource, Input input)
throws OrmException { throws OrmException {
ReviewDb db = dbProvider.get(); accountPatchReviewStore.get().clearReviewed(
AccountPatchReview apr = getExisting(db, resource); resource.getPatchKey().getParentKey(), resource.getAccountId(),
if (apr != null) { resource.getPatchKey().getFileName());
db.accountPatchReviews().delete(Collections.singleton(apr));
}
return Response.none(); return Response.none();
} }
} }
private static AccountPatchReview getExisting(ReviewDb db,
FileResource resource) throws OrmException {
AccountPatchReview.Key key = new AccountPatchReview.Key(
resource.getPatchKey(), resource.getAccountId());
return db.accountPatchReviews().get(key);
}
private Reviewed() { private Reviewed() {
} }
} }

View File

@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent; import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.GerritPersonIdentProvider; import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
import com.google.gerrit.server.change.AccountPatchReviewStoreImpl;
import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllProjectsNameProvider; import com.google.gerrit.server.config.AllProjectsNameProvider;
import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.config.AllUsersName;
@ -198,6 +199,7 @@ public class InMemoryModule extends FactoryModule {
install(new FakeEmailSender.Module()); install(new FakeEmailSender.Module());
install(new SignedTokenEmailTokenVerifier.Module()); install(new SignedTokenEmailTokenVerifier.Module());
install(new GpgModule(cfg)); install(new GpgModule(cfg));
install(new AccountPatchReviewStoreImpl.Module());
IndexType indexType = null; IndexType indexType = null;
try { try {

View File

@ -32,6 +32,7 @@ import com.google.gerrit.metrics.dropwizard.DropWizardMetricMaker;
import com.google.gerrit.reviewdb.client.AuthType; import com.google.gerrit.reviewdb.client.AuthType;
import com.google.gerrit.server.account.InternalAccountDirectory; import com.google.gerrit.server.account.InternalAccountDirectory;
import com.google.gerrit.server.cache.h2.DefaultCacheFactory; import com.google.gerrit.server.cache.h2.DefaultCacheFactory;
import com.google.gerrit.server.change.AccountPatchReviewStoreImpl;
import com.google.gerrit.server.change.ChangeCleanupRunner; import com.google.gerrit.server.change.ChangeCleanupRunner;
import com.google.gerrit.server.config.AuthConfig; import com.google.gerrit.server.config.AuthConfig;
import com.google.gerrit.server.config.AuthConfigModule; import com.google.gerrit.server.config.AuthConfigModule;
@ -295,6 +296,7 @@ public class WebAppInitializer extends GuiceServletContextListener
final List<Module> modules = new ArrayList<>(); final List<Module> modules = new ArrayList<>();
modules.add(new DropWizardMetricMaker.RestModule()); modules.add(new DropWizardMetricMaker.RestModule());
modules.add(new EventBroker.Module()); modules.add(new EventBroker.Module());
modules.add(new AccountPatchReviewStoreImpl.Module());
modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class)); modules.add(cfgInjector.getInstance(GitRepositoryManagerModule.class));
modules.add(new ChangeHookRunner.Module()); modules.add(new ChangeHookRunner.Module());
modules.add(new ReceiveCommitsExecutorModule()); modules.add(new ReceiveCommitsExecutorModule());