Delete a change from the index when it is not in the DB
If for some reason the secondary index is out of date, i.e. the change was deleted from the database but wasn't deleted from the secondary index, it was impossible to re-index (remove) that change. Add logic to automatically remove the change from the secondary index if this change is requested through the Change REST API endpoint and doesn't exist in the database. If a user click on search result from a stale change, he will a get a 404 page and the change will be removed from the index. To fix the problem without opening the change page, run a command like: curl --user <user name> -X POST http://<host>:<port>/a/changes/<change id>/index Issue: 2996 Change-Id: I1db5373e31585e99c5f45e05274d86d69b4f24e6
This commit is contained in:
parent
04ef5f717d
commit
903be04b02
@ -322,6 +322,19 @@ public class LuceneChangeIndex implements ChangeIndex {
|
||||
}
|
||||
}
|
||||
|
||||
@SuppressWarnings("unchecked")
|
||||
@Override
|
||||
public void delete(int id) throws IOException {
|
||||
Term idTerm = QueryBuilder.idTerm(id);
|
||||
try {
|
||||
Futures.allAsList(
|
||||
openIndex.delete(idTerm),
|
||||
closedIndex.delete(idTerm)).get();
|
||||
} catch (ExecutionException | InterruptedException e) {
|
||||
throw new IOException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void deleteAll() throws IOException {
|
||||
openIndex.deleteAll();
|
||||
|
@ -55,6 +55,10 @@ public class QueryBuilder {
|
||||
return intTerm(ID_FIELD, cd.getId().get());
|
||||
}
|
||||
|
||||
public static Term idTerm(int id) {
|
||||
return intTerm(ID_FIELD, id);
|
||||
}
|
||||
|
||||
private final Schema<ChangeData> schema;
|
||||
private final org.apache.lucene.util.QueryBuilder queryBuilder;
|
||||
|
||||
|
@ -15,6 +15,7 @@
|
||||
package com.google.gerrit.server.change;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.primitives.Ints;
|
||||
import com.google.gerrit.extensions.registration.DynamicMap;
|
||||
import com.google.gerrit.extensions.restapi.IdString;
|
||||
import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
|
||||
@ -23,7 +24,9 @@ import com.google.gerrit.extensions.restapi.RestView;
|
||||
import com.google.gerrit.extensions.restapi.TopLevelResource;
|
||||
import com.google.gerrit.reviewdb.client.Change;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.ChangeUtil;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
import com.google.gerrit.server.index.ChangeIndexer;
|
||||
import com.google.gerrit.server.project.ChangeControl;
|
||||
import com.google.gerrit.server.project.NoSuchChangeException;
|
||||
import com.google.gerrit.server.query.change.QueryChanges;
|
||||
@ -31,6 +34,7 @@ import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
import com.google.inject.Provider;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.util.Collections;
|
||||
import java.util.List;
|
||||
|
||||
@ -41,6 +45,7 @@ public class ChangesCollection implements
|
||||
private final ChangeControl.GenericFactory changeControlFactory;
|
||||
private final Provider<QueryChanges> queryFactory;
|
||||
private final DynamicMap<RestView<ChangeResource>> views;
|
||||
private final ChangeIndexer changeIndexer;
|
||||
|
||||
@Inject
|
||||
ChangesCollection(
|
||||
@ -48,12 +53,15 @@ public class ChangesCollection implements
|
||||
Provider<CurrentUser> user,
|
||||
ChangeControl.GenericFactory changeControlFactory,
|
||||
Provider<QueryChanges> queryFactory,
|
||||
DynamicMap<RestView<ChangeResource>> views) {
|
||||
DynamicMap<RestView<ChangeResource>> views,
|
||||
ChangeUtil changeUtil,
|
||||
ChangeIndexer changeIndexer) {
|
||||
this.db = dbProvider;
|
||||
this.user = user;
|
||||
this.changeControlFactory = changeControlFactory;
|
||||
this.queryFactory = queryFactory;
|
||||
this.views = views;
|
||||
this.changeIndexer = changeIndexer;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -70,6 +78,16 @@ public class ChangesCollection implements
|
||||
public ChangeResource parse(TopLevelResource root, IdString id)
|
||||
throws ResourceNotFoundException, OrmException {
|
||||
List<Change> changes = findChanges(id.encoded());
|
||||
if (changes.isEmpty()) {
|
||||
Integer changeId = Ints.tryParse(id.get());
|
||||
if (changeId != null) {
|
||||
try {
|
||||
changeIndexer.delete(changeId);
|
||||
} catch (IOException e) {
|
||||
throw new ResourceNotFoundException(id.get(), e);
|
||||
}
|
||||
}
|
||||
}
|
||||
if (changes.size() != 1) {
|
||||
throw new ResourceNotFoundException(id);
|
||||
}
|
||||
|
@ -72,6 +72,15 @@ public interface ChangeIndex {
|
||||
*/
|
||||
public void delete(ChangeData cd) throws IOException;
|
||||
|
||||
/**
|
||||
* Delete a change document from the index by id.
|
||||
*
|
||||
* @param id change document id
|
||||
*
|
||||
* @throws IOException
|
||||
*/
|
||||
public void delete(int id) throws IOException;
|
||||
|
||||
/**
|
||||
* Delete all change documents from the index.
|
||||
*
|
||||
|
@ -165,6 +165,17 @@ public class ChangeIndexer {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Synchronously delete a change by id.
|
||||
*
|
||||
* @param id change to delete.
|
||||
*/
|
||||
public void delete(int id) throws IOException {
|
||||
for (ChangeIndex i : getWriteIndexes()) {
|
||||
i.delete(id);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Synchronously delete a change.
|
||||
*
|
||||
|
@ -57,4 +57,8 @@ public class DummyIndex implements ChangeIndex {
|
||||
@Override
|
||||
public void markReady(boolean ready) throws IOException {
|
||||
}
|
||||
|
||||
@Override
|
||||
public void delete(int id) throws IOException {
|
||||
}
|
||||
}
|
||||
|
@ -22,6 +22,8 @@ import com.google.gerrit.server.query.change.ChangeDataSource;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.gwtorm.server.ResultSet;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
class FakeIndex implements ChangeIndex {
|
||||
static Schema<ChangeData> V1 = new Schema<ChangeData>(1, false,
|
||||
ImmutableList.<FieldDef<ChangeData, ?>> of(
|
||||
@ -106,4 +108,8 @@ class FakeIndex implements ChangeIndex {
|
||||
public void markReady(boolean ready) {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void delete(int id) throws IOException {
|
||||
}
|
||||
}
|
||||
|
@ -189,13 +189,27 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
|
||||
String id = cd.getId().toString();
|
||||
try {
|
||||
if (cd.change().getStatus().isOpen()) {
|
||||
openIndex.deleteById(id);
|
||||
commit(openIndex);
|
||||
delete(id, openIndex);
|
||||
} else {
|
||||
closedIndex.deleteById(id);
|
||||
commit(closedIndex);
|
||||
delete(id, closedIndex);
|
||||
}
|
||||
} catch (OrmException | SolrServerException e) {
|
||||
} catch (OrmException e) {
|
||||
throw new IOException(e);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public void delete(int id) throws IOException {
|
||||
String idString = Integer.toString(id);
|
||||
delete(idString, openIndex);
|
||||
delete(idString, closedIndex);
|
||||
}
|
||||
|
||||
private void delete(String id, CloudSolrServer index) throws IOException {
|
||||
try {
|
||||
index.deleteById(id);
|
||||
commit(index);
|
||||
} catch (SolrServerException e) {
|
||||
throw new IOException(e);
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user