Merge branch 'stable-2.9' into stable-2.10

* stable-2.9:
  Remove uneeded dependency in ChangesCollection
  Delete a change from the index when it is not in the DB

Change-Id: Ifed2fc5073071e9d4f457aa02b73caa2b6173599
This commit is contained in:
Dave Borowitz 2014-11-12 10:03:46 -08:00
commit fdb09997b5
8 changed files with 83 additions and 6 deletions

View File

@ -336,6 +336,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 @Override
public void deleteAll() throws IOException { public void deleteAll() throws IOException {
openIndex.deleteAll(); openIndex.deleteAll();

View File

@ -56,6 +56,10 @@ public class QueryBuilder {
return intTerm(ID_FIELD, cd.getId().get()); 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 Schema<ChangeData> schema;
private final org.apache.lucene.util.QueryBuilder queryBuilder; private final org.apache.lucene.util.QueryBuilder queryBuilder;

View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Ints;
import com.google.gerrit.extensions.registration.DynamicMap; import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.AcceptsPost; import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.IdString; import com.google.gerrit.extensions.restapi.IdString;
@ -26,6 +27,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; 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.ChangeControl;
import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.QueryChanges; import com.google.gerrit.server.query.change.QueryChanges;
@ -34,6 +36,7 @@ import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@ -47,6 +50,7 @@ public class ChangesCollection implements
private final Provider<QueryChanges> queryFactory; private final Provider<QueryChanges> queryFactory;
private final DynamicMap<RestView<ChangeResource>> views; private final DynamicMap<RestView<ChangeResource>> views;
private final CreateChange createChange; private final CreateChange createChange;
private final ChangeIndexer changeIndexer;
@Inject @Inject
ChangesCollection( ChangesCollection(
@ -55,13 +59,15 @@ public class ChangesCollection implements
ChangeControl.GenericFactory changeControlFactory, ChangeControl.GenericFactory changeControlFactory,
Provider<QueryChanges> queryFactory, Provider<QueryChanges> queryFactory,
DynamicMap<RestView<ChangeResource>> views, DynamicMap<RestView<ChangeResource>> views,
CreateChange createChange) { CreateChange createChange,
ChangeIndexer changeIndexer) {
this.db = dbProvider; this.db = dbProvider;
this.user = user; this.user = user;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.queryFactory = queryFactory; this.queryFactory = queryFactory;
this.views = views; this.views = views;
this.createChange = createChange; this.createChange = createChange;
this.changeIndexer = changeIndexer;
} }
@Override @Override
@ -78,6 +84,16 @@ public class ChangesCollection implements
public ChangeResource parse(TopLevelResource root, IdString id) public ChangeResource parse(TopLevelResource root, IdString id)
throws ResourceNotFoundException, OrmException { throws ResourceNotFoundException, OrmException {
List<Change> changes = findChanges(id.encoded()); 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) { if (changes.size() != 1) {
throw new ResourceNotFoundException(id); throw new ResourceNotFoundException(id);
} }

View File

@ -72,6 +72,15 @@ public interface ChangeIndex {
*/ */
public void delete(ChangeData cd) throws IOException; 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. * Delete all change documents from the index.
* *

View File

@ -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. * Synchronously delete a change.
* *

View File

@ -57,4 +57,8 @@ public class DummyIndex implements ChangeIndex {
@Override @Override
public void markReady(boolean ready) throws IOException { public void markReady(boolean ready) throws IOException {
} }
@Override
public void delete(int id) throws IOException {
}
} }

View File

@ -22,6 +22,8 @@ import com.google.gerrit.server.query.change.ChangeDataSource;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.ResultSet;
import java.io.IOException;
class FakeIndex implements ChangeIndex { class FakeIndex implements ChangeIndex {
static Schema<ChangeData> V1 = new Schema<>(1, false, static Schema<ChangeData> V1 = new Schema<>(1, false,
ImmutableList.<FieldDef<ChangeData, ?>> of( ImmutableList.<FieldDef<ChangeData, ?>> of(
@ -106,4 +108,8 @@ class FakeIndex implements ChangeIndex {
public void markReady(boolean ready) { public void markReady(boolean ready) {
throw new UnsupportedOperationException(); throw new UnsupportedOperationException();
} }
@Override
public void delete(int id) throws IOException {
}
} }

View File

@ -189,13 +189,27 @@ class SolrChangeIndex implements ChangeIndex, LifecycleListener {
String id = cd.getId().toString(); String id = cd.getId().toString();
try { try {
if (cd.change().getStatus().isOpen()) { if (cd.change().getStatus().isOpen()) {
openIndex.deleteById(id); delete(id, openIndex);
commit(openIndex);
} else { } else {
closedIndex.deleteById(id); delete(id, closedIndex);
commit(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); throw new IOException(e);
} }
} }