Merge "Fix "TypeError: Cannot read property 'error' of undefined""
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index a8eb875..16dd9b2 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -874,6 +874,12 @@
away from the defaults. The cache may be persisted by setting
`diskLimit`, which is only recommended if cold start performance is
problematic.
++
+`external_ids_map` supports computing the new cache value based on a
+previously cached state. This applies modifications based on the Git
+diff and is almost always faster.
+`cache.external_ids_map.enablePartialReloads` turns this behavior on
+or off. The default is `false`.
cache `"git_tags"`::
+
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index c8ea869..e292549 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2451,40 +2451,6 @@
Disabled plugins can be re-enabled using the
link:cmd-plugin-enable.html[plugin enable] command.
-== Known issues and bugs
-
-=== Error handling in UI when using the REST API
-
-When a plugin invokes a REST endpoint in the UI, it provides an
-`AsyncCallback` to handle the result. At the moment the
-`onFailure(Throwable)` of the callback is never invoked, even if there
-is an error. Errors are always handled by the Gerrit core UI which
-shows the error dialog. This means currently plugins cannot do any
-error handling and e.g. ignore expected errors.
-
-In the following example the REST endpoint would return '404 Not
-Found' if the user has no username and the Gerrit core UI would
-display an error dialog for this. However having no username is
-not an error and the plugin may like to handle this case.
-
-[source,java]
-----
-new RestApi("accounts").id("self").view("username")
- .get(new AsyncCallback<NativeString>() {
-
- @Override
- public void onSuccess(NativeString username) {
- // TODO
- }
-
- @Override
- public void onFailure(Throwable caught) {
- // never invoked
- }
-});
-----
-
-
[[reviewer-suggestion]]
== Reviewer Suggestion Plugins
diff --git a/Documentation/intro-gerrit-walkthrough.txt b/Documentation/intro-gerrit-walkthrough.txt
index 1fba1dc..b4f799c2 100644
--- a/Documentation/intro-gerrit-walkthrough.txt
+++ b/Documentation/intro-gerrit-walkthrough.txt
@@ -28,7 +28,7 @@
modify. To get this code, he runs the following `git clone` command:
----
-clone ssh://gerrithost:29418/RecipeBook.git RecipeBook
+git clone ssh://gerrithost:29418/RecipeBook.git RecipeBook
----
After he clones the repository, he runs a couple of commands to add a
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 49bed97..63cbd7c 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -138,6 +138,10 @@
* `notedb/stage_update_latency`: Latency for staging updates to NoteDb by table.
* `notedb/read_latency`: NoteDb read latency by table.
* `notedb/parse_latency`: NoteDb parse latency by table.
+* `notedb/external_id_cache_load_count`: Total number of times the external ID
+ cache loader was called.
+* `notedb/external_id_partial_read_latency`: Latency for generating a new external ID
+ cache state from a prior state.
* `notedb/external_id_update_count`: Total number of external ID updates.
* `notedb/read_all_external_ids_latency`: Latency for reading all
external ID's from NoteDb.
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 15f59c1..56376fa 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -849,8 +849,10 @@
Creates a new patch set with a new commit message.
The new commit message must be provided in the request body inside a
-link:#commit-message-input[CommitMessageInput] entity and contain the change ID footer if
-link:project-configuration.html#require-change-id[Require Change-Id] was specified.
+link:#commit-message-input[CommitMessageInput] entity. If a Change-Id
+footer is specified, it must match the current Change-Id footer. If
+the Change-Id footer is absent, the current Change-Id is added to the
+message.
.Request
----
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index f69c4ae..76151b4 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -3058,12 +3058,12 @@
|Field Name | |Description
|`match` | |A JavaScript regular expression to match
positions to be replaced with a hyperlink, as documented in
-link#config-gerrit.html#commentlink.name.match[commentlink.name.match].
+link:config-gerrit.html#commentlink.name.match[commentlink.name.match].
|`link` | |The URL to direct the user to whenever the
regular expression is matched, as documented in
-link#config-gerrit.html#commentlink.name.link[commentlink.name.link].
+link:config-gerrit.html#commentlink.name.link[commentlink.name.link].
|`enabled` |optional|Whether the commentlink is enabled, as documented
-in link#config-gerrit.html#commentlink.name.enabled[
+in link:config-gerrit.html#commentlink.name.enabled[
commentlink.name.enabled]. If not set the commentlink is enabled.
|==================================================
diff --git a/Documentation/user-inline-edit.txt b/Documentation/user-inline-edit.txt
index f64c449..1f5e195 100644
--- a/Documentation/user-inline-edit.txt
+++ b/Documentation/user-inline-edit.txt
@@ -72,7 +72,7 @@
To add a file to the change:
-. In the top left corner of the change, click Edit.
+. In the top right corner of the change, click Edit.
. Next to Files, click Open:
+
diff --git a/java/com/google/gerrit/extensions/api/changes/HashtagsInput.java b/java/com/google/gerrit/extensions/api/changes/HashtagsInput.java
index 8f66f12..bbc8a2e 100644
--- a/java/com/google/gerrit/extensions/api/changes/HashtagsInput.java
+++ b/java/com/google/gerrit/extensions/api/changes/HashtagsInput.java
@@ -26,4 +26,9 @@
public HashtagsInput(Set<String> add) {
this.add = add;
}
+
+ public HashtagsInput(Set<String> add, Set<String> remove) {
+ this(add);
+ this.remove = remove;
+ }
}
diff --git a/java/com/google/gerrit/json/BUILD b/java/com/google/gerrit/json/BUILD
index 7282dc4..030dddc 100644
--- a/java/com/google/gerrit/json/BUILD
+++ b/java/com/google/gerrit/json/BUILD
@@ -4,5 +4,6 @@
visibility = ["//visibility:public"],
deps = [
"//lib:gson",
+ "//lib/flogger:api",
],
)
diff --git a/java/com/google/gerrit/json/EnumTypeAdapterFactory.java b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
new file mode 100644
index 0000000..dc74f67
--- /dev/null
+++ b/java/com/google/gerrit/json/EnumTypeAdapterFactory.java
@@ -0,0 +1,78 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.json;
+
+import com.google.common.flogger.FluentLogger;
+import com.google.gson.Gson;
+import com.google.gson.TypeAdapter;
+import com.google.gson.TypeAdapterFactory;
+import com.google.gson.internal.bind.TypeAdapters;
+import com.google.gson.reflect.TypeToken;
+import com.google.gson.stream.JsonReader;
+import com.google.gson.stream.JsonToken;
+import com.google.gson.stream.JsonWriter;
+import java.io.IOException;
+
+/**
+ * A {@code TypeAdapterFactory} for enums.
+ *
+ * <p>This factory introduces a wrapper around Gson's own default enum handler to add the following
+ * special behavior: log when input which doesn't match any existing enum value is encountered.
+ */
+public class EnumTypeAdapterFactory implements TypeAdapterFactory {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ @SuppressWarnings({"unchecked"})
+ @Override
+ public <T> TypeAdapter<T> create(Gson gson, TypeToken<T> typeToken) {
+ TypeAdapter<T> defaultEnumAdapter = TypeAdapters.ENUM_FACTORY.create(gson, typeToken);
+ if (defaultEnumAdapter == null) {
+ // Not an enum. -> Enum type adapter doesn't apply.
+ return null;
+ }
+
+ return (TypeAdapter<T>) new EnumTypeAdapter(defaultEnumAdapter, typeToken);
+ }
+
+ private static class EnumTypeAdapter<T extends Enum<T>> extends TypeAdapter<T> {
+
+ private final TypeAdapter<T> defaultEnumAdapter;
+ private final TypeToken<T> typeToken;
+
+ public EnumTypeAdapter(TypeAdapter<T> defaultEnumAdapter, TypeToken<T> typeToken) {
+ this.defaultEnumAdapter = defaultEnumAdapter;
+ this.typeToken = typeToken;
+ }
+
+ @Override
+ public T read(JsonReader in) throws IOException {
+ // Still handle null values. -> Check them first.
+ if (in.peek() == JsonToken.NULL) {
+ in.nextNull();
+ return null;
+ }
+ T enumValue = defaultEnumAdapter.read(in);
+ if (enumValue == null) {
+ logger.atWarning().log("Expected an existing value for enum %s.", typeToken);
+ }
+ return enumValue;
+ }
+
+ @Override
+ public void write(JsonWriter out, T value) throws IOException {
+ defaultEnumAdapter.write(out, value);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/json/OutputFormat.java b/java/com/google/gerrit/json/OutputFormat.java
index a2d174f..3e7c319 100644
--- a/java/com/google/gerrit/json/OutputFormat.java
+++ b/java/com/google/gerrit/json/OutputFormat.java
@@ -55,7 +55,8 @@
GsonBuilder gb =
new GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES)
- .registerTypeAdapter(Timestamp.class, new SqlTimestampDeserializer());
+ .registerTypeAdapter(Timestamp.class, new SqlTimestampDeserializer())
+ .registerTypeAdapterFactory(new EnumTypeAdapterFactory());
if (this == OutputFormat.JSON) {
gb.setPrettyPrinting();
}
diff --git a/java/com/google/gerrit/server/BUILD b/java/com/google/gerrit/server/BUILD
index 2cc4fb2..6d4dfcf 100644
--- a/java/com/google/gerrit/server/BUILD
+++ b/java/com/google/gerrit/server/BUILD
@@ -8,6 +8,11 @@
"config/GerritGlobalModule.java",
]
+TESTING_SRC = [
+ "account/externalids/testing/ExternalIdInserter.java",
+ "account/externalids/testing/ExternalIdTestUtil.java",
+]
+
java_library(
name = "constants",
srcs = CONSTANTS_SRC,
@@ -24,7 +29,7 @@
name = "server",
srcs = glob(
["**/*.java"],
- exclude = CONSTANTS_SRC + GERRIT_GLOBAL_MODULE_SRC,
+ exclude = CONSTANTS_SRC + GERRIT_GLOBAL_MODULE_SRC + TESTING_SRC,
),
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/server"],
diff --git a/java/com/google/gerrit/server/account/AccountManager.java b/java/com/google/gerrit/server/account/AccountManager.java
index 7fb9997..09757eb 100644
--- a/java/com/google/gerrit/server/account/AccountManager.java
+++ b/java/com/google/gerrit/server/account/AccountManager.java
@@ -15,6 +15,7 @@
package com.google.gerrit.server.account;
import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import com.google.common.base.Strings;
@@ -448,8 +449,10 @@
"Delete External IDs on Update Link",
to,
(a, u) -> {
- Collection<ExternalId> filteredExtIdsByScheme =
- a.getExternalIds(who.getExternalIdKey().scheme());
+ Set<ExternalId> filteredExtIdsByScheme =
+ a.getExternalIds().stream()
+ .filter(e -> e.key().isScheme(who.getExternalIdKey().scheme()))
+ .collect(toImmutableSet());
if (filteredExtIdsByScheme.isEmpty()) {
return;
}
diff --git a/java/com/google/gerrit/server/account/AccountState.java b/java/com/google/gerrit/server/account/AccountState.java
index 4a04f01..8555166 100644
--- a/java/com/google/gerrit/server/account/AccountState.java
+++ b/java/com/google/gerrit/server/account/AccountState.java
@@ -14,13 +14,10 @@
package com.google.gerrit.server.account;
-import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
@@ -29,8 +26,6 @@
import com.google.gerrit.extensions.client.EditPreferencesInfo;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.CurrentUser.PropertyKey;
-import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
import com.google.gerrit.server.account.ProjectWatches.ProjectWatchKey;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -146,7 +141,6 @@
private final GeneralPreferencesInfo generalPreferences;
private final DiffPreferencesInfo diffPreferences;
private final EditPreferencesInfo editPreferences;
- private Cache<IdentifiedUser.PropertyKey<Object>, Object> properties;
private AccountState(
Account account,
@@ -209,11 +203,6 @@
return externalIds;
}
- /** The external identities that identify the account holder that match the given scheme. */
- public ImmutableSet<ExternalId> getExternalIds(String scheme) {
- return externalIds.stream().filter(e -> e.key().isScheme(scheme)).collect(toImmutableSet());
- }
-
/** The project watches of the account. */
public ImmutableMap<ProjectWatchKey, ImmutableSet<NotifyType>> getProjectWatches() {
return projectWatches;
@@ -234,61 +223,6 @@
return editPreferences;
}
- /**
- * Lookup a previously stored property.
- *
- * <p>All properties are automatically cleared when the account cache invalidates the {@code
- * AccountState}. This method is thread-safe.
- *
- * @param key unique property key.
- * @return previously stored value, or {@code null}.
- */
- @Nullable
- public <T> T get(PropertyKey<T> key) {
- Cache<PropertyKey<Object>, Object> p = properties(false);
- if (p != null) {
- @SuppressWarnings("unchecked")
- T value = (T) p.getIfPresent(key);
- return value;
- }
- return null;
- }
-
- /**
- * Store a property for later retrieval.
- *
- * <p>This method is thread-safe.
- *
- * @param key unique property key.
- * @param value value to store; or {@code null} to clear the value.
- */
- public <T> void put(PropertyKey<T> key, @Nullable T value) {
- Cache<PropertyKey<Object>, Object> p = properties(value != null);
- if (p != null) {
- @SuppressWarnings("unchecked")
- PropertyKey<Object> k = (PropertyKey<Object>) key;
- if (value != null) {
- p.put(k, value);
- } else {
- p.invalidate(k);
- }
- }
- }
-
- private synchronized Cache<PropertyKey<Object>, Object> properties(boolean allocate) {
- if (properties == null && allocate) {
- properties =
- CacheBuilder.newBuilder()
- .concurrencyLevel(1)
- .initialCapacity(16)
- // Use weakKeys to ensure plugins that garbage collect will also
- // eventually release data held in any still live AccountState.
- .weakKeys()
- .build();
- }
- return properties;
- }
-
@Override
public String toString() {
MoreObjects.ToStringHelper h = MoreObjects.toStringHelper(this);
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
index 25fca4d..35c4c98 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheImpl.java
@@ -14,17 +14,12 @@
package com.google.gerrit.server.account.externalids;
-import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.SetMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.reviewdb.client.Account;
-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.inject.Inject;
import com.google.inject.Singleton;
import com.google.inject.name.Named;
@@ -147,24 +142,4 @@
lock.unlock();
}
}
-
- static class Loader extends CacheLoader<ObjectId, AllExternalIds> {
- private final ExternalIdReader externalIdReader;
-
- @Inject
- Loader(ExternalIdReader externalIdReader) {
- this.externalIdReader = externalIdReader;
- }
-
- @Override
- public AllExternalIds load(ObjectId notesRev) throws Exception {
- try (TraceTimer timer =
- TraceContext.newTimer(
- "Loading external IDs", Metadata.builder().revision(notesRev.name()).build())) {
- ImmutableSet<ExternalId> externalIds = externalIdReader.all(notesRev);
- externalIds.forEach(ExternalId::checkThatBlobIdIsSet);
- return AllExternalIds.create(externalIds);
- }
- }
- }
}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
new file mode 100644
index 0000000..68841da
--- /dev/null
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdCacheLoader.java
@@ -0,0 +1,271 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids;
+
+import com.google.common.base.CharMatcher;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheLoader;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableSetMultimap;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.metrics.Counter1;
+import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Description.Units;
+import com.google.gerrit.metrics.Field;
+import com.google.gerrit.metrics.MetricMaker;
+import com.google.gerrit.metrics.Timer0;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.git.GitRepositoryManager;
+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.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.name.Named;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
+import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
+import org.eclipse.jgit.treewalk.filter.TreeFilter;
+
+/** Loads cache values for the external ID cache using either a full or a partial reload. */
+@Singleton
+public class ExternalIdCacheLoader extends CacheLoader<ObjectId, AllExternalIds> {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ // Maximum number of prior states we inspect to find a base for differential. If no cached state
+ // is found within this number of parents, we fall back to reading everything from scratch.
+ private static final int MAX_HISTORY_LOOKBACK = 10;
+
+ private final ExternalIdReader externalIdReader;
+ private final Provider<Cache<ObjectId, AllExternalIds>> externalIdCache;
+ private final GitRepositoryManager gitRepositoryManager;
+ private final AllUsersName allUsersName;
+ private final Counter1<Boolean> reloadCounter;
+ private final Timer0 reloadDifferential;
+ private final boolean enablePartialReloads;
+
+ @Inject
+ ExternalIdCacheLoader(
+ GitRepositoryManager gitRepositoryManager,
+ AllUsersName allUsersName,
+ ExternalIdReader externalIdReader,
+ @Named(ExternalIdCacheImpl.CACHE_NAME)
+ Provider<Cache<ObjectId, AllExternalIds>> externalIdCache,
+ MetricMaker metricMaker,
+ @GerritServerConfig Config config) {
+ this.externalIdReader = externalIdReader;
+ this.externalIdCache = externalIdCache;
+ this.gitRepositoryManager = gitRepositoryManager;
+ this.allUsersName = allUsersName;
+ this.reloadCounter =
+ metricMaker.newCounter(
+ "notedb/external_id_cache_load_count",
+ new Description("Total number of external ID cache reloads from Git.")
+ .setRate()
+ .setUnit("updates"),
+ Field.ofBoolean("partial", Metadata.Builder::partial).build());
+ this.reloadDifferential =
+ metricMaker.newTimer(
+ "notedb/external_id_partial_read_latency",
+ new Description(
+ "Latency for generating a new external ID cache state from a prior state.")
+ .setCumulative()
+ .setUnit(Units.MILLISECONDS));
+ this.enablePartialReloads =
+ config.getBoolean("cache", ExternalIdCacheImpl.CACHE_NAME, "enablePartialReloads", false);
+ }
+
+ @Override
+ public AllExternalIds load(ObjectId notesRev) throws IOException, ConfigInvalidException {
+ if (!enablePartialReloads) {
+ logger.atInfo().log(
+ "Partial reloads of "
+ + ExternalIdCacheImpl.CACHE_NAME
+ + " disabled. Falling back to full reload.");
+ return reloadAllExternalIds(notesRev);
+ }
+
+ // The requested value was not in the cache (hence, this loader was invoked). Therefore, try to
+ // create this entry from a past value using the minimal amount of Git operations possible to
+ // reduce latency.
+ //
+ // First, try to find the most recent state we have in the cache. Most of the time, this will be
+ // the state before the last update happened, but it can also date further back. We try a best
+ // effort approach and check the last 10 states. If nothing is found, we default to loading the
+ // value from scratch.
+ //
+ // If a prior state was found, we use Git to diff the trees and find modifications. This is
+ // faster than just loading the complete current tree and working off of that because of how the
+ // data is structured: NotesMaps use nested trees, so, for example, a NotesMap with 200k entries
+ // has two layers of nesting: 12/34/1234..99. TreeWalk is smart in skipping the traversal of
+ // identical subtrees.
+ //
+ // Once we know what files changed, we apply additions and removals to the previously cached
+ // state.
+
+ try (Repository repo = gitRepositoryManager.openRepository(allUsersName);
+ RevWalk rw = new RevWalk(repo)) {
+ long start = System.nanoTime();
+ Ref extIdRef = repo.exactRef(RefNames.REFS_EXTERNAL_IDS);
+ if (extIdRef == null) {
+ logger.atInfo().log(
+ RefNames.REFS_EXTERNAL_IDS + " not initialized, falling back to full reload.");
+ return reloadAllExternalIds(notesRev);
+ }
+
+ RevCommit currentCommit = rw.parseCommit(extIdRef.getObjectId());
+ rw.markStart(currentCommit);
+ RevCommit parentWithCacheValue;
+ AllExternalIds oldExternalIds = null;
+ int i = 0;
+ while ((parentWithCacheValue = rw.next()) != null
+ && i++ < MAX_HISTORY_LOOKBACK
+ && parentWithCacheValue.getParentCount() < 2) {
+ oldExternalIds = externalIdCache.get().getIfPresent(parentWithCacheValue.getId());
+ if (oldExternalIds != null) {
+ // We found a previously cached state.
+ break;
+ }
+ }
+ if (oldExternalIds == null) {
+ logger.atWarning().log(
+ "Unable to find an old ExternalId cache state, falling back to full reload");
+ return reloadAllExternalIds(notesRev);
+ }
+
+ // Diff trees to recognize modifications
+ Set<ObjectId> removals = new HashSet<>(); // Set<Blob-Object-Id>
+ Map<ObjectId, ObjectId> additions = new HashMap<>(); // Map<Name-ObjectId, Blob-Object-Id>
+ try (TreeWalk treeWalk = new TreeWalk(repo)) {
+ treeWalk.setFilter(TreeFilter.ANY_DIFF);
+ treeWalk.setRecursive(true);
+ treeWalk.reset(parentWithCacheValue.getTree(), currentCommit.getTree());
+ while (treeWalk.next()) {
+ String path = treeWalk.getPathString();
+ ObjectId oldBlob = treeWalk.getObjectId(0);
+ ObjectId newBlob = treeWalk.getObjectId(1);
+ if (ObjectId.zeroId().equals(newBlob)) {
+ // Deletion
+ removals.add(oldBlob);
+ } else if (ObjectId.zeroId().equals(oldBlob)) {
+ // Addition
+ additions.put(fileNameToObjectId(path), newBlob);
+ } else {
+ // Modification
+ removals.add(oldBlob);
+ additions.put(fileNameToObjectId(path), newBlob);
+ }
+ }
+ }
+
+ AllExternalIds allExternalIds =
+ buildAllExternalIds(repo, oldExternalIds, additions, removals);
+ reloadCounter.increment(true);
+ reloadDifferential.record(System.nanoTime() - start, TimeUnit.NANOSECONDS);
+ return allExternalIds;
+ }
+ }
+
+ private static ObjectId fileNameToObjectId(String path) {
+ return ObjectId.fromString(CharMatcher.is('/').removeFrom(path));
+ }
+
+ /**
+ * Build a new {@link AllExternalIds} from an old state by applying additions and removals that
+ * were performed since then.
+ *
+ * <p>Removals are applied before additions.
+ *
+ * @param repo open repository
+ * @param oldExternalIds prior state that is used as base
+ * @param additions map of name to blob ID for each external ID that should be added
+ * @param removals set of name {@link ObjectId}s that should be removed
+ */
+ private static AllExternalIds buildAllExternalIds(
+ Repository repo,
+ AllExternalIds oldExternalIds,
+ Map<ObjectId, ObjectId> additions,
+ Set<ObjectId> removals)
+ throws IOException {
+ ImmutableSetMultimap.Builder<Account.Id, ExternalId> byAccount = ImmutableSetMultimap.builder();
+ ImmutableSetMultimap.Builder<String, ExternalId> byEmail = ImmutableSetMultimap.builder();
+
+ // Copy over old ExternalIds but exclude deleted ones
+ for (ExternalId externalId : oldExternalIds.byAccount().values()) {
+ if (removals.contains(externalId.blobId())) {
+ continue;
+ }
+
+ byAccount.put(externalId.accountId(), externalId);
+ if (externalId.email() != null) {
+ byEmail.put(externalId.email(), externalId);
+ }
+ }
+
+ // Add newly discovered ExternalIds
+ try (ObjectReader reader = repo.newObjectReader()) {
+ for (Map.Entry<ObjectId, ObjectId> nameToBlob : additions.entrySet()) {
+ ExternalId parsedExternalId;
+ try {
+ parsedExternalId =
+ ExternalId.parse(
+ nameToBlob.getKey().name(),
+ reader.open(nameToBlob.getValue()).getCachedBytes(),
+ nameToBlob.getValue());
+ } catch (ConfigInvalidException | RuntimeException e) {
+ logger.atSevere().withCause(e).log(
+ "Ignoring invalid external ID note %s", nameToBlob.getKey().name());
+ continue;
+ }
+
+ byAccount.put(parsedExternalId.accountId(), parsedExternalId);
+ if (parsedExternalId.email() != null) {
+ byEmail.put(parsedExternalId.email(), parsedExternalId);
+ }
+ }
+ }
+ return new AutoValue_AllExternalIds(byAccount.build(), byEmail.build());
+ }
+
+ private AllExternalIds reloadAllExternalIds(ObjectId notesRev)
+ throws IOException, ConfigInvalidException {
+ try (TraceTimer ignored =
+ TraceContext.newTimer(
+ "Loading external IDs from scratch",
+ Metadata.builder().revision(notesRev.name()).build())) {
+ ImmutableSet<ExternalId> externalIds = externalIdReader.all(notesRev);
+ externalIds.forEach(ExternalId::checkThatBlobIdIsSet);
+ AllExternalIds allExternalIds = AllExternalIds.create(externalIds);
+ reloadCounter.increment(false);
+ return allExternalIds;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java
index fc311e7..3e5d7b8 100644
--- a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java
+++ b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java
@@ -14,7 +14,6 @@
package com.google.gerrit.server.account.externalids;
-import com.google.gerrit.server.account.externalids.ExternalIdCacheImpl.Loader;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer;
import com.google.inject.TypeLiteral;
@@ -31,9 +30,12 @@
// from the cache. Extend the cache size by 1 to cover this case, but expire the extra
// object after a short period of time, since it may be a potentially large amount of
// memory.
+ // When loading a new value because the primary data advanced, we want to leverage the old
+ // cache state to recompute only what changed. This doesn't affect cache size though as
+ // Guava calls the loader first and evicts later on.
.maximumWeight(2)
.expireFromMemoryAfterAccess(Duration.ofMinutes(1))
- .loader(Loader.class)
+ .loader(ExternalIdCacheLoader.class)
.diskLimit(-1)
.version(1)
.keySerializer(ObjectIdCacheSerializer.INSTANCE)
diff --git a/java/com/google/gerrit/server/account/externalids/testing/BUILD b/java/com/google/gerrit/server/account/externalids/testing/BUILD
new file mode 100644
index 0000000..ec98ec8
--- /dev/null
+++ b/java/com/google/gerrit/server/account/externalids/testing/BUILD
@@ -0,0 +1,11 @@
+java_library(
+ name = "testing",
+ testonly = 1,
+ srcs = glob(["**/*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/reviewdb:server",
+ "//java/com/google/gerrit/server",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ ],
+)
diff --git a/java/com/google/gerrit/server/account/externalids/testing/ExternalIdInserter.java b/java/com/google/gerrit/server/account/externalids/testing/ExternalIdInserter.java
new file mode 100644
index 0000000..a93192a
--- /dev/null
+++ b/java/com/google/gerrit/server/account/externalids/testing/ExternalIdInserter.java
@@ -0,0 +1,25 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids.testing;
+
+import java.io.IOException;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.notes.NoteMap;
+
+@FunctionalInterface
+public interface ExternalIdInserter {
+ public ObjectId addNote(ObjectInserter ins, NoteMap noteMap) throws IOException;
+}
diff --git a/java/com/google/gerrit/server/account/externalids/testing/ExternalIdTestUtil.java b/java/com/google/gerrit/server/account/externalids/testing/ExternalIdTestUtil.java
new file mode 100644
index 0000000..97fff60
--- /dev/null
+++ b/java/com/google/gerrit/server/account/externalids/testing/ExternalIdTestUtil.java
@@ -0,0 +1,163 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids.testing;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
+import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
+
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.externalids.ExternalId;
+import com.google.gerrit.server.account.externalids.ExternalIdReader;
+import java.io.IOException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.notes.NoteMap;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+
+/** Common methods for dealing with external IDs in tests. */
+public class ExternalIdTestUtil {
+
+ public static String insertExternalIdWithoutAccountId(
+ Repository repo, RevWalk rw, PersonIdent ident, Account.Id accountId, String externalId)
+ throws IOException {
+ return insertExternalId(
+ repo,
+ rw,
+ ident,
+ (ins, noteMap) -> {
+ ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), accountId);
+ ObjectId noteId = extId.key().sha1();
+ Config c = new Config();
+ extId.writeToConfig(c);
+ c.unset("externalId", extId.key().get(), "accountId");
+ byte[] raw = c.toText().getBytes(UTF_8);
+ ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
+ noteMap.set(noteId, dataBlob);
+ return noteId;
+ });
+ }
+
+ public static String insertExternalIdWithKeyThatDoesntMatchNoteId(
+ Repository repo, RevWalk rw, PersonIdent ident, Account.Id accountId, String externalId)
+ throws IOException {
+ return insertExternalId(
+ repo,
+ rw,
+ ident,
+ (ins, noteMap) -> {
+ ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), accountId);
+ ObjectId noteId = ExternalId.Key.parse(externalId + "x").sha1();
+ Config c = new Config();
+ extId.writeToConfig(c);
+ byte[] raw = c.toText().getBytes(UTF_8);
+ ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
+ noteMap.set(noteId, dataBlob);
+ return noteId;
+ });
+ }
+
+ public static String insertExternalIdWithInvalidConfig(
+ Repository repo, RevWalk rw, PersonIdent ident, String externalId) throws IOException {
+ return insertExternalId(
+ repo,
+ rw,
+ ident,
+ (ins, noteMap) -> {
+ ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
+ byte[] raw = "bad-config".getBytes(UTF_8);
+ ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
+ noteMap.set(noteId, dataBlob);
+ return noteId;
+ });
+ }
+
+ public static String insertExternalIdWithEmptyNote(
+ Repository repo, RevWalk rw, PersonIdent ident, String externalId) throws IOException {
+ return insertExternalId(
+ repo,
+ rw,
+ ident,
+ (ins, noteMap) -> {
+ ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
+ byte[] raw = "".getBytes(UTF_8);
+ ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
+ noteMap.set(noteId, dataBlob);
+ return noteId;
+ });
+ }
+
+ private static String insertExternalId(
+ Repository repo, RevWalk rw, PersonIdent ident, ExternalIdInserter extIdInserter)
+ throws IOException {
+ ObjectId rev = ExternalIdReader.readRevision(repo);
+ NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
+
+ try (ObjectInserter ins = repo.newObjectInserter()) {
+ ObjectId noteId = extIdInserter.addNote(ins, noteMap);
+
+ CommitBuilder cb = new CommitBuilder();
+ cb.setMessage("Update external IDs");
+ cb.setTreeId(noteMap.writeTree(ins));
+ cb.setAuthor(ident);
+ cb.setCommitter(ident);
+ if (!rev.equals(ObjectId.zeroId())) {
+ cb.setParentId(rev);
+ } else {
+ cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
+ }
+ if (cb.getTreeId() == null) {
+ if (rev.equals(ObjectId.zeroId())) {
+ cb.setTreeId(ins.insert(OBJ_TREE, new byte[] {})); // No parent, assume empty tree.
+ } else {
+ RevCommit p = rw.parseCommit(rev);
+ cb.setTreeId(p.getTree()); // Copy tree from parent.
+ }
+ }
+ ObjectId commitId = ins.insert(cb);
+ ins.flush();
+
+ RefUpdate u = repo.updateRef(RefNames.REFS_EXTERNAL_IDS);
+ u.setExpectedOldObjectId(rev);
+ u.setNewObjectId(commitId);
+ RefUpdate.Result res = u.update();
+ switch (res) {
+ case NEW:
+ case FAST_FORWARD:
+ case NO_CHANGE:
+ case RENAMED:
+ case FORCED:
+ break;
+ case LOCK_FAILURE:
+ case IO_FAILURE:
+ case NOT_ATTEMPTED:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
+ default:
+ throw new IOException("Updating external IDs failed with " + res);
+ }
+ return noteId.getName();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/change/ChangeETagComputation.java b/java/com/google/gerrit/server/change/ChangeETagComputation.java
index 5f03200..463989b 100644
--- a/java/com/google/gerrit/server/change/ChangeETagComputation.java
+++ b/java/com/google/gerrit/server/change/ChangeETagComputation.java
@@ -50,6 +50,11 @@
* <p><strong>Note:</strong> Change ETags are computed very frequently and the computation must be
* cheap. Take good care to not perform any expensive computations when implementing this.
*
+ * <p>If an error is encountered during the ETag computation the plugin can indicate this by
+ * throwing any RuntimeException. In this case no value will be included in the change ETag
+ * computation. This means if the error is transient, the ETag will differ when the computation
+ * succeeds on a follow-up run.
+ *
* @param projectName the name of the project that contains the change
* @param changeId ID of the change for which the ETag should be computed
* @return the ETag
diff --git a/java/com/google/gerrit/server/change/ChangeResource.java b/java/com/google/gerrit/server/change/ChangeResource.java
index bb0040d..d8d82c6 100644
--- a/java/com/google/gerrit/server/change/ChangeResource.java
+++ b/java/com/google/gerrit/server/change/ChangeResource.java
@@ -197,6 +197,14 @@
for (ProjectState p : projectStateTree) {
hashObjectId(h, p.getConfig().getRevision(), buf);
}
+
+ changeETagComputation.runEach(
+ c -> {
+ String pluginETag = c.getETag(notes.getProjectName(), notes.getChangeId());
+ if (pluginETag != null) {
+ h.putString(pluginETag, UTF_8);
+ }
+ });
}
@Override
@@ -206,13 +214,6 @@
h.putString(starredChangesUtil.getObjectId(user.getAccountId(), getId()).name(), UTF_8);
}
prepareETag(h, user);
- changeETagComputation.runEach(
- c -> {
- String pluginETag = c.getETag(notes.getProjectName(), notes.getChangeId());
- if (pluginETag != null) {
- h.putString(pluginETag, UTF_8);
- }
- });
return h.hash().toString();
}
diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
index 5e1f178..a9a49bf 100644
--- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java
+++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java
@@ -481,11 +481,7 @@
change.setCurrentPatchSet(info);
List<String> idList = commit.getFooterLines(CHANGE_ID);
- if (idList.isEmpty()) {
- change.setKey(Change.key("I" + commitId.name()));
- } else {
- change.setKey(Change.key(idList.get(idList.size() - 1).trim()));
- }
+ change.setKey(Change.key(idList.get(idList.size() - 1).trim()));
}
private List<Comment> publishComments(ChangeContext ctx, boolean workInProgress) {
diff --git a/java/com/google/gerrit/server/logging/Metadata.java b/java/com/google/gerrit/server/logging/Metadata.java
index 400f48f..7eba4de 100644
--- a/java/com/google/gerrit/server/logging/Metadata.java
+++ b/java/com/google/gerrit/server/logging/Metadata.java
@@ -86,9 +86,12 @@
// The name of the implementation method.
public abstract Optional<String> methodName();
- // Boolean: one or more
+ // One or more resources
public abstract Optional<Boolean> multiple();
+ // Partial or full computation
+ public abstract Optional<Boolean> partial();
+
// Path of a metadata file in NoteDb.
public abstract Optional<String> noteDbFilePath();
@@ -182,6 +185,8 @@
public abstract Builder multiple(boolean multiple);
+ public abstract Builder partial(boolean partial);
+
public abstract Builder noteDbFilePath(@Nullable String noteDbFilePath);
public abstract Builder noteDbRefName(@Nullable String noteDbRefName);
diff --git a/java/com/google/gerrit/server/plugincontext/PluginContext.java b/java/com/google/gerrit/server/plugincontext/PluginContext.java
index 266eb92..90d56c8 100644
--- a/java/com/google/gerrit/server/plugincontext/PluginContext.java
+++ b/java/com/google/gerrit/server/plugincontext/PluginContext.java
@@ -272,7 +272,7 @@
Throwables.throwIfUnchecked(e);
pluginMetrics.incrementErrorCount(extension);
logger.atWarning().withCause(e).log(
- "Failure in %s of plugin invoke%s", extensionImpl.getClass(), extension.getPluginName());
+ "Failure in %s of plugin %s", extensionImpl.getClass(), extension.getPluginName());
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/PutMessage.java b/java/com/google/gerrit/server/restapi/change/PutMessage.java
index db02418..f05641f 100644
--- a/java/com/google/gerrit/server/restapi/change/PutMessage.java
+++ b/java/com/google/gerrit/server/restapi/change/PutMessage.java
@@ -111,10 +111,13 @@
String sanitizedCommitMessage = CommitMessageUtil.checkAndSanitizeCommitMessage(input.message);
ensureCanEditCommitMessage(resource.getNotes());
- ensureChangeIdIsCorrect(
- projectCache.checkedGet(resource.getProject()).is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
- resource.getChange().getKey().get(),
- sanitizedCommitMessage);
+ sanitizedCommitMessage =
+ ensureChangeIdIsCorrect(
+ projectCache
+ .checkedGet(resource.getProject())
+ .is(BooleanProjectConfig.REQUIRE_CHANGE_ID),
+ resource.getChange().getKey().get(),
+ sanitizedCommitMessage);
try (Repository repository = repositoryManager.openRepository(resource.getProject());
RevWalk revWalk = new RevWalk(repository);
@@ -193,7 +196,7 @@
}
}
- private static void ensureChangeIdIsCorrect(
+ private static String ensureChangeIdIsCorrect(
boolean requireChangeId, String currentChangeId, String newCommitMessage)
throws ResourceConflictException, BadRequestException {
RevCommit revCommit =
@@ -204,14 +207,21 @@
CommitMessageUtil.checkAndSanitizeCommitMessage(revCommit.getShortMessage());
List<String> changeIdFooters = revCommit.getFooterLines(FooterConstants.CHANGE_ID);
- if (requireChangeId && changeIdFooters.isEmpty()) {
- throw new ResourceConflictException("missing Change-Id footer");
- }
if (!changeIdFooters.isEmpty() && !changeIdFooters.get(0).equals(currentChangeId)) {
throw new ResourceConflictException("wrong Change-Id footer");
}
- if (changeIdFooters.size() > 1) {
+
+ if (requireChangeId && revCommit.getFooterLines().isEmpty()) {
+ // sanitization always adds '\n' at the end.
+ newCommitMessage += "\n";
+ }
+
+ if (requireChangeId && changeIdFooters.isEmpty()) {
+ newCommitMessage += FooterConstants.CHANGE_ID.getName() + ": " + currentChangeId + "\n";
+ } else if (changeIdFooters.size() > 1) {
throw new ResourceConflictException("multiple Change-Id footers");
}
+
+ return newCommitMessage;
}
}
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index 695feba..f50f857 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -291,7 +291,7 @@
.addTag(RequestId.Type.TRACE_ID, "retry-on-failure-" + new RequestId())
.forceLogging();
logger.atFine().withCause(t).log(
- "%s failed, retry with tracing eanbled",
+ "%s failed, retry with tracing enabled",
opts.caller().map(Class::getSimpleName).orElse("N/A"));
return true;
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 5c2f033..be4162e 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -87,6 +87,7 @@
import com.google.gerrit.common.data.LabelFunction;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.annotations.Exports;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.AddReviewerResult;
@@ -2183,6 +2184,24 @@
}
@Test
+ public void throwingExceptionFromETagComputationDoesNotBreakGerrit() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String oldETag = parseResource(r).getETag();
+
+ RegistrationHandle registrationHandle =
+ changeETagComputations.add(
+ "gerrit",
+ (p, id) -> {
+ throw new StorageException("exception during test");
+ });
+ try {
+ assertThat(parseResource(r).getETag()).isEqualTo(oldETag);
+ } finally {
+ registrationHandle.remove();
+ }
+ }
+
+ @Test
public void emailNotificationForFileLevelComment() throws Exception {
String changeId = createChange().getChangeId();
@@ -3861,15 +3880,13 @@
}
@Test
- public void changeCommitMessageWithNoChangeIdFails() throws Exception {
+ public void changeCommitMessageWithNoChangeIdRetainsChangeID() throws Exception {
PushOneCommit.Result r = createChange();
assertThat(getCommitMessage(r.getChangeId()))
.isEqualTo("test commit\n\nChange-Id: " + r.getChangeId() + "\n");
- ResourceConflictException thrown =
- assertThrows(
- ResourceConflictException.class,
- () -> gApi.changes().id(r.getChangeId()).setMessage("modified commit\n"));
- assertThat(thrown).hasMessageThat().contains("missing Change-Id footer");
+ gApi.changes().id(r.getChangeId()).setMessage("modified commit\n");
+ assertThat(getCommitMessage(r.getChangeId()))
+ .isEqualTo("modified commit\n\nChange-Id: " + r.getChangeId() + "\n");
}
@Test
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/BUILD b/javatests/com/google/gerrit/acceptance/rest/account/BUILD
index 3b46414..17a6053 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/BUILD
+++ b/javatests/com/google/gerrit/acceptance/rest/account/BUILD
@@ -4,7 +4,10 @@
srcs = glob(["*IT.java"]),
group = "rest_account",
labels = ["rest"],
- deps = [":util"],
+ deps = [
+ ":util",
+ "//java/com/google/gerrit/server/account/externalids/testing",
+ ],
)
java_library(
@@ -18,6 +21,7 @@
deps = [
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/reviewdb:server",
+ "//java/com/google/gerrit/server/account/externalids/testing",
"//lib:junit",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
index ba3c7ea..e9e8b7f 100644
--- a/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/account/ExternalIdIT.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.acceptance.GitUtil.fetch;
import static com.google.gerrit.acceptance.GitUtil.pushHead;
import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allow;
@@ -23,12 +24,13 @@
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_UUID;
+import static com.google.gerrit.server.account.externalids.testing.ExternalIdTestUtil.insertExternalIdWithEmptyNote;
+import static com.google.gerrit.server.account.externalids.testing.ExternalIdTestUtil.insertExternalIdWithInvalidConfig;
+import static com.google.gerrit.server.account.externalids.testing.ExternalIdTestUtil.insertExternalIdWithKeyThatDoesntMatchNoteId;
+import static com.google.gerrit.server.account.externalids.testing.ExternalIdTestUtil.insertExternalIdWithoutAccountId;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
-import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
-import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
-import static org.eclipse.jgit.lib.Constants.OBJ_TREE;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -58,6 +60,7 @@
import com.google.gerrit.server.account.externalids.ExternalIds;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.testing.ConfigSuite;
import com.google.gson.reflect.TypeToken;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -74,13 +77,8 @@
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterLine;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
@@ -98,6 +96,20 @@
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
+ @ConfigSuite.Default
+ public static Config partialCacheReloadingEnabled() {
+ Config cfg = new Config();
+ cfg.setBoolean("cache", "external_ids_map", "enablePartialReloads", true);
+ return cfg;
+ }
+
+ @ConfigSuite.Config
+ public static Config partialCacheReloadingDisabled() {
+ Config cfg = new Config();
+ cfg.setBoolean("cache", "external_ids_map", "enablePartialReloads", false);
+ return cfg;
+ }
+
@Test
public void getExternalIds() throws Exception {
Collection<ExternalId> expectedIds = getAccountState(user.id()).getExternalIds();
@@ -329,7 +341,11 @@
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
insertExternalIdWithoutAccountId(
- allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), "foo:bar");
+ allUsersRepo.getRepository(),
+ allUsersRepo.getRevWalk(),
+ admin.newIdent(),
+ admin.id(),
+ "foo:bar");
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
allowPushOfExternalIds();
@@ -350,7 +366,11 @@
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
insertExternalIdWithKeyThatDoesntMatchNoteId(
- allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), "foo:bar");
+ allUsersRepo.getRepository(),
+ allUsersRepo.getRevWalk(),
+ admin.newIdent(),
+ admin.id(),
+ "foo:bar");
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
allowPushOfExternalIds();
@@ -370,7 +390,7 @@
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
insertExternalIdWithInvalidConfig(
- allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), "foo:bar");
+ allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), admin.newIdent(), "foo:bar");
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
allowPushOfExternalIds();
@@ -390,7 +410,7 @@
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
insertExternalIdWithEmptyNote(
- allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), "foo:bar");
+ allUsersRepo.getRepository(), allUsersRepo.getRevWalk(), admin.newIdent(), "foo:bar");
allUsersRepo.reset(RefNames.REFS_EXTERNAL_IDS);
allowPushOfExternalIds();
@@ -571,7 +591,8 @@
try (Repository repo = repoManager.openRepository(allUsers);
RevWalk rw = new RevWalk(repo)) {
String externalId = nextId(scheme, i);
- String noteId = insertExternalIdWithoutAccountId(repo, rw, externalId);
+ String noteId =
+ insertExternalIdWithoutAccountId(repo, rw, admin.newIdent(), admin.id(), externalId);
expectedProblems.add(
consistencyError(
"Invalid external ID config for note '"
@@ -581,7 +602,9 @@
+ ".accountId' is missing, expected account ID"));
externalId = nextId(scheme, i);
- noteId = insertExternalIdWithKeyThatDoesntMatchNoteId(repo, rw, externalId);
+ noteId =
+ insertExternalIdWithKeyThatDoesntMatchNoteId(
+ repo, rw, admin.newIdent(), admin.id(), externalId);
expectedProblems.add(
consistencyError(
"Invalid external ID config for note '"
@@ -592,12 +615,12 @@
+ noteId
+ "'"));
- noteId = insertExternalIdWithInvalidConfig(repo, rw, nextId(scheme, i));
+ noteId = insertExternalIdWithInvalidConfig(repo, rw, admin.newIdent(), nextId(scheme, i));
expectedProblems.add(
consistencyError(
"Invalid external ID config for note '" + noteId + "': Invalid line in config file"));
- noteId = insertExternalIdWithEmptyNote(repo, rw, nextId(scheme, i));
+ noteId = insertExternalIdWithEmptyNote(repo, rw, admin.newIdent(), nextId(scheme, i));
expectedProblems.add(
consistencyError(
"Invalid external ID config for note '"
@@ -616,123 +639,6 @@
"password");
}
- private String insertExternalIdWithoutAccountId(Repository repo, RevWalk rw, String externalId)
- throws IOException {
- return insertExternalId(
- repo,
- rw,
- (ins, noteMap) -> {
- ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), admin.id());
- ObjectId noteId = extId.key().sha1();
- Config c = new Config();
- extId.writeToConfig(c);
- c.unset("externalId", extId.key().get(), "accountId");
- byte[] raw = c.toText().getBytes(UTF_8);
- ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
- noteMap.set(noteId, dataBlob);
- return noteId;
- });
- }
-
- private String insertExternalIdWithKeyThatDoesntMatchNoteId(
- Repository repo, RevWalk rw, String externalId) throws IOException {
- return insertExternalId(
- repo,
- rw,
- (ins, noteMap) -> {
- ExternalId extId = ExternalId.create(ExternalId.Key.parse(externalId), admin.id());
- ObjectId noteId = ExternalId.Key.parse(externalId + "x").sha1();
- Config c = new Config();
- extId.writeToConfig(c);
- byte[] raw = c.toText().getBytes(UTF_8);
- ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
- noteMap.set(noteId, dataBlob);
- return noteId;
- });
- }
-
- private String insertExternalIdWithInvalidConfig(Repository repo, RevWalk rw, String externalId)
- throws IOException {
- return insertExternalId(
- repo,
- rw,
- (ins, noteMap) -> {
- ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
- byte[] raw = "bad-config".getBytes(UTF_8);
- ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
- noteMap.set(noteId, dataBlob);
- return noteId;
- });
- }
-
- private String insertExternalIdWithEmptyNote(Repository repo, RevWalk rw, String externalId)
- throws IOException {
- return insertExternalId(
- repo,
- rw,
- (ins, noteMap) -> {
- ObjectId noteId = ExternalId.Key.parse(externalId).sha1();
- byte[] raw = "".getBytes(UTF_8);
- ObjectId dataBlob = ins.insert(OBJ_BLOB, raw);
- noteMap.set(noteId, dataBlob);
- return noteId;
- });
- }
-
- private String insertExternalId(Repository repo, RevWalk rw, ExternalIdInserter extIdInserter)
- throws IOException {
- ObjectId rev = ExternalIdReader.readRevision(repo);
- NoteMap noteMap = ExternalIdReader.readNoteMap(rw, rev);
-
- try (ObjectInserter ins = repo.newObjectInserter()) {
- ObjectId noteId = extIdInserter.addNote(ins, noteMap);
-
- CommitBuilder cb = new CommitBuilder();
- cb.setMessage("Update external IDs");
- cb.setTreeId(noteMap.writeTree(ins));
- cb.setAuthor(admin.newIdent());
- cb.setCommitter(admin.newIdent());
- if (!rev.equals(ObjectId.zeroId())) {
- cb.setParentId(rev);
- } else {
- cb.setParentIds(); // Ref is currently nonexistent, commit has no parents.
- }
- if (cb.getTreeId() == null) {
- if (rev.equals(ObjectId.zeroId())) {
- cb.setTreeId(ins.insert(OBJ_TREE, new byte[] {})); // No parent, assume empty tree.
- } else {
- RevCommit p = rw.parseCommit(rev);
- cb.setTreeId(p.getTree()); // Copy tree from parent.
- }
- }
- ObjectId commitId = ins.insert(cb);
- ins.flush();
-
- RefUpdate u = repo.updateRef(RefNames.REFS_EXTERNAL_IDS);
- u.setExpectedOldObjectId(rev);
- u.setNewObjectId(commitId);
- RefUpdate.Result res = u.update();
- switch (res) {
- case NEW:
- case FAST_FORWARD:
- case NO_CHANGE:
- case RENAMED:
- case FORCED:
- break;
- case LOCK_FAILURE:
- case IO_FAILURE:
- case NOT_ATTEMPTED:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- default:
- throw new IOException("Updating external IDs failed with " + res);
- }
- return noteId.getName();
- }
- }
-
private ExternalId createExternalIdForNonExistingAccount(String externalId) {
return ExternalId.create(ExternalId.Key.parse(externalId), Account.id(1));
}
@@ -803,6 +709,8 @@
@Test
public void byAccountFailIfReadingExternalIdsFails() throws Exception {
+ assume().that(isPartialCacheReloadingEnabled()).isFalse();
+
try (AutoCloseable ctx = createFailOnLoadContext()) {
// update external ID branch so that external IDs need to be reloaded
insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id()));
@@ -813,6 +721,8 @@
@Test
public void byEmailFailIfReadingExternalIdsFails() throws Exception {
+ assume().that(isPartialCacheReloadingEnabled()).isFalse();
+
try (AutoCloseable ctx = createFailOnLoadContext()) {
// update external ID branch so that external IDs need to be reloaded
insertExtIdBehindGerritsBack(ExternalId.create("foo", "bar", admin.id()));
@@ -954,6 +864,10 @@
}
}
+ private boolean isPartialCacheReloadingEnabled() {
+ return cfg.getBoolean("cache", "external_ids_map", "enablePartialReloads", true);
+ }
+
private void insertExtId(ExternalId extId) throws Exception {
accountsUpdateProvider
.get()
@@ -1038,9 +952,4 @@
externalIdReader.setFailOnLoad(true);
return () -> externalIdReader.setFailOnLoad(false);
}
-
- @FunctionalInterface
- private interface ExternalIdInserter {
- public ObjectId addNote(ObjectInserter ins, NoteMap noteMap) throws IOException;
- }
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
index 5fb42da..ae212b6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ActionsIT.java
@@ -25,6 +25,7 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.TestProjectInput;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.api.changes.ActionVisitor;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.client.ListChangesOption;
@@ -36,6 +37,7 @@
import com.google.gerrit.extensions.registration.RegistrationHandle;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.change.ChangeETagComputation;
import com.google.gerrit.server.change.RevisionJson;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.ConfigSuite;
@@ -58,6 +60,7 @@
@Inject private DynamicSet<ActionVisitor> actionVisitors;
@Inject private RequestScopeOperations requestScopeOperations;
@Inject private RevisionJson.Factory revisionJsonFactory;
+ @Inject private DynamicSet<ChangeETagComputation> changeETagComputations;
private RegistrationHandle visitorHandle;
@@ -206,6 +209,52 @@
}
@Test
+ public void pluginCanContributeToETagComputation() throws Exception {
+ String change = createChange().getChangeId();
+ String oldETag = getETag(change);
+
+ RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> "foo");
+ try {
+ assertThat(getETag(change)).isNotEqualTo(oldETag);
+ } finally {
+ registrationHandle.remove();
+ }
+
+ assertThat(getETag(change)).isEqualTo(oldETag);
+ }
+
+ @Test
+ public void returningNullFromETagComputationDoesNotBreakGerrit() throws Exception {
+ String change = createChange().getChangeId();
+ String oldETag = getETag(change);
+
+ RegistrationHandle registrationHandle = changeETagComputations.add("gerrit", (p, id) -> null);
+ try {
+ assertThat(getETag(change)).isEqualTo(oldETag);
+ } finally {
+ registrationHandle.remove();
+ }
+ }
+
+ @Test
+ public void throwingExceptionFromETagComputationDoesNotBreakGerrit() throws Exception {
+ String change = createChange().getChangeId();
+ String oldETag = getETag(change);
+
+ RegistrationHandle registrationHandle =
+ changeETagComputations.add(
+ "gerrit",
+ (p, id) -> {
+ throw new StorageException("exception during test");
+ });
+ try {
+ assertThat(getETag(change)).isEqualTo(oldETag);
+ } finally {
+ registrationHandle.remove();
+ }
+ }
+
+ @Test
public void revisionActionsTwoChangesInTopic_conflicting() throws Exception {
String changeId = createChangeWithTopic().getChangeId();
approve(changeId);
diff --git a/javatests/com/google/gerrit/json/JsonEnumMappingTest.java b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
new file mode 100644
index 0000000..6e57b01
--- /dev/null
+++ b/javatests/com/google/gerrit/json/JsonEnumMappingTest.java
@@ -0,0 +1,84 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.json;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gson.Gson;
+import org.junit.Test;
+
+public class JsonEnumMappingTest {
+
+ // Use the regular, pre-configured Gson object we use throughout the Gerrit server to ensure that
+ // the EnumTypeAdapterFactory is properly set up.
+ private final Gson gson = OutputFormat.JSON.newGson();
+
+ @Test
+ public void nullCanBeWrittenAndParsedBack() {
+ String resultingJson = gson.toJson(null, TestEnum.class);
+ TestEnum value = gson.fromJson(resultingJson, TestEnum.class);
+ assertThat(value).isNull();
+ }
+
+ @Test
+ public void enumValueCanBeWrittenAndParsedBack() {
+ String resultingJson = gson.toJson(TestEnum.ONE, TestEnum.class);
+ TestEnum value = gson.fromJson(resultingJson, TestEnum.class);
+ assertThat(value).isEqualTo(TestEnum.ONE);
+ }
+
+ @Test
+ public void enumValueCanBeParsed() {
+ TestData data = gson.fromJson("{\"value\":\"ONE\"}", TestData.class);
+ assertThat(data.value).isEqualTo(TestEnum.ONE);
+ }
+
+ @Test
+ public void mixedCaseEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"oNe\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void lowerCaseEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"one\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void notExistingEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"FOUR\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ @Test
+ public void emptyEnumValueIsTreatedAsUnset() {
+ TestData data = gson.fromJson("{\"value\":\"\"}", TestData.class);
+ assertThat(data.value).isNull();
+ }
+
+ private static class TestData {
+ TestEnum value;
+
+ public TestData(TestEnum value) {
+ this.value = value;
+ }
+ }
+
+ private enum TestEnum {
+ ONE,
+ TWO
+ }
+}
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 525a416..f6ed5ef 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -51,6 +51,7 @@
"//java/com/google/gerrit/proto/testing",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/account/externalids/testing",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/cache/testing",
"//java/com/google/gerrit/server/group/testing",
@@ -75,6 +76,7 @@
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/mockito",
"//lib/truth",
"//lib/truth:truth-java8-extension",
"//lib/truth:truth-proto-extension",
diff --git a/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java b/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java
new file mode 100644
index 0000000..9cd8023
--- /dev/null
+++ b/javatests/com/google/gerrit/server/account/externalids/ExternalIDCacheLoaderTest.java
@@ -0,0 +1,307 @@
+// Copyright (C) 2019 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.gerrit.server.account.externalids;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
+import static org.mockito.Mockito.when;
+
+import com.google.common.cache.Cache;
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.metrics.DisabledMetricMaker;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.account.externalids.testing.ExternalIdTestUtil;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.testing.InMemoryRepositoryManager;
+import com.google.inject.util.Providers;
+import java.io.IOException;
+import java.util.function.Consumer;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.treewalk.TreeWalk;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class ExternalIDCacheLoaderTest {
+ private static AllUsersName ALL_USERS = new AllUsersName(AllUsersNameProvider.DEFAULT);
+
+ @Mock Cache<ObjectId, AllExternalIds> externalIdCache;
+
+ private ExternalIdCacheLoader loader;
+ private GitRepositoryManager repoManager = new InMemoryRepositoryManager();
+ private ExternalIdReader externalIdReader;
+ private ExternalIdReader externalIdReaderSpy;
+
+ @Before
+ public void setUp() throws Exception {
+ repoManager.createRepository(ALL_USERS).close();
+ externalIdReader = new ExternalIdReader(repoManager, ALL_USERS, new DisabledMetricMaker());
+ externalIdReaderSpy = Mockito.spy(externalIdReader);
+ loader = createLoader(true);
+ }
+
+ @Test
+ public void worksOnSingleCommit() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ assertThat(loader.load(firstState)).isEqualTo(allFromGit(firstState));
+ verify(externalIdReaderSpy, times(1)).all(firstState);
+ }
+
+ @Test
+ public void reloadsSingleUpdateUsingPartialReload() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ ObjectId head = insertExternalId(2, 2);
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(allFromGit(firstState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void reloadsMultipleUpdatesUsingPartialReload() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ insertExternalId(2, 2);
+ insertExternalId(3, 3);
+ ObjectId head = insertExternalId(4, 4);
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(allFromGit(firstState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void reloadsAllExternalIdsWhenNoOldStateIsCached() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ ObjectId head = insertExternalId(2, 2);
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(null);
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verify(externalIdReaderSpy, times(1)).all(head);
+ }
+
+ @Test
+ public void partialReloadingDisabledAlwaysTriggersFullReload() throws Exception {
+ loader = createLoader(false);
+ insertExternalId(1, 1);
+ ObjectId head = insertExternalId(2, 2);
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verify(externalIdReaderSpy, times(1)).all(head);
+ }
+
+ @Test
+ public void fallsBackToFullReloadOnManyUpdatesOnBranch() throws Exception {
+ insertExternalId(1, 1);
+ ObjectId head = null;
+ for (int i = 2; i < 20; i++) {
+ head = insertExternalId(i, i);
+ }
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verify(externalIdReaderSpy, times(1)).all(head);
+ }
+
+ @Test
+ public void doesFullReloadWhenNoCacheStateIsFound() throws Exception {
+ ObjectId head = insertExternalId(1, 1);
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verify(externalIdReaderSpy, times(1)).all(head);
+ }
+
+ @Test
+ public void handlesDeletionInPartialReload() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ ObjectId head = deleteExternalId(1, 1);
+ assertThat(allFromGit(head).byAccount().size()).isEqualTo(0);
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(allFromGit(firstState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void handlesModifyInPartialReload() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ ObjectId head =
+ modifyExternalId(
+ externalId(1, 1),
+ ExternalId.create("fooschema", "bar1", Account.id(1), "foo@bar.com", "password"));
+ assertThat(allFromGit(head).byAccount().size()).isEqualTo(1);
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(allFromGit(firstState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void ignoresInvalidExternalId() throws Exception {
+ ObjectId firstState = insertExternalId(1, 1);
+ ObjectId head;
+ try (Repository repo = repoManager.openRepository(ALL_USERS);
+ RevWalk rw = new RevWalk(repo)) {
+ ExternalIdTestUtil.insertExternalIdWithKeyThatDoesntMatchNoteId(
+ repo, rw, new PersonIdent("foo", "foo@bar.com"), Account.id(2), "test");
+ head = repo.exactRef(RefNames.REFS_EXTERNAL_IDS).getObjectId();
+ }
+
+ when(externalIdCache.getIfPresent(firstState)).thenReturn(allFromGit(firstState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void handlesTreePrefixesInDifferentialReload() throws Exception {
+ // Create more than 256 notes (NoteMap's current sharding limit) and check that we really have
+ // created a situation where NoteNames are sharded.
+ ObjectId oldState = inserExternalIds(257);
+ assertAllFilesHaveSlashesInPath();
+ ObjectId head = insertExternalId(500, 500);
+
+ when(externalIdCache.getIfPresent(oldState)).thenReturn(allFromGit(oldState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ @Test
+ public void handlesReshard() throws Exception {
+ // Create 256 notes (NoteMap's current sharding limit) and check that we are not yet sharding
+ ObjectId oldState = inserExternalIds(256);
+ assertNoFilesHaveSlashesInPath();
+ // Create one more external ID and then have the Loader compute the new state
+ ObjectId head = insertExternalId(500, 500);
+ assertAllFilesHaveSlashesInPath(); // NoteMap resharded
+
+ when(externalIdCache.getIfPresent(oldState)).thenReturn(allFromGit(oldState));
+
+ assertThat(loader.load(head)).isEqualTo(allFromGit(head));
+ verifyZeroInteractions(externalIdReaderSpy);
+ }
+
+ private ExternalIdCacheLoader createLoader(boolean allowPartial) {
+ Config cfg = new Config();
+ cfg.setBoolean("cache", "external_ids_map", "enablePartialReloads", allowPartial);
+ return new ExternalIdCacheLoader(
+ repoManager,
+ ALL_USERS,
+ externalIdReaderSpy,
+ Providers.of(externalIdCache),
+ new DisabledMetricMaker(),
+ cfg);
+ }
+
+ private AllExternalIds allFromGit(ObjectId revision) throws Exception {
+ return AllExternalIds.create(externalIdReader.all(revision));
+ }
+
+ private ObjectId inserExternalIds(int numberOfIdsToInsert) throws Exception {
+ ObjectId oldState = null;
+ // Create more than 256 notes (NoteMap's current sharding limit) and check that we really have
+ // created a situation where NoteNames are sharded.
+ for (int i = 0; i < numberOfIdsToInsert; i++) {
+ oldState = insertExternalId(i, i);
+ }
+ return oldState;
+ }
+
+ private ObjectId insertExternalId(int key, int accountId) throws Exception {
+ return performExternalIdUpdate(
+ u -> {
+ try {
+ u.insert(externalId(key, accountId));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ private ObjectId modifyExternalId(ExternalId oldId, ExternalId newId) throws Exception {
+ return performExternalIdUpdate(
+ u -> {
+ try {
+ u.replace(oldId, newId);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
+ });
+ }
+
+ private ObjectId deleteExternalId(int key, int accountId) throws Exception {
+ return performExternalIdUpdate(u -> u.delete(externalId(key, accountId)));
+ }
+
+ private ExternalId externalId(int key, int accountId) {
+ return ExternalId.create("fooschema", "bar" + key, Account.id(accountId));
+ }
+
+ private ObjectId performExternalIdUpdate(Consumer<ExternalIdNotes> update) throws Exception {
+ try (Repository repo = repoManager.openRepository(ALL_USERS)) {
+ PersonIdent updater = new PersonIdent("Foo bar", "foo@bar.com");
+ ExternalIdNotes extIdNotes = ExternalIdNotes.loadNoCacheUpdate(ALL_USERS, repo);
+ update.accept(extIdNotes);
+ try (MetaDataUpdate metaDataUpdate =
+ new MetaDataUpdate(GitReferenceUpdated.DISABLED, null, repo)) {
+ metaDataUpdate.getCommitBuilder().setAuthor(updater);
+ metaDataUpdate.getCommitBuilder().setCommitter(updater);
+ return extIdNotes.commit(metaDataUpdate).getId();
+ }
+ }
+ }
+
+ private void assertAllFilesHaveSlashesInPath() throws Exception {
+ assertThat(allFilesInExternalIdRef().stream().allMatch(f -> f.contains("/"))).isTrue();
+ }
+
+ private void assertNoFilesHaveSlashesInPath() throws Exception {
+ assertThat(allFilesInExternalIdRef().stream().noneMatch(f -> f.contains("/"))).isTrue();
+ }
+
+ private ImmutableList<String> allFilesInExternalIdRef() throws Exception {
+ try (Repository repo = repoManager.openRepository(ALL_USERS);
+ TreeWalk treeWalk = new TreeWalk(repo);
+ RevWalk rw = new RevWalk(repo)) {
+ treeWalk.reset(
+ rw.parseCommit(repo.exactRef(RefNames.REFS_EXTERNAL_IDS).getObjectId()).getTree());
+ treeWalk.setRecursive(true);
+ ImmutableList.Builder<String> allPaths = ImmutableList.builder();
+ while (treeWalk.next()) {
+ allPaths.add(treeWalk.getPathString());
+ }
+ return allPaths.build();
+ }
+ }
+}
diff --git a/plugins/plugin-manager b/plugins/plugin-manager
index f681e7e..833889d 160000
--- a/plugins/plugin-manager
+++ b/plugins/plugin-manager
@@ -1 +1 @@
-Subproject commit f681e7ecb6ddbc52fe9e07cf3672ccdcad7d7d0b
+Subproject commit 833889d327a159b5ccea7064f4fcff3f94d4b26e
diff --git a/plugins/webhooks b/plugins/webhooks
index 8078749..f860a0c 160000
--- a/plugins/webhooks
+++ b/plugins/webhooks
@@ -1 +1 @@
-Subproject commit 8078749bfd64d9dcd3372bc3f8965d192747c5b4
+Subproject commit f860a0cf6931164a6e5c2b333eaa0004ea14acec
diff --git a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
index 4d7fd0c..5b728eb 100644
--- a/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
+++ b/polygerrit-ui/app/elements/admin/gr-admin-view/gr-admin-view.js
@@ -198,10 +198,6 @@
this.reload();
},
- _computeSelectedTitle(params) {
- return this.getSelectedTitle(params.view);
- },
-
// TODO (beckysiegel): Update these functions after router abstraction is
// updated. They are currently copied from gr-dropdown (and should be
// updated there as well once complete).
@@ -228,6 +224,7 @@
* @param {string=} opt_detailType
*/
_computeSelectedClass(itemView, params, opt_detailType) {
+ if (!params) return '';
// Group params are structured differently from admin params. Compute
// selected differently for groups.
// TODO(wyatta): Simplify this when all routes work like group params.
diff --git a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
index 08b59c2..fded936 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
+++ b/polygerrit-ui/app/elements/admin/gr-repo/gr-repo.js
@@ -309,6 +309,9 @@
},
_computeCommands(repo, schemesObj, _selectedScheme) {
+ if (!schemesObj || !repo || !_selectedScheme) {
+ return [];
+ }
const commands = [];
let commandObj;
if (schemesObj.hasOwnProperty(_selectedScheme)) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
index e69d48f..1e50e4b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.html
@@ -176,7 +176,7 @@
on-cancel="_handleConfirmDialogCancel"
branch="[[change.branch]]"
has-parent="[[hasParent]]"
- rebase-on-current="[[revisionActions.rebase.rebaseOnCurrent]]"
+ rebase-on-current="[[_revisionRebaseAction.rebaseOnCurrent]]"
hidden></gr-confirm-rebase-dialog>
<gr-confirm-cherrypick-dialog id="confirmCherrypick"
class="confirmDialog"
@@ -216,7 +216,7 @@
id="confirmSubmitDialog"
class="confirmDialog"
change="[[change]]"
- action="[[revisionActions.submit]]"
+ action="[[_revisionSubmitAction]]"
on-cancel="_handleConfirmDialogCancel"
on-confirm="_handleSubmitConfirm" hidden></gr-confirm-submit-dialog>
<gr-dialog id="createFollowUpDialog"
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
index 83a454b..a448377 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -271,6 +271,20 @@
type: Object,
value() { return {}; },
},
+ // If property binds directly to [[revisionActions.submit]] it is not
+ // updated when revisionActions doesn't contain submit action.
+ /** @type {?} */
+ _revisionSubmitAction: {
+ type: Object,
+ computed: '_getSubmitAction(revisionActions)',
+ },
+ // If property binds directly to [[revisionActions.rebase]] it is not
+ // updated when revisionActions doesn't contain rebase action.
+ /** @type {?} */
+ _revisionRebaseAction: {
+ type: Object,
+ computed: '_getRebaseAction(revisionActions)',
+ },
privateByDefault: String,
_loading: {
@@ -415,6 +429,28 @@
this._handleLoadingComplete();
},
+ _getSubmitAction(revisionActions) {
+ return this._getRevisionAction(revisionActions, 'submit', null);
+ },
+
+ _getRebaseAction(revisionActions) {
+ return this._getRevisionAction(revisionActions, 'rebase',
+ {rebaseOnCurrent: null}
+ );
+ },
+
+ _getRevisionAction(revisionActions, actionName, emptyActionValue) {
+ if (!revisionActions) {
+ return undefined;
+ }
+ if (revisionActions[actionName] === undefined) {
+ // Return null to fire an event when reveisionActions was loaded
+ // but doesn't contain actionName. undefined doesn't fire an event
+ return emptyActionValue;
+ }
+ return revisionActions[actionName];
+ },
+
reload() {
if (!this.changeNum || !this.latestPatchNum) {
return Promise.resolve();
@@ -575,7 +611,9 @@
_deleteAndNotify(actionName) {
if (this.actions[actionName]) {
delete this.actions[actionName];
- this.notifyPath('actions.' + actionName);
+ // We assign a fake value of 'false' to support Polymer 2
+ // see https://github.com/Polymer/polymer/issues/2631
+ this.notifyPath('actions.' + actionName, false);
}
},
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
index 184e175..b88e06b 100644
--- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -1513,4 +1513,50 @@
assert.equal(reportStub.lastCall.args[0], 'type-key');
});
});
+
+ suite('getChangeRevisionActions returns only some actions', () => {
+ let element;
+ let sandbox;
+ let changeRevisionActions;
+
+ setup(() => {
+ stub('gr-rest-api-interface', {
+ getChangeRevisionActions() {
+ return Promise.resolve(changeRevisionActions);
+ },
+ send(method, url, payload) {
+ return Promise.reject(new Error('error'));
+ },
+ getProjectConfig() { return Promise.resolve({}); },
+ });
+
+ sandbox = sinon.sandbox.create();
+ sandbox.stub(Gerrit, 'awaitPluginsLoaded').returns(Promise.resolve());
+
+ element = fixture('basic');
+ // getChangeRevisionActions is not called without
+ // set the following properies
+ element.change = {};
+ element.changeNum = '42';
+ element.latestPatchNum = '2';
+
+
+ sandbox.stub(element.$.confirmCherrypick.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
+ sandbox.stub(element.$.confirmMove.$.restAPI,
+ 'getRepoBranches').returns(Promise.resolve([]));
+ return element.reload();
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('confirmSubmitDialog and confirmRebase properties are changed', () => {
+ changeRevisionActions = {};
+ element.reload();
+ assert.strictEqual(element.$.confirmSubmitDialog.action, null);
+ assert.strictEqual(element.$.confirmRebase.rebaseOnCurrent, null);
+ });
+ });
</script>
diff --git a/polygerrit-ui/app/elements/gr-app-element.html b/polygerrit-ui/app/elements/gr-app-element.html
index aff71f7..46c4502 100644
--- a/polygerrit-ui/app/elements/gr-app-element.html
+++ b/polygerrit-ui/app/elements/gr-app-element.html
@@ -196,7 +196,9 @@
<div>
<a class="feedback"
href$="[[_feedbackUrl]]"
- rel="noopener" target="_blank">Report bug</a>
+ rel="noopener"
+ target="_blank"
+ hidden$="[[!_showFeedbackUrl(_feedbackUrl)]]">Report bug</a>
| Press “?” for keyboard shortcuts
<gr-endpoint-decorator name="footer-right"></gr-endpoint-decorator>
</div>
diff --git a/polygerrit-ui/app/elements/gr-app-element.js b/polygerrit-ui/app/elements/gr-app-element.js
index 4d556ae..eb35878 100644
--- a/polygerrit-ui/app/elements/gr-app-element.js
+++ b/polygerrit-ui/app/elements/gr-app-element.js
@@ -78,11 +78,7 @@
computed: '_computePluginScreenName(params)',
},
_settingsUrl: String,
- _feedbackUrl: {
- type: String,
- value: 'https://bugs.chromium.org/p/gerrit/issues/entry' +
- '?template=PolyGerrit%20Issue',
- },
+ _feedbackUrl: String,
// Used to allow searching on mobile
mobileSearch: {
type: Boolean,
@@ -441,5 +437,13 @@
_mobileSearchToggle(e) {
this.mobileSearch = !this.mobileSearch;
},
+
+ _showFeedbackUrl(feedbackUrl) {
+ if (feedbackUrl) {
+ return feedbackUrl;
+ }
+
+ return false;
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/settings/gr-identities/gr-identities.html b/polygerrit-ui/app/elements/settings/gr-identities/gr-identities.html
index be851b1..a876611 100644
--- a/polygerrit-ui/app/elements/settings/gr-identities/gr-identities.html
+++ b/polygerrit-ui/app/elements/settings/gr-identities/gr-identities.html
@@ -27,8 +27,20 @@
<template>
<style include="shared-styles"></style>
<style include="gr-form-styles">
- td {
- width: 5em;
+ tr th.emailAddressHeader,
+ tr th.identityHeader {
+ width: 15em;
+ padding: 0 10px;
+ }
+ tr td.statusColumn,
+ tr td.emailAddressColumn,
+ tr td.identityColumn {
+ word-break: break-word;
+ }
+ tr td.emailAddressColumn,
+ tr td.identityColumn {
+ padding: 4px 10px;
+ width: 15em;
}
.deleteButton {
float: right;
@@ -36,40 +48,38 @@
.deleteButton:not(.show) {
display: none;
}
- .statusColumn {
- white-space: nowrap;
- }
</style>
<div class="gr-form-styles">
- <table id="identities">
- <thead>
- <tr>
- <th class="statusHeader">Status</th>
- <th class="emailAddressHeader">Email Address</th>
- <th class="identityHeader">Identity</th>
- <th class="deleteHeader"></th>
- </tr>
- </thead>
- <tbody>
- <template is="dom-repeat" items="[[_identities]]" filter="filterIdentities">
+ <fieldset>
+ <table>
+ <thead>
<tr>
- <td class$="statusColumn">
- [[_computeIsTrusted(item.trusted)]]
- </td>
- <td>[[item.email_address]]</td>
- <td>[[_computeIdentity(item.identity)]]</td>
- <td>
- <gr-button
- link
- class$="deleteButton [[_computeHideDeleteClass(item.can_delete)]]"
- on-tap="_handleDeleteItem">
- Delete
- </gr-button>
- </td>
+ <th class="statusHeader">Status</th>
+ <th class="emailAddressHeader">Email Address</th>
+ <th class="identityHeader">Identity</th>
+ <th class="deleteHeader"></th>
</tr>
- </template>
- </tbody>
- </table>
+ </thead>
+ <tbody>
+ <template is="dom-repeat" items="[[_identities]]" filter="filterIdentities">
+ <tr>
+ <td class="statusColumn">
+ [[_computeIsTrusted(item.trusted)]]
+ </td>
+ <td class="emailAddressColumn">[[item.email_address]]</td>
+ <td class="identityColumn">[[_computeIdentity(item.identity)]]</td>
+ <td class="deleteColumn">
+ <gr-button
+ class$="deleteButton [[_computeHideDeleteClass(item.can_delete)]]"
+ on-tap="_handleDeleteItem">
+ Delete
+ </gr-button>
+ </td>
+ </tr>
+ </template>
+ </tbody>
+ </table>
+ </fieldset>
</div>
<gr-overlay id="overlay" with-backdrop>
<gr-confirm-delete-item-dialog
diff --git a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
index 53d05e1..2508196 100644
--- a/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
+++ b/polygerrit-ui/app/elements/shared/gr-list-view/gr-list-view.js
@@ -90,11 +90,11 @@
},
_hideNextArrow(loading, items) {
- let lastPage = false;
- if (items.length < this.itemsPerPage + 1) {
- lastPage = true;
+ if (loading || !items || !items.length) {
+ return true;
}
- return loading || lastPage || !items || !items.length;
+ const lastPage = items.length < this.itemsPerPage + 1;
+ return lastPage;
},
});
})();
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 9448662..2b052a0 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -197,7 +197,6 @@
for (let file of elements) {
file = elementsPath + file;
testFiles.push(file);
- testFiles.push(file + '?dom=shadow');
}
// Behaviors tests.
