Delete changes from the index when deleting whole draft changes

Change-Id: If76211aed5449740a3616598409dafc081523f60
This commit is contained in:
Dave Borowitz
2013-09-20 11:35:01 -07:00
parent 3e5bdf421c
commit 2bd0c24bc0
7 changed files with 91 additions and 16 deletions

View File

@@ -21,6 +21,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtjsonrpc.common.VoidResult; import com.google.gwtjsonrpc.common.VoidResult;
@@ -39,6 +40,7 @@ class DeleteDraftChange extends Handler<VoidResult> {
private final ReviewDb db; private final ReviewDb db;
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final ChangeIndexer indexer;
private final PatchSet.Id patchSetId; private final PatchSet.Id patchSetId;
@@ -47,11 +49,13 @@ class DeleteDraftChange extends Handler<VoidResult> {
final ChangeControl.Factory changeControlFactory, final ChangeControl.Factory changeControlFactory,
final GitRepositoryManager gitManager, final GitRepositoryManager gitManager,
final GitReferenceUpdated gitRefUpdated, final GitReferenceUpdated gitRefUpdated,
final ChangeIndexer indexer,
@Assisted final PatchSet.Id patchSetId) { @Assisted final PatchSet.Id patchSetId) {
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.db = db; this.db = db;
this.gitManager = gitManager; this.gitManager = gitManager;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.indexer = indexer;
this.patchSetId = patchSetId; this.patchSetId = patchSetId;
} }
@@ -65,7 +69,8 @@ class DeleteDraftChange extends Handler<VoidResult> {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
} }
ChangeUtil.deleteDraftChange(patchSetId, gitManager, gitRefUpdated, db); ChangeUtil.deleteDraftChange(patchSetId, gitManager, gitRefUpdated, db,
indexer);
return VoidResult.INSTANCE; return VoidResult.INSTANCE;
} }
} }

View File

@@ -35,6 +35,8 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidationException;
import com.google.gerrit.server.git.validators.CommitValidators; import com.google.gerrit.server.git.validators.CommitValidators;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.mail.CommitMessageEditedSender; import com.google.gerrit.server.mail.CommitMessageEditedSender;
import com.google.gerrit.server.mail.RevertedSender; import com.google.gerrit.server.mail.RevertedSender;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -392,19 +394,19 @@ public class ChangeUtil {
} }
} }
public static void deleteDraftChange(final PatchSet.Id patchSetId, public static void deleteDraftChange(PatchSet.Id patchSetId,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
final GitReferenceUpdated gitRefUpdated, final ReviewDb db) GitReferenceUpdated gitRefUpdated, ReviewDb db, ChangeIndexer indexer)
throws NoSuchChangeException, OrmException, IOException { throws NoSuchChangeException, OrmException, IOException {
final Change.Id changeId = patchSetId.getParentKey(); final Change.Id changeId = patchSetId.getParentKey();
deleteDraftChange(changeId, gitManager, gitRefUpdated, db); deleteDraftChange(changeId, gitManager, gitRefUpdated, db, indexer);
} }
public static void deleteDraftChange(final Change.Id changeId, public static void deleteDraftChange(Change.Id changeId,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
final GitReferenceUpdated gitRefUpdated, final ReviewDb db) GitReferenceUpdated gitRefUpdated, ReviewDb db, ChangeIndexer indexer)
throws NoSuchChangeException, OrmException, IOException { throws NoSuchChangeException, OrmException, IOException {
final Change change = db.changes().get(changeId); Change change = db.changes().get(changeId);
if (change == null || change.getStatus() != Change.Status.DRAFT) { if (change == null || change.getStatus() != Change.Status.DRAFT) {
throw new NoSuchChangeException(changeId); throw new NoSuchChangeException(changeId);
} }
@@ -418,6 +420,7 @@ public class ChangeUtil {
db.starredChanges().delete(db.starredChanges().byChange(changeId)); db.starredChanges().delete(db.starredChanges().byChange(changeId));
db.trackingIds().delete(db.trackingIds().byChange(changeId)); db.trackingIds().delete(db.trackingIds().byChange(changeId));
db.changes().delete(Collections.singleton(change)); db.changes().delete(Collections.singleton(change));
indexer.delete(change);
} }
public static void deleteOnlyDraftPatchSet(final PatchSet patch, public static void deleteOnlyDraftPatchSet(final PatchSet patch,

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.DeleteDraftChange.Input; import com.google.gerrit.server.change.DeleteDraftChange.Input;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -42,15 +43,18 @@ public class DeleteDraftChange implements
protected final Provider<ReviewDb> dbProvider; protected final Provider<ReviewDb> dbProvider;
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final ChangeIndexer indexer;
@Inject @Inject
public DeleteDraftChange(Provider<ReviewDb> dbProvider, public DeleteDraftChange(Provider<ReviewDb> dbProvider,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
GitReferenceUpdated gitRefUpdated, GitReferenceUpdated gitRefUpdated,
PatchSetInfoFactory patchSetInfoFactory) { PatchSetInfoFactory patchSetInfoFactory,
ChangeIndexer indexer) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.gitManager = gitManager; this.gitManager = gitManager;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.indexer = indexer;
} }
@Override @Override
@@ -67,7 +71,7 @@ public class DeleteDraftChange implements
try { try {
ChangeUtil.deleteDraftChange(rsrc.getChange().getId(), ChangeUtil.deleteDraftChange(rsrc.getChange().getId(),
gitManager, gitRefUpdated, dbProvider.get()); gitManager, gitRefUpdated, dbProvider.get(), indexer);
} catch (NoSuchChangeException e) { } catch (NoSuchChangeException e) {
throw new ResourceNotFoundException(e.getMessage()); throw new ResourceNotFoundException(e.getMessage());
} }

View File

@@ -28,6 +28,7 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.change.DeleteDraftPatchSet.Input; import com.google.gerrit.server.change.DeleteDraftPatchSet.Input;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
@@ -47,16 +48,19 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final ChangeIndexer indexer;
@Inject @Inject
public DeleteDraftPatchSet(Provider<ReviewDb> dbProvider, public DeleteDraftPatchSet(Provider<ReviewDb> dbProvider,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
GitReferenceUpdated gitRefUpdated, GitReferenceUpdated gitRefUpdated,
PatchSetInfoFactory patchSetInfoFactory) { PatchSetInfoFactory patchSetInfoFactory,
ChangeIndexer indexer) {
this.dbProvider = dbProvider; this.dbProvider = dbProvider;
this.gitManager = gitManager; this.gitManager = gitManager;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.indexer = indexer;
} }
@Override @Override
@@ -129,7 +133,7 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
throws OrmException, IOException, ResourceNotFoundException { throws OrmException, IOException, ResourceNotFoundException {
try { try {
ChangeUtil.deleteDraftChange(patchSetId, ChangeUtil.deleteDraftChange(patchSetId,
gitManager, gitRefUpdated, dbProvider.get()); gitManager, gitRefUpdated, dbProvider.get(), indexer);
} catch (NoSuchChangeException e) { } catch (NoSuchChangeException e) {
throw new ResourceNotFoundException(e.getMessage()); throw new ResourceNotFoundException(e.getMessage());
} }

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.index.ChangeIndexer;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
@@ -46,6 +47,7 @@ public class DeleteDraftPatchSet implements Callable<ReviewResult> {
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final GitReferenceUpdated gitRefUpdated; private final GitReferenceUpdated gitRefUpdated;
private final PatchSetInfoFactory patchSetInfoFactory; private final PatchSetInfoFactory patchSetInfoFactory;
private final ChangeIndexer indexer;
private final PatchSet.Id patchSetId; private final PatchSet.Id patchSetId;
@@ -53,12 +55,14 @@ public class DeleteDraftPatchSet implements Callable<ReviewResult> {
DeleteDraftPatchSet(ChangeControl.Factory changeControlFactory, DeleteDraftPatchSet(ChangeControl.Factory changeControlFactory,
ReviewDb db, GitRepositoryManager gitManager, ReviewDb db, GitRepositoryManager gitManager,
GitReferenceUpdated gitRefUpdated, PatchSetInfoFactory patchSetInfoFactory, GitReferenceUpdated gitRefUpdated, PatchSetInfoFactory patchSetInfoFactory,
ChangeIndexer indexer,
@Assisted final PatchSet.Id patchSetId) { @Assisted final PatchSet.Id patchSetId) {
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.db = db; this.db = db;
this.gitManager = gitManager; this.gitManager = gitManager;
this.gitRefUpdated = gitRefUpdated; this.gitRefUpdated = gitRefUpdated;
this.patchSetInfoFactory = patchSetInfoFactory; this.patchSetInfoFactory = patchSetInfoFactory;
this.indexer = indexer;
this.patchSetId = patchSetId; this.patchSetId = patchSetId;
} }
@@ -97,7 +101,8 @@ public class DeleteDraftPatchSet implements Callable<ReviewResult> {
List<PatchSet> restOfPatches = db.patchSets().byChange(changeId).toList(); List<PatchSet> restOfPatches = db.patchSets().byChange(changeId).toList();
if (restOfPatches.size() == 0) { if (restOfPatches.size() == 0) {
try { try {
ChangeUtil.deleteDraftChange(patchSetId, gitManager, gitRefUpdated, db); ChangeUtil.deleteDraftChange(patchSetId, gitManager, gitRefUpdated, db,
indexer);
result.setChangeId(null); result.setChangeId(null);
} catch (IOException e) { } catch (IOException e) {
result.addError(new ReviewResult.Error( result.addError(new ReviewResult.Error(

View File

@@ -50,6 +50,16 @@ public abstract class ChangeIndexer {
} }
}; };
} }
@Override
public Callable<Void> deleteTask(ChangeData cd) {
return new Callable<Void>() {
@Override
public Void call() {
return null;
}
};
}
}; };
private final ListeningScheduledExecutorService executor; private final ListeningScheduledExecutorService executor;
@@ -85,4 +95,32 @@ public abstract class ChangeIndexer {
* @return unstarted runnable to index the change. * @return unstarted runnable to index the change.
*/ */
public abstract Callable<Void> indexTask(ChangeData cd); public abstract Callable<Void> indexTask(ChangeData cd);
/**
* Start deleting a change.
*
* @param change change to delete.
* @return future for the deleting task.
*/
public ListenableFuture<?> delete(Change change) {
return delete(new ChangeData(change));
}
/**
* Start deleting a change.
*
* @param change change to delete.
* @return future for the deleting task.
*/
public ListenableFuture<?> delete(ChangeData cd) {
return executor.submit(deleteTask(cd));
}
/**
* Create a runnable to delete a change.
*
* @param cd change to delete.
* @return unstarted runnable to delete the change.
*/
public abstract Callable<Void> deleteTask(ChangeData cd);
} }

View File

@@ -30,6 +30,7 @@ import com.google.inject.util.Providers;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
/** /**
@@ -73,14 +74,21 @@ public class ChangeIndexerImpl extends ChangeIndexer {
@Override @Override
public Callable<Void> indexTask(ChangeData cd) { public Callable<Void> indexTask(ChangeData cd) {
return new Task(cd); return new Task(cd, false);
}
@Override
public Callable<Void> deleteTask(ChangeData cd) {
return new Task(cd, true);
} }
private class Task implements Callable<Void> { private class Task implements Callable<Void> {
private final ChangeData cd; private final ChangeData cd;
private final boolean delete;
private Task(ChangeData cd) { private Task(ChangeData cd, boolean delete) {
this.cd = cd; this.cd = cd;
this.delete = delete;
} }
@Override @Override
@@ -101,10 +109,10 @@ public class ChangeIndexerImpl extends ChangeIndexer {
}); });
if (indexes != null) { if (indexes != null) {
for (ChangeIndex i : indexes.getWriteIndexes()) { for (ChangeIndex i : indexes.getWriteIndexes()) {
i.replace(cd); // TODO(dborowitz): Parallelize these apply(i, cd); // TODO(dborowitz): Parallelize these
} }
} else { } else {
index.replace(cd); apply(index, cd);
} }
return null; return null;
} finally { } finally {
@@ -119,6 +127,14 @@ public class ChangeIndexerImpl extends ChangeIndexer {
} }
} }
private void apply(ChangeIndex i, ChangeData cd) throws IOException {
if (delete) {
i.delete(cd);
} else {
i.replace(cd);
}
}
@Override @Override
public String toString() { public String toString() {
return "index-change-" + cd.getId().get(); return "index-change-" + cd.getId().get();