Merge "Bazel: Add support for toolchain_java11"
diff --git a/Documentation/dev-roles.txt b/Documentation/dev-roles.txt
index d8f3d38..3278b5a 100644
--- a/Documentation/dev-roles.txt
+++ b/Documentation/dev-roles.txt
@@ -253,6 +253,11 @@
responsibilities and live up to the promise of answering incoming
requests in a timely manner.
+Community members may submit new items under the
+link:https://bugs.chromium.org/p/gerrit/issues/list?q=component:ESC[ESC component]
+in the issue tracker, or add that component to existing items, to raise them to
+the attention of ESC members.
+
link:#maintainer[Maintainers] can become steering committee member by
election, or by being appointed by Google (only for the seats that
belong to Google).
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index ea63d73..4279764 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -522,7 +522,9 @@
cfg.setInt("sshd", null, "commandStartThreads", 1);
cfg.setInt("receive", null, "threadPoolSize", 1);
cfg.setInt("index", null, "threads", 1);
- cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
+ if (cfg.getString("index", null, "reindexAfterRefUpdate") == null) {
+ cfg.setBoolean("index", null, "reindexAfterRefUpdate", false);
+ }
}
private static Injector createTestInjector(Daemon daemon) throws Exception {
diff --git a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
index 3d70996..c6d9dee 100644
--- a/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
+++ b/java/com/google/gerrit/extensions/api/projects/ProjectApi.java
@@ -201,6 +201,9 @@
*/
void index(boolean indexChildren) throws RestApiException;
+ /** Reindexes all changes of the project. */
+ void indexChanges() throws RestApiException;
+
/**
* A default implementation which allows source compatibility when adding new methods to the
* interface.
@@ -370,5 +373,10 @@
public void index(boolean indexChildren) throws RestApiException {
throw new NotImplementedException();
}
+
+ @Override
+ public void indexChanges() throws RestApiException {
+ throw new NotImplementedException();
+ }
}
}
diff --git a/java/com/google/gerrit/extensions/events/ChangeIndexedListener.java b/java/com/google/gerrit/extensions/events/ChangeIndexedListener.java
index 8dd64ed..64bc022 100644
--- a/java/com/google/gerrit/extensions/events/ChangeIndexedListener.java
+++ b/java/com/google/gerrit/extensions/events/ChangeIndexedListener.java
@@ -20,6 +20,21 @@
@ExtensionPoint
public interface ChangeIndexedListener {
/**
+ * Invoked when a change is scheduled for indexing.
+ *
+ * @param projectName project containing the change
+ * @param id id of the change that was scheduled for indexing
+ */
+ default void onChangeScheduledForIndexing(String projectName, int id) {}
+
+ /**
+ * Invoked when a change is scheduled for deletion from indexing.
+ *
+ * @param id id of the change that was scheduled for deletion from indexing
+ */
+ default void onChangeScheduledForDeletionFromIndex(int id) {}
+
+ /**
* Invoked when a change is indexed.
*
* @param projectName project containing the change
diff --git a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
index dc968f6..1ac905d 100644
--- a/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
+++ b/java/com/google/gerrit/server/api/projects/ProjectApiImpl.java
@@ -43,6 +43,7 @@
import com.google.gerrit.extensions.api.projects.TagApi;
import com.google.gerrit.extensions.api.projects.TagInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.Input;
import com.google.gerrit.extensions.common.ProjectInfo;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
@@ -69,6 +70,7 @@
import com.google.gerrit.server.restapi.project.GetHead;
import com.google.gerrit.server.restapi.project.GetParent;
import com.google.gerrit.server.restapi.project.Index;
+import com.google.gerrit.server.restapi.project.IndexChanges;
import com.google.gerrit.server.restapi.project.ListBranches;
import com.google.gerrit.server.restapi.project.ListDashboards;
import com.google.gerrit.server.restapi.project.ListTags;
@@ -124,6 +126,7 @@
private final GetParent getParent;
private final SetParent setParent;
private final Index index;
+ private final IndexChanges indexChanges;
@AssistedInject
ProjectApiImpl(
@@ -158,6 +161,7 @@
GetParent getParent,
SetParent setParent,
Index index,
+ IndexChanges indexChanges,
@Assisted ProjectResource project) {
this(
permissionBackend,
@@ -192,6 +196,7 @@
getParent,
setParent,
index,
+ indexChanges,
null);
}
@@ -228,6 +233,7 @@
GetParent getParent,
SetParent setParent,
Index index,
+ IndexChanges indexChanges,
@Assisted String name) {
this(
permissionBackend,
@@ -262,6 +268,7 @@
getParent,
setParent,
index,
+ indexChanges,
name);
}
@@ -298,6 +305,7 @@
GetParent getParent,
SetParent setParent,
Index index,
+ IndexChanges indexChanges,
String name) {
this.permissionBackend = permissionBackend;
this.createProject = createProject;
@@ -332,6 +340,7 @@
this.setParent = setParent;
this.name = name;
this.index = index;
+ this.indexChanges = indexChanges;
}
@Override
@@ -648,6 +657,15 @@
}
}
+ @Override
+ public void indexChanges() throws RestApiException {
+ try {
+ indexChanges.apply(checkExists(), new Input());
+ } catch (Exception e) {
+ throw asRestApiException("Cannot index changes", e);
+ }
+ }
+
private ProjectResource checkExists() throws ResourceNotFoundException {
if (project == null) {
throw new ResourceNotFoundException(name);
diff --git a/java/com/google/gerrit/server/index/change/ChangeIndexer.java b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
index 07bd963..4a1180c 100644
--- a/java/com/google/gerrit/server/index/change/ChangeIndexer.java
+++ b/java/com/google/gerrit/server/index/change/ChangeIndexer.java
@@ -31,7 +31,9 @@
import com.google.gerrit.server.logging.Metadata;
import com.google.gerrit.server.logging.TraceContext;
import com.google.gerrit.server.logging.TraceContext.TraceTimer;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.util.RequestContext;
import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -67,6 +69,7 @@
@Nullable private final ChangeIndexCollection indexes;
@Nullable private final ChangeIndex index;
private final ChangeData.Factory changeDataFactory;
+ private final ChangeNotes.Factory notesFactory;
private final ThreadLocalRequestContext context;
private final ListeningExecutorService batchExecutor;
private final ListeningExecutorService executor;
@@ -83,6 +86,7 @@
ChangeIndexer(
@GerritServerConfig Config cfg,
ChangeData.Factory changeDataFactory,
+ ChangeNotes.Factory notesFactory,
ThreadLocalRequestContext context,
PluginSetContext<ChangeIndexedListener> indexedListeners,
StalenessChecker stalenessChecker,
@@ -91,6 +95,7 @@
@Assisted ChangeIndex index) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
+ this.notesFactory = notesFactory;
this.context = context;
this.indexedListeners = indexedListeners;
this.stalenessChecker = stalenessChecker;
@@ -104,6 +109,7 @@
ChangeIndexer(
@GerritServerConfig Config cfg,
ChangeData.Factory changeDataFactory,
+ ChangeNotes.Factory notesFactory,
ThreadLocalRequestContext context,
PluginSetContext<ChangeIndexedListener> indexedListeners,
StalenessChecker stalenessChecker,
@@ -112,6 +118,7 @@
@Assisted ChangeIndexCollection indexes) {
this.executor = executor;
this.changeDataFactory = changeDataFactory;
+ this.notesFactory = notesFactory;
this.context = context;
this.indexedListeners = indexedListeners;
this.stalenessChecker = stalenessChecker;
@@ -134,6 +141,7 @@
public ListenableFuture<?> indexAsync(Project.NameKey project, Change.Id id) {
IndexTask task = new IndexTask(project, id);
if (queuedIndexTasks.add(task)) {
+ fireChangeScheduledForIndexingEvent(project.get(), id.get());
return submit(task);
}
return Futures.immediateFuture(null);
@@ -159,6 +167,11 @@
* @param cd change to index.
*/
public void index(ChangeData cd) {
+ fireChangeScheduledForIndexingEvent(cd.project().get(), cd.getId().get());
+ doIndex(cd);
+ }
+
+ private void doIndex(ChangeData cd) {
indexImpl(cd);
// Always double-check whether the change might be stale immediately after
@@ -199,10 +212,18 @@
fireChangeIndexedEvent(cd.project().get(), cd.getId().get());
}
+ private void fireChangeScheduledForIndexingEvent(String projectName, int id) {
+ indexedListeners.runEach(l -> l.onChangeScheduledForIndexing(projectName, id));
+ }
+
private void fireChangeIndexedEvent(String projectName, int id) {
indexedListeners.runEach(l -> l.onChangeIndexed(projectName, id));
}
+ private void fireChangeScheduledForDeletionFromIndexEvent(int id) {
+ indexedListeners.runEach(l -> l.onChangeScheduledForDeletionFromIndex(id));
+ }
+
private void fireChangeDeletedFromIndexEvent(int id) {
indexedListeners.runEach(l -> l.onChangeDeleted(id));
}
@@ -233,6 +254,7 @@
* @return future for the deleting task.
*/
public ListenableFuture<?> deleteAsync(Change.Id id) {
+ fireChangeScheduledForDeletionFromIndexEvent(id.get());
return submit(new DeleteTask(id));
}
@@ -242,6 +264,11 @@
* @param id change ID to delete.
*/
public void delete(Change.Id id) {
+ fireChangeScheduledForDeletionFromIndexEvent(id.get());
+ doDelete(id);
+ }
+
+ private void doDelete(Change.Id id) {
new DeleteTask(id).call();
}
@@ -332,8 +359,12 @@
@Override
public Void callImpl() throws Exception {
remove();
- ChangeData cd = changeDataFactory.create(project, id);
- index(cd);
+ try {
+ ChangeNotes changeNotes = notesFactory.createChecked(project, id);
+ doIndex(changeDataFactory.create(changeNotes));
+ } catch (NoSuchChangeException e) {
+ doDelete(id);
+ }
return null;
}
diff --git a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
index 21579d2..c429158 100644
--- a/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
+++ b/java/com/google/gerrit/server/index/change/ReindexAfterRefUpdate.java
@@ -17,7 +17,6 @@
import static com.google.common.util.concurrent.MoreExecutors.directExecutor;
import static com.google.gerrit.server.query.change.ChangeData.asChanges;
-import com.google.common.base.Objects;
import com.google.common.flogger.FluentLogger;
import com.google.common.util.concurrent.FutureCallback;
import com.google.common.util.concurrent.Futures;
@@ -34,20 +33,14 @@
import com.google.gerrit.server.git.QueueProvider.QueueType;
import com.google.gerrit.server.index.IndexExecutor;
import com.google.gerrit.server.index.account.AccountIndexer;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.server.util.RequestContext;
import com.google.inject.Inject;
import com.google.inject.Provider;
-import java.io.IOException;
-import java.util.Collections;
import java.util.List;
-import java.util.Set;
import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Future;
import org.eclipse.jgit.lib.Config;
@@ -58,15 +51,12 @@
private final Provider<InternalChangeQuery> queryProvider;
private final ChangeIndexer.Factory indexerFactory;
private final ChangeIndexCollection indexes;
- private final ChangeNotes.Factory notesFactory;
private final AllUsersName allUsersName;
private final AccountCache accountCache;
private final Provider<AccountIndexer> indexer;
private final ListeningExecutorService executor;
private final boolean enabled;
- private final Set<Index> queuedIndexTasks = Collections.newSetFromMap(new ConcurrentHashMap<>());
-
@Inject
ReindexAfterRefUpdate(
@GerritServerConfig Config cfg,
@@ -74,7 +64,6 @@
Provider<InternalChangeQuery> queryProvider,
ChangeIndexer.Factory indexerFactory,
ChangeIndexCollection indexes,
- ChangeNotes.Factory notesFactory,
AllUsersName allUsersName,
AccountCache accountCache,
Provider<AccountIndexer> indexer,
@@ -83,7 +72,6 @@
this.queryProvider = queryProvider;
this.indexerFactory = indexerFactory;
this.indexes = indexes;
- this.notesFactory = notesFactory;
this.allUsersName = allUsersName;
this.accountCache = accountCache;
this.indexer = indexer;
@@ -113,12 +101,9 @@
@Override
public void onSuccess(List<Change> changes) {
for (Change c : changes) {
- Index task = new Index(event, c.getId());
- if (queuedIndexTasks.add(task)) {
- // Don't retry indefinitely; if this fails changes may be stale.
- @SuppressWarnings("unused")
- Future<?> possiblyIgnoredError = executor.submit(task);
- }
+ @SuppressWarnings("unused")
+ Future<?> possiblyIgnoredError =
+ indexerFactory.create(executor, indexes).indexAsync(c.getProject(), c.getId());
}
}
@@ -178,51 +163,4 @@
@Override
protected void remove() {}
}
-
- private class Index extends Task<Void> {
- private final Change.Id id;
-
- Index(Event event, Change.Id id) {
- super(event);
- this.id = id;
- }
-
- @Override
- protected Void impl(RequestContext ctx) throws IOException {
- // Reload change, as some time may have passed since GetChanges.
- remove();
- try {
- Change c =
- notesFactory.createChecked(Project.nameKey(event.getProjectName()), id).getChange();
- indexerFactory.create(executor, indexes).index(c);
- } catch (NoSuchChangeException e) {
- indexerFactory.create(executor, indexes).delete(id);
- }
- return null;
- }
-
- @Override
- public int hashCode() {
- return Objects.hashCode(Index.class, id.get());
- }
-
- @Override
- public boolean equals(Object obj) {
- if (!(obj instanceof Index)) {
- return false;
- }
- Index other = (Index) obj;
- return id.get() == other.id.get();
- }
-
- @Override
- public String toString() {
- return "Index change " + id.get() + " of project " + event.getProjectName();
- }
-
- @Override
- protected void remove() {
- queuedIndexTasks.remove(this);
- }
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 53940a7..cf72874 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -1396,10 +1396,10 @@
Optional<ChangeData> result =
idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
- assertThat(result.isPresent()).isTrue();
+ assertThat(result).isPresent();
gApi.changes().id(changeId).delete();
result = idx.get(id, IndexedChangeQuery.createOptions(indexConfig, 0, 1, ImmutableSet.of()));
- assertThat(result.isPresent()).isFalse();
+ assertThat(result).isEmpty();
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
index a9c7d99..e3796c4 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIT.java
@@ -24,6 +24,9 @@
import static com.google.gerrit.server.project.ProjectState.OVERRIDDEN_BY_PARENT;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static java.util.stream.Collectors.toSet;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -47,6 +50,7 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ProjectState;
import com.google.gerrit.extensions.client.SubmitType;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.events.ProjectIndexedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
@@ -54,6 +58,7 @@
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.group.SystemGroupBackend;
@@ -79,6 +84,7 @@
private static final String JIRA_MATCH = "(jira\\\\s+#?)(\\\\d+)";
@Inject private DynamicSet<ProjectIndexedListener> projectIndexedListeners;
+ @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -473,6 +479,26 @@
}
@Test
+ public void reindexChangesOfProject() throws Exception {
+ Change.Id changeId1 = createChange().getChange().getId();
+ Change.Id changeId2 = createChange().getChange().getId();
+
+ ChangeIndexedListener changeIndexedListener = mock(ChangeIndexedListener.class);
+ RegistrationHandle registrationHandle =
+ changeIndexedListeners.add("gerrit", changeIndexedListener);
+ try {
+ gApi.projects().name(project.get()).indexChanges();
+
+ verify(changeIndexedListener, times(1))
+ .onChangeScheduledForIndexing(project.get(), changeId1.get());
+ verify(changeIndexedListener, times(1))
+ .onChangeScheduledForIndexing(project.get(), changeId2.get());
+ } finally {
+ registrationHandle.remove();
+ }
+ }
+
+ @Test
public void maxObjectSizeIsNotSetByDefault() throws Exception {
ConfigInfo info = getConfig();
assertThat(info.maxObjectSizeLimit.value).isNull();
diff --git a/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java b/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
index 5892536..49fd781 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/ProjectIndexerIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.api.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
@@ -61,7 +62,7 @@
Optional<FieldBundle> result =
i.getRaw(project, QueryOptions.create(indexConfig, 0, 1, FIELDS));
- assertThat(result.isPresent()).isTrue();
+ assertThat(result).isPresent();
Iterable<byte[]> refState = result.get().getValue(ProjectField.REF_STATE);
assertThat(refState).isNotEmpty();
diff --git a/javatests/com/google/gerrit/acceptance/pgm/BUILD b/javatests/com/google/gerrit/acceptance/pgm/BUILD
index d15c6ce..57b93f6 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/BUILD
+++ b/javatests/com/google/gerrit/acceptance/pgm/BUILD
@@ -30,7 +30,7 @@
"pgm",
"no_windows",
],
- vm_args = ["-Xmx512m"],
+ vm_args = ["-Xmx1024m"],
deps = [
":util",
"//java/com/google/gerrit/elasticsearch",
diff --git a/javatests/com/google/gerrit/acceptance/pgm/InitIT.java b/javatests/com/google/gerrit/acceptance/pgm/InitIT.java
index a573e35..aba0684 100644
--- a/javatests/com/google/gerrit/acceptance/pgm/InitIT.java
+++ b/javatests/com/google/gerrit/acceptance/pgm/InitIT.java
@@ -14,7 +14,7 @@
package com.google.gerrit.acceptance.pgm;
-import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.NoHttpd;
@@ -43,9 +43,9 @@
QueryOptions opts =
QueryOptions.create(IndexConfig.createDefault(), 0, 1, ImmutableSet.of("name"));
Optional<ProjectData> allProjectsData = projectIndex.getSearchIndex().get(allProjects, opts);
- assertThat(allProjectsData.isPresent()).isTrue();
+ assertThat(allProjectsData).isPresent();
Optional<ProjectData> allUsersData = projectIndex.getSearchIndex().get(allUsers, opts);
- assertThat(allUsersData.isPresent()).isTrue();
+ assertThat(allUsersData).isPresent();
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
index c2df9ca..a756c97 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java
@@ -32,6 +32,10 @@
import static java.util.concurrent.TimeUnit.SECONDS;
import static java.util.stream.Collectors.toList;
import static org.eclipse.jgit.lib.Constants.EMPTY_TREE_ID;
+import static org.mockito.Mockito.atLeast;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -39,6 +43,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
@@ -60,6 +65,7 @@
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.common.LabelInfo;
+import com.google.gerrit.extensions.events.ChangeIndexedListener;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -125,6 +131,7 @@
@Inject private ApprovalsUtil approvalsUtil;
@Inject private DynamicSet<OnSubmitValidationListener> onSubmitValidationListeners;
+ @Inject private DynamicSet<ChangeIndexedListener> changeIndexedListeners;
@Inject private IdentifiedUser.GenericFactory userFactory;
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -1210,6 +1217,63 @@
}
}
+ @Test
+ @GerritConfig(name = "index.reindexAfterRefUpdate", value = "true")
+ public void submitSchedulesOpenChangesOfSameBranchForReindexing() throws Throwable {
+ // Create a merged change.
+ PushOneCommit push =
+ pushFactory.create(admin.newIdent(), testRepo, "Merged Change", "foo.txt", "foo");
+ PushOneCommit.Result mergedChange = push.to("refs/for/master");
+ mergedChange.assertOkStatus();
+ approve(mergedChange.getChangeId());
+ submit(mergedChange.getChangeId());
+
+ // Create some open changes.
+ PushOneCommit.Result change1 = createChange();
+ PushOneCommit.Result change2 = createChange();
+ PushOneCommit.Result change3 = createChange();
+
+ // Create a branch with one open change.
+ BranchInput in = new BranchInput();
+ in.revision = projectOperations.project(project).getHead("master").name();
+ gApi.projects().name(project.get()).branch("dev").create(in);
+ PushOneCommit.Result changeOtherBranch = createChange("refs/for/dev");
+
+ ChangeIndexedListener changeIndexedListener = mock(ChangeIndexedListener.class);
+ RegistrationHandle registrationHandle =
+ changeIndexedListeners.add("gerrit", changeIndexedListener);
+ try {
+ // submit a change, this should trigger asynchronous reindexing of the open changes on the
+ // same branch
+ approve(change1.getChangeId());
+ submit(change1.getChangeId());
+ assertThat(gApi.changes().id(change1.getChangeId()).get().status)
+ .isEqualTo(ChangeStatus.MERGED);
+
+ // on submit the change that is submitted gets reindexed synchronously
+ verify(changeIndexedListener, atLeast(1))
+ .onChangeScheduledForIndexing(project.get(), change1.getChange().getId().get());
+ verify(changeIndexedListener, atLeast(1))
+ .onChangeIndexed(project.get(), change1.getChange().getId().get());
+
+ // the open changes on the same branch get reindexed asynchronously
+ verify(changeIndexedListener, times(1))
+ .onChangeScheduledForIndexing(project.get(), change2.getChange().getId().get());
+ verify(changeIndexedListener, times(1))
+ .onChangeScheduledForIndexing(project.get(), change3.getChange().getId().get());
+
+ // merged changes don't get reindexed
+ verify(changeIndexedListener, times(0))
+ .onChangeScheduledForIndexing(project.get(), mergedChange.getChange().getId().get());
+
+ // open changes on other branches don't get reindexed
+ verify(changeIndexedListener, times(0))
+ .onChangeScheduledForIndexing(project.get(), changeOtherBranch.getChange().getId().get());
+ } finally {
+ registrationHandle.remove();
+ }
+ }
+
private void assertSubmitter(PushOneCommit.Result change) throws Throwable {
ChangeInfo info = get(change.getChangeId(), ListChangesOption.MESSAGES);
assertThat(info.messages).isNotNull();
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
index 8ddfa45..a55f17e 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeMessagesIT.java
@@ -15,6 +15,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowCapability;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
@@ -362,8 +363,8 @@
parseCommitMessageRange(commitBefore);
Optional<ChangeNoteUtil.CommitMessageRange> rangeAfter =
parseCommitMessageRange(commitAfter);
- assertThat(rangeBefore.isPresent()).isTrue();
- assertThat(rangeAfter.isPresent()).isTrue();
+ assertThat(rangeBefore).isPresent();
+ assertThat(rangeAfter).isPresent();
String subjectBefore =
decode(
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
index 6e14635..5531709 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/MailProcessorIT.java
@@ -15,6 +15,10 @@
package com.google.gerrit.acceptance.server.mail;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.clearInvocations;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
@@ -37,16 +41,17 @@
import java.time.ZonedDateTime;
import java.util.Collection;
import java.util.List;
-import org.easymock.EasyMock;
import org.junit.Before;
+import org.junit.BeforeClass;
import org.junit.Test;
public class MailProcessorIT extends AbstractMailIT {
@Inject private MailProcessor mailProcessor;
@Inject private AccountOperations accountOperations;
- @Inject private CommentValidator mockCommentValidator;
@Inject private TestCommentHelper testCommentHelper;
+ private static final CommentValidator mockCommentValidator = mock(CommentValidator.class);
+
private static final String COMMENT_TEXT = "The comment text";
@Override
@@ -54,7 +59,6 @@
return new FactoryModule() {
@Override
public void configure() {
- CommentValidator mockCommentValidator = EasyMock.createMock(CommentValidator.class);
bind(CommentValidator.class)
.annotatedWith(Exports.named(mockCommentValidator.getClass()))
.toInstance(mockCommentValidator);
@@ -63,13 +67,15 @@
};
}
+ @BeforeClass
+ public static void setUpMock() {
+ // Let the mock comment validator accept all comments during test setup.
+ when(mockCommentValidator.validateComments(any())).thenReturn(ImmutableList.of());
+ }
+
@Before
public void setUp() {
- // Let the mock comment validator accept all comments during test setup.
- EasyMock.reset(mockCommentValidator);
- EasyMock.expect(mockCommentValidator.validateComments(EasyMock.anyObject()))
- .andReturn(ImmutableList.of());
- EasyMock.replay(mockCommentValidator);
+ clearInvocations(mockCommentValidator);
}
@Test
@@ -268,16 +274,7 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- EasyMock.reset(mockCommentValidator);
- CommentForValidation commentForValidation =
- CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
- EasyMock.expect(
- mockCommentValidator.validateComments(
- ImmutableList.of(
- CommentForValidation.create(
- CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT))))
- .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- EasyMock.replay(mockCommentValidator);
+ setupFailValidation(CommentForValidation.CommentType.CHANGE_MESSAGE);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", COMMENT_TEXT, null, null, null);
@@ -290,7 +287,6 @@
assertNotifyTo(user);
Message message = sender.nextMessage();
assertThat(message.body()).contains("rejected one or more comments");
- EasyMock.verify(mockCommentValidator);
}
@Test
@@ -302,16 +298,7 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- EasyMock.reset(mockCommentValidator);
- CommentForValidation commentForValidation =
- CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
- EasyMock.expect(
- mockCommentValidator.validateComments(
- ImmutableList.of(
- CommentForValidation.create(
- CommentForValidation.CommentType.INLINE_COMMENT, COMMENT_TEXT))))
- .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- EasyMock.replay(mockCommentValidator);
+ setupFailValidation(CommentForValidation.CommentType.INLINE_COMMENT);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, COMMENT_TEXT, null, null);
@@ -324,7 +311,6 @@
assertNotifyTo(user);
Message message = sender.nextMessage();
assertThat(message.body()).contains("rejected one or more comments");
- EasyMock.verify(mockCommentValidator);
}
@Test
@@ -336,16 +322,7 @@
MailProcessingUtil.rfcDateformatter.format(
ZonedDateTime.ofInstant(comments.get(0).updated.toInstant(), ZoneId.of("UTC")));
- EasyMock.reset(mockCommentValidator);
- CommentForValidation commentForValidation =
- CommentForValidation.create(CommentForValidation.CommentType.CHANGE_MESSAGE, COMMENT_TEXT);
- EasyMock.expect(
- mockCommentValidator.validateComments(
- ImmutableList.of(
- CommentForValidation.create(
- CommentForValidation.CommentType.FILE_COMMENT, COMMENT_TEXT))))
- .andReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
- EasyMock.replay(mockCommentValidator);
+ setupFailValidation(CommentForValidation.CommentType.FILE_COMMENT);
MailMessage.Builder b = messageBuilderWithDefaultFields();
String txt = newPlaintextBody(getChangeUrl(changeInfo) + "/1", null, null, COMMENT_TEXT, null);
@@ -358,10 +335,17 @@
assertNotifyTo(user);
Message message = sender.nextMessage();
assertThat(message.body()).contains("rejected one or more comments");
- EasyMock.verify(mockCommentValidator);
}
private String getChangeUrl(ChangeInfo changeInfo) {
return canonicalWebUrl.get() + "c/" + changeInfo.project + "/+/" + changeInfo._number;
}
+
+ private void setupFailValidation(CommentForValidation.CommentType type) {
+ CommentForValidation commentForValidation = CommentForValidation.create(type, COMMENT_TEXT);
+
+ when(mockCommentValidator.validateComments(
+ ImmutableList.of(CommentForValidation.create(type, COMMENT_TEXT))))
+ .thenReturn(ImmutableList.of(commentForValidation.failValidation("Oh no!")));
+ }
}
diff --git a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
index 4f47927..0ad2010 100644
--- a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java
@@ -15,6 +15,7 @@
package com.google.gerrit.acceptance.server.quota;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
@@ -104,7 +105,7 @@
assertThrows(
NullPointerException.class,
() -> quotaBackend.user(identifiedAdmin).requestToken("testGroup"));
- assertThat(exception).isEqualTo(thrown);
+ assertThat(thrown).isEqualTo(exception);
verify(quotaEnforcerA);
}
@@ -136,7 +137,7 @@
OptionalLong tokens =
quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
- assertThat(tokens.isPresent()).isTrue();
+ assertThat(tokens).isPresent();
assertThat(tokens.getAsLong()).isEqualTo(10L);
}
@@ -151,7 +152,7 @@
OptionalLong tokens =
quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens();
- assertThat(tokens.isPresent()).isTrue();
+ assertThat(tokens).isPresent();
assertThat(tokens.getAsLong()).isEqualTo(20L);
}
}
diff --git a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
index 7a1cf51..ee75153 100644
--- a/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
+++ b/javatests/com/google/gerrit/server/extensions/webui/UiActionsTest.java
@@ -15,6 +15,9 @@
package com.google.gerrit.server.extensions.webui;
import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.only;
+import static org.mockito.Mockito.verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -35,7 +38,6 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
-import org.easymock.EasyMock;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
@@ -131,9 +133,7 @@
// Set up the Mock to expect a call of bulkEvaluateTest to only contain cond{1,2} since cond3
// needs to be identified as duplicate and not called out explicitly.
- PermissionBackend permissionBackendMock = EasyMock.createMock(PermissionBackend.class);
- permissionBackendMock.bulkEvaluateTest(ImmutableSet.of(cond1, cond2));
- EasyMock.replay(permissionBackendMock);
+ PermissionBackend permissionBackendMock = mock(PermissionBackend.class);
UiActions.evaluatePermissionBackendConditions(
permissionBackendMock, ImmutableList.of(cond1, cond2, cond3));
@@ -142,6 +142,8 @@
// the value of cond1 and issues no additional call to PermissionBackend.
forProject.disallowValueQueries();
+ verify(permissionBackendMock, only()).bulkEvaluateTest(ImmutableSet.of(cond1, cond2));
+
// Assert the values of all conditions
assertThat(cond1.value()).isFalse();
assertThat(cond2.value()).isTrue();
diff --git a/javatests/com/google/gerrit/server/project/GroupListTest.java b/javatests/com/google/gerrit/server/project/GroupListTest.java
index 5ccefa0..7d6a2c3 100644
--- a/javatests/com/google/gerrit/server/project/GroupListTest.java
+++ b/javatests/com/google/gerrit/server/project/GroupListTest.java
@@ -14,16 +14,14 @@
package com.google.gerrit.server.project;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.expectLastCall;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.reviewdb.client.AccountGroup;
@@ -48,8 +46,7 @@
@Before
public void setup() throws IOException {
- ValidationError.Sink sink = createNiceMock(ValidationError.Sink.class);
- replay(sink);
+ ValidationError.Sink sink = mock(ValidationError.Sink.class);
groupList = GroupList.parse(PROJECT, TEXT, sink);
}
@@ -97,12 +94,9 @@
@Test
public void validationError() throws Exception {
- ValidationError.Sink sink = createMock(ValidationError.Sink.class);
- sink.error(anyObject(ValidationError.class));
- expectLastCall().times(2);
- replay(sink);
+ ValidationError.Sink sink = mock(ValidationError.Sink.class);
groupList = GroupList.parse(PROJECT, TEXT.replace("\t", " "), sink);
- verify(sink);
+ verify(sink, times(2)).error(any(ValidationError.class));
}
@Test
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index b0555ca..20f3bb9 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -2115,6 +2115,12 @@
gApi.changes().id(change1.getChangeId()).current().review(ReviewInput.approve());
gApi.changes().id(change1.getChangeId()).current().submit();
+ // If a change gets submitted, the remaining open changes get reindexed asynchronously to update
+ // their mergeability information. If the further assertions in this test are done before the
+ // asynchronous reindex completed they fail because the mergeability information in the index
+ // was not updated yet. To avoid this flakiness we index change2 synchronously here.
+ gApi.changes().id(change2.getChangeId()).index();
+
assertQuery("status:open conflicts:" + change2.getId().get());
assertQuery("status:open is:mergeable");
assertQuery("status:open -is:mergeable", change2);
diff --git a/javatests/com/google/gerrit/server/rules/BUILD b/javatests/com/google/gerrit/server/rules/BUILD
index 62d9a79..2545431 100644
--- a/javatests/com/google/gerrit/server/rules/BUILD
+++ b/javatests/com/google/gerrit/server/rules/BUILD
@@ -15,6 +15,7 @@
"//lib:guava",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/mockito",
"//lib/prolog:runtime",
"//lib/truth",
"//prolog:gerrit-prolog-common",
diff --git a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
index be0b8e7..9d7afbc 100644
--- a/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
+++ b/javatests/com/google/gerrit/server/rules/GerritCommonTest.java
@@ -16,7 +16,8 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static org.easymock.EasyMock.expect;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import com.google.gerrit.common.data.LabelTypes;
import com.google.gerrit.server.project.testing.TestLabels;
@@ -31,7 +32,6 @@
import java.io.PushbackReader;
import java.io.StringReader;
import java.util.Arrays;
-import org.easymock.EasyMock;
import org.eclipse.jgit.lib.Config;
import org.junit.Before;
import org.junit.Test;
@@ -60,9 +60,8 @@
protected void setUpEnvironment(PrologEnvironment env) throws Exception {
LabelTypes labelTypes =
new LabelTypes(Arrays.asList(TestLabels.codeReview(), TestLabels.verified()));
- ChangeData cd = EasyMock.createMock(ChangeData.class);
- expect(cd.getLabelTypes()).andStubReturn(labelTypes);
- EasyMock.replay(cd);
+ ChangeData cd = mock(ChangeData.class);
+ when(cd.getLabelTypes()).thenReturn(labelTypes);
env.set(StoredValues.CHANGE_DATA, cd);
}
diff --git a/plugins/replication b/plugins/replication
index f1e84b0..5a3519e 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit f1e84b0738b60d89d8bd710e58ebba4eac61b770
+Subproject commit 5a3519e6e1733e2515900866b8db9ca98ba9da7e
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 6962121..a019388 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -83,16 +83,11 @@
};
GrDiffBuilder.ContextButtonType = {
- STEP: 'step',
+ ABOVE: 'above',
+ BELOW: 'below',
ALL: 'all',
};
- const ContextPlacement = {
- LAST: 'last',
- FIRST: 'first',
- MIDDLE: 'middle',
- };
-
const PARTIAL_CONTEXT_AMOUNT = 10;
/**
@@ -236,53 +231,40 @@
group => { return group.element; });
};
- /**
- * @param {!Array<!Object>} contextGroups (!Array<!GrDiffGroup>)
- */
- GrDiffBuilder.prototype._getContextControlPlacementFor = function(
- contextGroups) {
- const firstContextGroup = contextGroups[0];
- const lastContextGroup = contextGroups[contextGroups.length - 1];
- const firstLines = firstContextGroup.lines;
- const lastLines = lastContextGroup.lines;
- const firstContextInFile = firstLines.length && firstLines[0].firstInFile;
- const lastContextInFile = lastLines.length &&
- lastLines[lastLines.length - 1].lastInFile;
- if (firstContextInFile && !lastContextInFile) {
- return ContextPlacement.FIRST;
- } else if (lastContextInFile && !firstContextInFile) {
- return ContextPlacement.LAST;
- }
- return ContextPlacement.MIDDLE;
- };
-
GrDiffBuilder.prototype._createContextControl = function(section, line) {
- const contextGroups = line.contextGroups;
- if (!contextGroups) return null;
- const numLines = contextGroups[contextGroups.length - 1].lineRange.left.end
- - contextGroups[0].lineRange.left.start + 1;
+ if (!line.contextGroups) return null;
+
+ const numLines =
+ line.contextGroups[line.contextGroups.length - 1].lineRange.left.end -
+ line.contextGroups[0].lineRange.left.start + 1;
+
if (numLines === 0) return null;
- const contextPlacement = this._getContextControlPlacementFor(contextGroups);
- const isStepBidirectional = (contextPlacement === ContextPlacement.MIDDLE);
- const minimalForStepExpansion = isStepBidirectional ?
- PARTIAL_CONTEXT_AMOUNT * 2 : PARTIAL_CONTEXT_AMOUNT;
- const showStepExpansion = numLines > minimalForStepExpansion;
-
const td = this._createElement('td');
- if (showStepExpansion) {
+ const showPartialLinks = numLines > PARTIAL_CONTEXT_AMOUNT;
+
+ if (showPartialLinks) {
td.appendChild(this._createContextButton(
- GrDiffBuilder.ContextButtonType.STEP, section, line,
- numLines, contextPlacement));
+ GrDiffBuilder.ContextButtonType.ABOVE, section, line, numLines));
+ td.appendChild(document.createTextNode(' - '));
}
+
td.appendChild(this._createContextButton(
GrDiffBuilder.ContextButtonType.ALL, section, line, numLines));
+ if (showPartialLinks) {
+ td.appendChild(document.createTextNode(' - '));
+ td.appendChild(this._createContextButton(
+ GrDiffBuilder.ContextButtonType.BELOW, section, line, numLines));
+ }
+
return td;
};
GrDiffBuilder.prototype._createContextButton = function(type, section, line,
- numLines, contextPlacement) {
+ numLines) {
+ const context = PARTIAL_CONTEXT_AMOUNT;
+
const button = this._createElement('gr-button', 'showContext');
button.setAttribute('link', true);
button.setAttribute('no-uppercase', true);
@@ -294,14 +276,14 @@
text = 'Show ' + numLines + ' common line';
if (numLines > 1) { text += 's'; }
groups.push(...line.contextGroups);
- } else if (type === GrDiffBuilder.ContextButtonType.STEP) {
- const linesToShowAbove = contextPlacement === ContextPlacement.FIRST ?
- 0 : PARTIAL_CONTEXT_AMOUNT;
- const linesToShowBelow = contextPlacement === ContextPlacement.LAST ?
- 0 : PARTIAL_CONTEXT_AMOUNT;
- text = '+' + PARTIAL_CONTEXT_AMOUNT + ' Lines';
- groups = GrDiffGroup.hideInContextControl(
- line.contextGroups, linesToShowAbove, numLines - linesToShowBelow);
+ } else if (type === GrDiffBuilder.ContextButtonType.ABOVE) {
+ text = '+' + context + '↑';
+ groups = GrDiffGroup.hideInContextControl(line.contextGroups,
+ context, numLines);
+ } else if (type === GrDiffBuilder.ContextButtonType.BELOW) {
+ text = '+' + context + '↓';
+ groups = GrDiffGroup.hideInContextControl(line.contextGroups,
+ 0, numLines - context);
}
Polymer.dom(button).textContent = text;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index 49fdc06..b917845 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -89,92 +89,44 @@
assert.isTrue(node.classList.contains('classes'));
});
- function assertContextControl(buttons, expectedButtons) {
- const actualButtonLabels = buttons.map(
- b => Polymer.dom(b).textContent);
- assert.deepEqual(actualButtonLabels, expectedButtons);
- }
-
- function createContextControl({numLines,
- firstInFile, lastInFile,
- expectStepExpansion}) {
+ test('context control buttons', () => {
+ // Create 10 lines.
const lines = [];
- for (let i = 0; i < numLines; i++) {
+ for (let i = 0; i < 10; i++) {
const line = new GrDiffLine(GrDiffLine.Type.BOTH);
line.beforeNumber = i + 1;
line.afterNumber = i + 1;
line.text = 'lorem upsum';
lines.push(line);
}
- lines[0].firstInFile = !!firstInFile;
- lines[lines.length - 1].lastInFile = !!lastInFile;
+
const contextLine = {
contextGroups: [new GrDiffGroup(GrDiffGroup.Type.BOTH, lines)],
};
+
const section = {};
- const td = builder._createContextControl(section, contextLine);
- return [...td.querySelectorAll('gr-button.showContext')];
- }
-
- function getGroupsAfterExpanding(button) {
- let newGroups;
- button.addEventListener('tap', e => { newGroups = e.detail.groups; });
- MockInteractions.tap(button);
- return newGroups;
- }
-
- test('context control buttons in the middle of the file', () => {
- // Does not include +10 buttons when there are fewer than 21 lines.
- let buttons = createContextControl({numLines: 20});
- assertContextControl(buttons, ['Show 20 common lines']);
-
- // Includes +10 buttons when there are at least 21 lines.
- buttons = createContextControl({numLines: 21});
- assertContextControl(buttons, ['+10 Lines', 'Show 21 common lines']);
-
- // When clicked with 22 Lines expands in both directions with remaining context in the middle.
-
- buttons = createContextControl({numLines: 22});
- assertContextControl(buttons, ['+10 Lines', 'Show 22 common lines']);
- const newGroupTypes = getGroupsAfterExpanding(buttons[0]).map(g => g.type);
- assert.deepEqual(newGroupTypes,
- [GrDiffLine.Type.BOTH, GrDiffLine.Type.CONTEXT_CONTROL,
- GrDiffLine.Type.BOTH]);
- });
-
- test('context control buttons in the beginning of the file', () => {
// Does not include +10 buttons when there are fewer than 11 lines.
- let buttons = createContextControl({numLines: 10, firstInFile: true});
- assertContextControl(buttons, ['Show 10 common lines']);
+ let td = builder._createContextControl(section, contextLine);
+ let buttons = td.querySelectorAll('gr-button.showContext');
- // Includes +10 button when there are at least 11 lines.
- buttons = createContextControl({numLines: 11, firstInFile: true});
- assertContextControl(buttons, ['+10 Lines', 'Show 11 common lines']);
+ assert.equal(buttons.length, 1);
+ assert.equal(Polymer.dom(buttons[0]).textContent, 'Show 10 common lines');
- // When clicked with 12 Lines expands only up and remaining context in the beginning of the file.
- buttons = createContextControl({numLines: 12, firstInFile: true});
- assertContextControl(buttons, ['+10 Lines', 'Show 12 common lines']);
- const newGroupTypes = getGroupsAfterExpanding(buttons[0]).map(g => g.type);
- assert.deepEqual(newGroupTypes,
- [GrDiffLine.Type.CONTEXT_CONTROL, GrDiffLine.Type.BOTH]);
- });
+ // Add another line.
+ const line = new GrDiffLine(GrDiffLine.Type.BOTH);
+ line.text = 'lorem upsum';
+ line.beforeNumber = 11;
+ line.afterNumber = 11;
+ contextLine.contextGroups[0].addLine(line);
- test('context control buttons in the end of the file', () => {
- // Does not include +10 buttons when there are fewer than 11 lines.
- let buttons = createContextControl({numLines: 10, lastInFile: true});
- assertContextControl(buttons, ['Show 10 common lines']);
+ // Includes +10 buttons when there are at least 11 lines.
+ td = builder._createContextControl(section, contextLine);
+ buttons = td.querySelectorAll('gr-button.showContext');
- // Includes +10 button when there are at least 11 lines.
- buttons = createContextControl({numLines: 11, lastInFile: true});
- assertContextControl(buttons, ['+10 Lines', 'Show 11 common lines']);
-
- // When clicked with 12 Lines expands only up and remaining context in the beginning of the file.
- buttons = createContextControl({numLines: 12, lastInFile: true});
- assertContextControl(buttons, ['+10 Lines', 'Show 12 common lines']);
- // When clicked with 12 Lines expands only down and remaining context in the end of the file.
- const newGroupTypes = getGroupsAfterExpanding(buttons[0]).map(g => g.type);
- assert.deepEqual(newGroupTypes, [GrDiffLine.Type.BOTH,
- GrDiffLine.Type.CONTEXT_CONTROL]);
+ assert.equal(buttons.length, 3);
+ assert.equal(Polymer.dom(buttons[0]).textContent, '+10↑');
+ assert.equal(Polymer.dom(buttons[1]).textContent, 'Show 11 common lines');
+ assert.equal(Polymer.dom(buttons[2]).textContent, '+10↓');
});
test('newlines 1', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 65d81bf..104c0c2 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -305,6 +305,7 @@
// Stub the network calls into requests that never resolve.
sandbox.stub(element, '_getDiff', () => new Promise(() => {}));
+ element.patchRange = {};
element.reload();
assert.isTrue(cancelStub.called);
@@ -368,6 +369,7 @@
(changeNum, basePatchNum, patchNum, path, onErr) => {
onErr(error);
});
+ element.patchRange = {};
return element.reload().then(() => {
assert.isTrue(onErrStub.calledOnce);
});
@@ -725,6 +727,7 @@
test('delegates cancel()', () => {
const stub = sandbox.stub(element.$.diff, 'cancel');
+ element.patchRange = {};
element.reload();
assert.isTrue(stub.calledOnce);
assert.equal(stub.lastCall.args.length, 0);
@@ -1243,9 +1246,11 @@
const l = document.createElement('div');
l.setAttribute('comment-side', 'left');
+ l.setAttribute('line-num', 'FILE');
const r = document.createElement('div');
r.setAttribute('comment-side', 'right');
+ r.setAttribute('line-num', 'FILE');
const threadEls = [l, r];
assert.deepEqual(element._filterThreadElsForLocation(threadEls, line),
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index d1c58b2..d4b4e2b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -66,6 +66,7 @@
ADDED: 'edit_b',
REMOVED: 'edit_a',
};
+
/**
* The maximum size for an addition or removal chunk before it is broken down
* into a series of chunks that are this size at most.
@@ -268,8 +269,7 @@
right: this._linesRight(chunk).length,
},
groups: [this._chunkToGroup(
- chunk, state.lineNums.left + 1, state.lineNums.right + 1,
- /* firstInFile */ false, /* lastInFile */ false)],
+ chunk, state.lineNums.left + 1, state.lineNums.right + 1)],
newChunkIndex: state.chunkIndex + 1,
};
}
@@ -321,13 +321,10 @@
const lineCount = collapsibleChunks.reduce(
(sum, chunk) => sum + this._commonChunkLength(chunk), 0);
- const firstChunk = state.chunkIndex === 0;
- const lastChunk = state.chunkIndex === chunks.length - 1;
let groups = this._chunksToGroups(
collapsibleChunks,
state.lineNums.left + 1,
- state.lineNums.right + 1,
- firstChunk, lastChunk);
+ state.lineNums.right + 1);
if (this.context !== WHOLE_FILE) {
const hiddenStart = state.chunkIndex === 0 ? 0 : this.context;
@@ -360,17 +357,11 @@
* @param {!Array<!Object>} chunks
* @param {number} offsetLeft
* @param {number} offsetRight
- * @param {boolean} firstProcessed
- * @param {boolean} lastProcessed
* @return {!Array<!Object>} (GrDiffGroup)
*/
- _chunksToGroups(chunks, offsetLeft, offsetRight, firstProcessed,
- lastProcessed) {
- return chunks.map((chunk, index) => {
- const firstInFile = firstProcessed && index === 0;
- const lastInFile = lastProcessed && index === chunks.length - 1;
- const group = this._chunkToGroup(chunk, offsetLeft,
- offsetRight, firstInFile, lastInFile);
+ _chunksToGroups(chunks, offsetLeft, offsetRight) {
+ return chunks.map(chunk => {
+ const group = this._chunkToGroup(chunk, offsetLeft, offsetRight);
const chunkLength = this._commonChunkLength(chunk);
offsetLeft += chunkLength;
offsetRight += chunkLength;
@@ -384,10 +375,9 @@
* @param {number} offsetRight
* @return {!Object} (GrDiffGroup)
*/
- _chunkToGroup(chunk, offsetLeft, offsetRight, firstInFile, lastInFile) {
+ _chunkToGroup(chunk, offsetLeft, offsetRight) {
const type = chunk.ab ? GrDiffGroup.Type.BOTH : GrDiffGroup.Type.DELTA;
- const lines = this._linesFromChunk(chunk, offsetLeft, offsetRight,
- firstInFile, lastInFile);
+ const lines = this._linesFromChunk(chunk, offsetLeft, offsetRight);
const group = new GrDiffGroup(type, lines);
group.keyLocation = chunk.keyLocation;
group.dueToRebase = chunk.due_to_rebase;
@@ -395,30 +385,25 @@
return group;
},
- _linesFromChunk(chunk, offsetLeft, offsetRight, firstInFile, lastInFile) {
- let lines = [];
+ _linesFromChunk(chunk, offsetLeft, offsetRight) {
if (chunk.ab) {
- lines = chunk.ab.map((row, i) => this._lineFromRow(
+ return chunk.ab.map((row, i) => this._lineFromRow(
GrDiffLine.Type.BOTH, offsetLeft, offsetRight, row, i));
- } else {
- if (chunk.a) {
- // Avoiding a.push(...b) because that causes callstack overflows for
- // large b, which can occur when large files are added removed.
- lines = lines.concat(this._linesFromRows(
- GrDiffLine.Type.REMOVE, chunk.a, offsetLeft,
- chunk[DiffHighlights.REMOVED]));
- }
- if (chunk.b) {
- // Avoiding a.push(...b) because that causes callstack overflows for
- // large b, which can occur when large files are added removed.
- lines = lines.concat(this._linesFromRows(
- GrDiffLine.Type.ADD, chunk.b, offsetRight,
- chunk[DiffHighlights.ADDED]));
- }
}
- if (lines.length > 0) {
- lines[0].firstInFile = firstInFile;
- lines[lines.length - 1].lastInFile = lastInFile;
+ let lines = [];
+ if (chunk.a) {
+ // Avoiding a.push(...b) because that causes callstack overflows for
+ // large b, which can occur when large files are added removed.
+ lines = lines.concat(this._linesFromRows(
+ GrDiffLine.Type.REMOVE, chunk.a, offsetLeft,
+ chunk[DiffHighlights.REMOVED]));
+ }
+ if (chunk.b) {
+ // Avoiding a.push(...b) because that causes callstack overflows for
+ // large b, which can occur when large files are added removed.
+ lines = lines.concat(this._linesFromRows(
+ GrDiffLine.Type.ADD, chunk.b, offsetRight,
+ chunk[DiffHighlights.ADDED]));
}
return lines;
},
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
index 49d583f..b64385d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff-line.js
@@ -44,12 +44,6 @@
this.contextGroups = null;
this.text = '';
-
- /** @type {boolean} */
- this.firstInFile = false;
-
- /** @type {boolean} */
- this.lastInFile = false;
}
GrDiffLine.Type = {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index a9755ce..bc8af9d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -187,8 +187,6 @@
}
.contextControl gr-button {
display: inline-block;
- margin-left: 1em;
- margin-right: 1em;
text-decoration: none;
--gr-button: {
color: var(--diff-context-control-color);
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 6cdac42..f5a64dc 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -111,8 +111,7 @@
'diff/gr-diff-cursor/gr-diff-cursor_test.html',
'diff/gr-diff-highlight/gr-annotation_test.html',
'diff/gr-diff-highlight/gr-diff-highlight_test.html',
- // TODO: uncomment file & fix tests. The file was missed in this list for a long time.
- // 'diff/gr-diff-host/gr-diff-host_test.html',
+ 'diff/gr-diff-host/gr-diff-host_test.html',
'diff/gr-diff-mode-selector/gr-diff-mode-selector_test.html',
'diff/gr-diff-processor/gr-diff-processor_test.html',
'diff/gr-diff-selection/gr-diff-selection_test.html',
