diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index e395abed83..d0d77a4a50 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -318,6 +318,11 @@ effect. A change's mergeability can be requested separately by calling the link:#get-mergeable[get-mergeable] endpoint. -- +[[skip_diffstat]] +-- +* `SKIP_DIFFSTAT`: skip the 'insertions' and 'deletions' field in link:#change-info[ChangeInfo]. + For large trees, their computation may be expensive. +-- [[submittable]] -- diff --git a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java index 47ccb49ed0..010ef6d125 100644 --- a/java/com/google/gerrit/extensions/api/changes/ChangeApi.java +++ b/java/com/google/gerrit/extensions/api/changes/ChangeApi.java @@ -248,12 +248,16 @@ public interface ChangeApi { * */ default ChangeInfo get() throws RestApiException { return get( EnumSet.complementOf( - EnumSet.of(ListChangesOption.CHECK, ListChangesOption.SKIP_MERGEABLE))); + EnumSet.of( + ListChangesOption.CHECK, + ListChangesOption.SKIP_MERGEABLE, + ListChangesOption.SKIP_DIFFSTAT))); } /** {@link #get(ListChangesOption...)} with no options included. */ diff --git a/java/com/google/gerrit/extensions/client/ListChangesOption.java b/java/com/google/gerrit/extensions/client/ListChangesOption.java index c842adc61f..425265b3fe 100644 --- a/java/com/google/gerrit/extensions/client/ListChangesOption.java +++ b/java/com/google/gerrit/extensions/client/ListChangesOption.java @@ -75,7 +75,13 @@ public enum ListChangesOption implements ListOption { TRACKING_IDS(21), /** Skip mergeability data */ - SKIP_MERGEABLE(22); + SKIP_MERGEABLE(22), + + /** + * Skip diffstat computation that compute the insertions field (number of lines inserted) and + * deletions field (number of lines deleted) + */ + SKIP_DIFFSTAT(23); private final int value; diff --git a/java/com/google/gerrit/server/change/ChangeJson.java b/java/com/google/gerrit/server/change/ChangeJson.java index d4b347bd57..a3c2e926de 100644 --- a/java/com/google/gerrit/server/change/ChangeJson.java +++ b/java/com/google/gerrit/server/change/ChangeJson.java @@ -28,6 +28,7 @@ import static com.google.gerrit.extensions.client.ListChangesOption.LABELS; import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES; import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWED; import static com.google.gerrit.extensions.client.ListChangesOption.REVIEWER_UPDATES; +import static com.google.gerrit.extensions.client.ListChangesOption.SKIP_DIFFSTAT; import static com.google.gerrit.extensions.client.ListChangesOption.SKIP_MERGEABLE; import static com.google.gerrit.extensions.client.ListChangesOption.SUBMITTABLE; import static com.google.gerrit.extensions.client.ListChangesOption.TRACKING_IDS; @@ -516,10 +517,12 @@ public class ChangeJson { out.submittable = submittable(cd); } } - Optional changedLines = cd.changedLines(); - if (changedLines.isPresent()) { - out.insertions = changedLines.get().insertions; - out.deletions = changedLines.get().deletions; + if (!has(SKIP_DIFFSTAT)) { + Optional changedLines = cd.changedLines(); + if (changedLines.isPresent()) { + out.insertions = changedLines.get().insertions; + out.deletions = changedLines.get().deletions; + } } out.isPrivate = in.isPrivate() ? true : null; out.workInProgress = in.isWorkInProgress() ? true : null; diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java index 5128c3f868..c9c139ea4e 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java @@ -55,12 +55,16 @@ import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS import static com.google.gerrit.server.project.testing.TestLabels.label; import static com.google.gerrit.server.project.testing.TestLabels.value; import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static com.google.gerrit.truth.CacheStatsSubject.assertThat; +import static com.google.gerrit.truth.CacheStatsSubject.cloneStats; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toSet; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheStats; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -156,6 +160,12 @@ import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.index.change.ChangeIndex; import com.google.gerrit.server.index.change.ChangeIndexCollection; import com.google.gerrit.server.index.change.IndexedChangeQuery; +import com.google.gerrit.server.patch.DiffSummary; +import com.google.gerrit.server.patch.DiffSummaryKey; +import com.google.gerrit.server.patch.IntraLineDiff; +import com.google.gerrit.server.patch.IntraLineDiffKey; +import com.google.gerrit.server.patch.PatchList; +import com.google.gerrit.server.patch.PatchListKey; import com.google.gerrit.server.project.testing.TestLabels; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeQueryBuilder.ChangeOperatorFactory; @@ -168,6 +178,7 @@ import com.google.gerrit.testing.FakeEmailSender.Message; import com.google.gerrit.testing.TestTimeUtil; import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.name.Named; import java.io.IOException; import java.sql.Timestamp; import java.util.ArrayList; @@ -207,6 +218,18 @@ public class ChangeIT extends AbstractDaemonTest { @Inject private ProjectOperations projectOperations; @Inject private RequestScopeOperations requestScopeOperations; + @Inject + @Named("diff") + private Cache fileCache; + + @Inject + @Named("diff_intraline") + private Cache intraCache; + + @Inject + @Named("diff_summary") + private Cache diffSummaryCache; + private ChangeIndexedCounter changeIndexedCounter; private RegistrationHandle changeIndexedCounterHandle; @@ -257,6 +280,48 @@ public class ChangeIT extends AbstractDaemonTest { assertThat(c.owner.avatars).isNull(); } + @Test + public void diffStatShouldComputeInsertionsAndDeletions() throws Exception { + String fileName = "a_new_file.txt"; + String fileContent = "First line\nSecond line\n"; + PushOneCommit.Result result = createChange("Add a file", fileName, fileContent); + String triplet = project.get() + "~master~" + result.getChangeId(); + ChangeInfo change = gApi.changes().id(triplet).get(); + assertThat(change.insertions).isNotNull(); + assertThat(change.deletions).isNotNull(); + } + + @Test + public void diffStatShouldSkipInsertionsAndDeletions() throws Exception { + String fileName = "a_new_file.txt"; + String fileContent = "First line\nSecond line\n"; + PushOneCommit.Result result = createChange("Add a file", fileName, fileContent); + String triplet = project.get() + "~master~" + result.getChangeId(); + ChangeInfo change = + gApi.changes().id(triplet).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT)); + assertThat(change.insertions).isNull(); + assertThat(change.deletions).isNull(); + } + + @Test + public void skipDiffstatOptionAvoidsAllDiffComputations() throws Exception { + String fileName = "a_new_file.txt"; + String fileContent = "First line\nSecond line\n"; + PushOneCommit.Result result = createChange("Add a file", fileName, fileContent); + String triplet = project.get() + "~master~" + result.getChangeId(); + CacheStats startPatch = cloneStats(fileCache.stats()); + CacheStats startIntra = cloneStats(intraCache.stats()); + CacheStats startSummary = cloneStats(diffSummaryCache.stats()); + gApi.changes().id(triplet).get(ImmutableList.of(ListChangesOption.SKIP_DIFFSTAT)); + + assertThat(fileCache.stats()).since(startPatch).hasMissCount(0); + assertThat(fileCache.stats()).since(startPatch).hasHitCount(0); + assertThat(intraCache.stats()).since(startIntra).hasMissCount(0); + assertThat(intraCache.stats()).since(startIntra).hasHitCount(0); + assertThat(diffSummaryCache.stats()).since(startSummary).hasMissCount(0); + assertThat(diffSummaryCache.stats()).since(startSummary).hasHitCount(0); + } + @Test public void skipMergeable() throws Exception { PushOneCommit.Result r = createChange();