Add indexed field for total inline comment count

Like unresolved comment count, may be useful for downstream tooling. Not
currently exposed as a searchable predicate, since the utility would be
lower than `has:unresolved`.

Change-Id: I9bb597b6f5c21424efa69510d41985175699977e
This commit is contained in:
Dave Borowitz 2018-10-31 14:31:08 -07:00
parent 72b5b96044
commit c001f32298
10 changed files with 60 additions and 7 deletions

View File

@ -5769,8 +5769,12 @@ Only set if link:#submittable[requested].
Number of inserted lines. Number of inserted lines.
|`deletions` || |`deletions` ||
Number of deleted lines. Number of deleted lines.
|`total_comment_count` |optional|
Total number of inline comments across all patch sets. Not set if the current
change index doesn't have the data.
|`unresolved_comment_count` |optional| |`unresolved_comment_count` |optional|
Number of unresolved comments. Not set if the current change index doesn't have the data. Number of unresolved inline comment threads across all patch sets. Not set if
the current change index doesn't have the data.
|`_number` ||The legacy numeric ID of the change. |`_number` ||The legacy numeric ID of the change.
|`owner` || |`owner` ||
The owner of the change as an link:rest-api-accounts.html#account-info[ The owner of the change as an link:rest-api-accounts.html#account-info[

View File

@ -46,6 +46,7 @@ public class ChangeInfo {
public Boolean submittable; public Boolean submittable;
public Integer insertions; public Integer insertions;
public Integer deletions; public Integer deletions;
public Integer totalCommentCount;
public Integer unresolvedCommentCount; public Integer unresolvedCommentCount;
public Boolean isPrivate; public Boolean isPrivate;
public Boolean workInProgress; public Boolean workInProgress;

View File

@ -76,6 +76,7 @@ import java.util.Set;
import java.util.concurrent.Callable; import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutionException;
import java.util.concurrent.Future; import java.util.concurrent.Future;
import java.util.function.Consumer;
import org.apache.lucene.document.Document; import org.apache.lucene.document.Document;
import org.apache.lucene.index.IndexWriter; import org.apache.lucene.index.IndexWriter;
import org.apache.lucene.index.IndexableField; import org.apache.lucene.index.IndexableField;
@ -129,6 +130,7 @@ public class LuceneChangeIndex implements ChangeIndex {
ChangeField.STORED_SUBMIT_RECORD_LENIENT.getName(); ChangeField.STORED_SUBMIT_RECORD_LENIENT.getName();
private static final String SUBMIT_RECORD_STRICT_FIELD = private static final String SUBMIT_RECORD_STRICT_FIELD =
ChangeField.STORED_SUBMIT_RECORD_STRICT.getName(); ChangeField.STORED_SUBMIT_RECORD_STRICT.getName();
private static final String TOTAL_COMMENT_COUNT_FIELD = ChangeField.TOTAL_COMMENT_COUNT.getName();
private static final String UNRESOLVED_COMMENT_COUNT_FIELD = private static final String UNRESOLVED_COMMENT_COUNT_FIELD =
ChangeField.UNRESOLVED_COMMENT_COUNT.getName(); ChangeField.UNRESOLVED_COMMENT_COUNT.getName();
@ -504,6 +506,7 @@ public class LuceneChangeIndex implements ChangeIndex {
} }
decodeUnresolvedCommentCount(doc, cd); decodeUnresolvedCommentCount(doc, cd);
decodeTotalCommentCount(doc, cd);
return cd; return cd;
} }
@ -633,9 +636,18 @@ public class LuceneChangeIndex implements ChangeIndex {
private void decodeUnresolvedCommentCount( private void decodeUnresolvedCommentCount(
ListMultimap<String, IndexableField> doc, ChangeData cd) { ListMultimap<String, IndexableField> doc, ChangeData cd) {
IndexableField f = Iterables.getFirst(doc.get(UNRESOLVED_COMMENT_COUNT_FIELD), null); decodeIntField(doc, UNRESOLVED_COMMENT_COUNT_FIELD, cd::setUnresolvedCommentCount);
}
private void decodeTotalCommentCount(ListMultimap<String, IndexableField> doc, ChangeData cd) {
decodeIntField(doc, TOTAL_COMMENT_COUNT_FIELD, cd::setTotalCommentCount);
}
private static void decodeIntField(
ListMultimap<String, IndexableField> doc, String fieldName, Consumer<Integer> consumer) {
IndexableField f = Iterables.getFirst(doc.get(fieldName), null);
if (f != null && f.numericValue() != null) { if (f != null && f.numericValue() != null) {
cd.setUnresolvedCommentCount(f.numericValue().intValue()); consumer.accept(f.numericValue().intValue());
} }
} }

View File

@ -535,6 +535,7 @@ public class ChangeJson {
out.created = in.getCreatedOn(); out.created = in.getCreatedOn();
out.updated = in.getLastUpdatedOn(); out.updated = in.getLastUpdatedOn();
out._number = in.getId().get(); out._number = in.getId().get();
out.totalCommentCount = cd.totalCommentCount();
out.unresolvedCommentCount = cd.unresolvedCommentCount(); out.unresolvedCommentCount = cd.unresolvedCommentCount();
if (user.isIdentifiedUser()) { if (user.isIdentifiedUser()) {

View File

@ -508,11 +508,15 @@ public class ChangeField {
cd.messages().stream().map(ChangeMessage::getMessage)) cd.messages().stream().map(ChangeMessage::getMessage))
.collect(toSet())); .collect(toSet()));
/** Number of unresolved comments of the change. */ /** Number of unresolved comment threads of the change, including robot comments. */
public static final FieldDef<ChangeData, Integer> UNRESOLVED_COMMENT_COUNT = public static final FieldDef<ChangeData, Integer> UNRESOLVED_COMMENT_COUNT =
intRange(ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT) intRange(ChangeQueryBuilder.FIELD_UNRESOLVED_COMMENT_COUNT)
.build(ChangeData::unresolvedCommentCount); .build(ChangeData::unresolvedCommentCount);
/** Total number of published inline comments of the change, including robot comments. */
public static final FieldDef<ChangeData, Integer> TOTAL_COMMENT_COUNT =
intRange("total_comments").build(ChangeData::totalCommentCount);
/** Whether the change is mergeable. */ /** Whether the change is mergeable. */
public static final FieldDef<ChangeData, String> MERGEABLE = public static final FieldDef<ChangeData, String> MERGEABLE =
exact(ChangeQueryBuilder.FIELD_MERGEABLE) exact(ChangeQueryBuilder.FIELD_MERGEABLE)

View File

@ -99,7 +99,9 @@ public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated static final Schema<ChangeData> V49 = schema(V48); @Deprecated static final Schema<ChangeData> V49 = schema(V48);
// Bump Lucene version requires reindexing // Bump Lucene version requires reindexing
static final Schema<ChangeData> V50 = schema(V49); @Deprecated static final Schema<ChangeData> V50 = schema(V49);
static final Schema<ChangeData> V51 = schema(V50, ChangeField.TOTAL_COMMENT_COUNT);
public static final String NAME = "changes"; public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions(); public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();

View File

@ -30,6 +30,7 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap; import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Maps; import com.google.common.collect.Maps;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
@ -391,6 +392,7 @@ public class ChangeData {
private PersonIdent committer; private PersonIdent committer;
private int parentCount; private int parentCount;
private Integer unresolvedCommentCount; private Integer unresolvedCommentCount;
private Integer totalCommentCount;
private LabelTypes labelTypes; private LabelTypes labelTypes;
private ImmutableList<byte[]> refStates; private ImmutableList<byte[]> refStates;
@ -925,6 +927,23 @@ public class ChangeData {
this.unresolvedCommentCount = count; this.unresolvedCommentCount = count;
} }
public Integer totalCommentCount() throws OrmException {
if (totalCommentCount == null) {
if (!lazyLoad) {
return null;
}
// Fail on overflow.
totalCommentCount =
Ints.checkedCast((long) publishedComments().size() + robotComments().size());
}
return totalCommentCount;
}
public void setTotalCommentCount(Integer count) {
this.totalCommentCount = count;
}
public List<ChangeMessage> messages() throws OrmException { public List<ChangeMessage> messages() throws OrmException {
if (messages == null) { if (messages == null) {
if (!lazyLoad) { if (!lazyLoad) {

View File

@ -1024,7 +1024,7 @@ public class RobotCommentsIT extends AbstractDaemonTest {
} }
@Test @Test
public void queryChangesWithUnresolvedCommentCount() throws Exception { public void queryChangesWithCommentCounts() throws Exception {
assume().that(notesMigration.readChanges()).isTrue(); assume().that(notesMigration.readChanges()).isTrue();
PushOneCommit.Result r1 = createChange(); PushOneCommit.Result r1 = createChange();
@ -1043,6 +1043,7 @@ public class RobotCommentsIT extends AbstractDaemonTest {
// if we allow users to resolve a robot comment, then this test should // if we allow users to resolve a robot comment, then this test should
// be modified. // be modified.
assertThat(result.unresolvedCommentCount).isEqualTo(0); assertThat(result.unresolvedCommentCount).isEqualTo(0);
assertThat(result.totalCommentCount).isEqualTo(1);
} finally { } finally {
enableDb(ctx); enableDb(ctx);
} }

View File

@ -716,7 +716,7 @@ public class CommentsIT extends AbstractDaemonTest {
} }
@Test @Test
public void queryChangesWithUnresolvedCommentCount() throws Exception { public void queryChangesWithCommentCount() throws Exception {
// PS1 has three comments in three different threads, PS2 has one comment in one thread. // PS1 has three comments in three different threads, PS2 has one comment in one thread.
PushOneCommit.Result result = createChange("change 1", FILE_NAME, "content 1"); PushOneCommit.Result result = createChange("change 1", FILE_NAME, "content 1");
String changeId1 = result.getChangeId(); String changeId1 = result.getChangeId();
@ -751,8 +751,11 @@ public class CommentsIT extends AbstractDaemonTest {
ChangeInfo changeInfo2 = Iterables.getOnlyElement(query(changeId2)); ChangeInfo changeInfo2 = Iterables.getOnlyElement(query(changeId2));
ChangeInfo changeInfo3 = Iterables.getOnlyElement(query(changeId3)); ChangeInfo changeInfo3 = Iterables.getOnlyElement(query(changeId3));
assertThat(changeInfo1.unresolvedCommentCount).isEqualTo(2); assertThat(changeInfo1.unresolvedCommentCount).isEqualTo(2);
assertThat(changeInfo1.totalCommentCount).isEqualTo(4);
assertThat(changeInfo2.unresolvedCommentCount).isEqualTo(0); assertThat(changeInfo2.unresolvedCommentCount).isEqualTo(0);
assertThat(changeInfo2.totalCommentCount).isEqualTo(2);
assertThat(changeInfo3.unresolvedCommentCount).isEqualTo(1); assertThat(changeInfo3.unresolvedCommentCount).isEqualTo(1);
assertThat(changeInfo3.totalCommentCount).isEqualTo(2);
} finally { } finally {
enableDb(ctx); enableDb(ctx);
} }

View File

@ -2378,6 +2378,12 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
cd.reviewers(); cd.reviewers();
cd.unresolvedCommentCount(); cd.unresolvedCommentCount();
if (getSchemaVersion() < 51) {
assertMissingField(ChangeField.TOTAL_COMMENT_COUNT);
} else {
cd.totalCommentCount();
}
// TODO(dborowitz): Swap out GitRepositoryManager somehow? Will probably be // TODO(dborowitz): Swap out GitRepositoryManager somehow? Will probably be
// necessary for NoteDb anyway. // necessary for NoteDb anyway.
cd.isMergeable(); cd.isMergeable();