Merge "VersionedAccountDestinations: Remove unused createSink(String) method"
diff --git a/gerrit-gwtui/BUILD b/gerrit-gwtui/BUILD
index a6c9763..56ac0ea 100644
--- a/gerrit-gwtui/BUILD
+++ b/gerrit-gwtui/BUILD
@@ -34,8 +34,8 @@
"//java/com/google/gerrit/common:client",
"//java/com/google/gerrit/extensions:client",
"//lib:junit",
- "//lib:truth",
"//lib/gwt:dev",
"//lib/gwt:user",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index acd5130a..9587860 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -76,9 +76,8 @@
"//java/com/google/gerrit/server/group/testing",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
"//lib:jimfs",
- "//lib:truth",
- "//lib:truth-java8-extension",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/httpcomponents:fluent-hc",
@@ -88,6 +87,8 @@
"//lib/jgit/org.eclipse.jgit.junit:junit",
"//lib/log:impl_log4j",
"//lib/log:log4j",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
"//prolog:gerrit-prolog-common",
],
visibility = ["//visibility:public"],
diff --git a/java/com/google/gerrit/common/data/testing/BUILD b/java/com/google/gerrit/common/data/testing/BUILD
index 83f1c06..3899e39 100644
--- a/java/com/google/gerrit/common/data/testing/BUILD
+++ b/java/com/google/gerrit/common/data/testing/BUILD
@@ -6,6 +6,6 @@
deps = [
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/reviewdb:server",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/extensions/common/testing/BUILD b/java/com/google/gerrit/extensions/common/testing/BUILD
index 82dd425..94fecbf 100644
--- a/java/com/google/gerrit/extensions/common/testing/BUILD
+++ b/java/com/google/gerrit/extensions/common/testing/BUILD
@@ -6,7 +6,7 @@
deps = [
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/truth",
- "//lib:truth",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/extensions/restapi/testing/BUILD b/java/com/google/gerrit/extensions/restapi/testing/BUILD
index d035816..434591e 100644
--- a/java/com/google/gerrit/extensions/restapi/testing/BUILD
+++ b/java/com/google/gerrit/extensions/restapi/testing/BUILD
@@ -6,6 +6,6 @@
deps = [
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/truth",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/git/testing/BUILD b/java/com/google/gerrit/git/testing/BUILD
index 0b83560..4900339 100644
--- a/java/com/google/gerrit/git/testing/BUILD
+++ b/java/com/google/gerrit/git/testing/BUILD
@@ -7,8 +7,8 @@
deps = [
"//java/com/google/gerrit/common:annotations",
"//lib:guava",
- "//lib:truth",
- "//lib:truth-java8-extension",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
],
)
diff --git a/java/com/google/gerrit/gpg/server/PostGpgKeys.java b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
index e487a54..4b92ec3 100644
--- a/java/com/google/gerrit/gpg/server/PostGpgKeys.java
+++ b/java/com/google/gerrit/gpg/server/PostGpgKeys.java
@@ -259,10 +259,10 @@
if (accountStates.size() > 1) {
StringBuilder msg = new StringBuilder();
- msg.append("GPG key ").append(extIdKey.get()).append(" associated with multiple accounts: ");
- Joiner.on(", ")
- .appendTo(msg, Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
- log.error(msg.toString());
+ msg.append("GPG key ")
+ .append(extIdKey.get())
+ .append(" associated with multiple accounts: ")
+ .append(Lists.transform(accountStates, AccountState.ACCOUNT_ID_FUNCTION));
throw new IllegalStateException(msg.toString());
}
diff --git a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
index 55bd4d5..6174644 100644
--- a/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
+++ b/java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
@@ -167,6 +167,8 @@
rsp.sendError(SC_UNAUTHORIZED);
return false;
} catch (AuthenticationFailedException e) {
+ // This exception is thrown if the user provided wrong credentials, we don't need to log a
+ // stacktrace for it.
log.warn(authenticationFailedMsg(username, req) + ": " + e.getMessage());
rsp.sendError(SC_UNAUTHORIZED);
return false;
diff --git a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
index 5b60a36f..cc22d24 100644
--- a/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
+++ b/java/com/google/gerrit/httpd/gitweb/GitwebServlet.java
@@ -659,7 +659,7 @@
dst.close();
}
} catch (IOException e) {
- log.debug("Unexpected error copying input to CGI", e);
+ log.error("Unexpected error copying input to CGI", e);
}
},
"Gitweb-InputFeeder")
@@ -669,14 +669,19 @@
private void copyStderrToLog(InputStream in) {
new Thread(
() -> {
+ StringBuilder b = new StringBuilder();
try (BufferedReader br =
new BufferedReader(new InputStreamReader(in, ISO_8859_1.name()))) {
String line;
while ((line = br.readLine()) != null) {
- log.error("CGI: " + line);
+ if (b.length() > 0) {
+ b.append('\n');
+ }
+ b.append("CGI: ").append(line);
}
+ log.error(b.toString());
} catch (IOException e) {
- log.debug("Unexpected error copying stderr from CGI", e);
+ log.error("Unexpected error copying stderr from CGI", e);
}
},
"Gitweb-ErrorLogger")
diff --git a/java/com/google/gerrit/httpd/raw/BazelBuild.java b/java/com/google/gerrit/httpd/raw/BazelBuild.java
index 85453fb..f52792c 100644
--- a/java/com/google/gerrit/httpd/raw/BazelBuild.java
+++ b/java/com/google/gerrit/httpd/raw/BazelBuild.java
@@ -17,6 +17,7 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static java.nio.charset.StandardCharsets.UTF_8;
+import com.google.common.base.Joiner;
import com.google.common.escape.Escaper;
import com.google.common.html.HtmlEscapers;
import com.google.common.io.ByteStreams;
@@ -62,7 +63,8 @@
try {
status = rebuild.waitFor();
} catch (InterruptedException e) {
- throw new InterruptedIOException("interrupted waiting for " + proc.toString());
+ throw new InterruptedIOException(
+ "interrupted waiting for: " + Joiner.on(' ').join(proc.command()));
}
if (status != 0) {
log.warn("build failed: " + new String(out, UTF_8));
diff --git a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
index b6eac05..25a28a4 100644
--- a/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
+++ b/java/com/google/gerrit/pgm/http/jetty/JettyServer.java
@@ -69,13 +69,9 @@
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.eclipse.jetty.util.thread.ThreadPool;
import org.eclipse.jgit.lib.Config;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
@Singleton
public class JettyServer {
- private static final Logger log = LoggerFactory.getLogger(JettyServer.class);
-
static class Lifecycle implements LifecycleListener {
private final JettyServer server;
private final Config cfg;
@@ -425,9 +421,8 @@
"/*",
EnumSet.of(DispatcherType.REQUEST, DispatcherType.ASYNC));
} catch (Throwable e) {
- String errorMessage = "Unable to instantiate front-end HTTP Filter " + filterClassName;
- log.error(errorMessage, e);
- throw new IllegalArgumentException(errorMessage, e);
+ throw new IllegalArgumentException(
+ "Unable to instantiate front-end HTTP Filter " + filterClassName, e);
}
}
diff --git a/java/com/google/gerrit/server/cache/testing/BUILD b/java/com/google/gerrit/server/cache/testing/BUILD
index c1293f9..ed412af 100644
--- a/java/com/google/gerrit/server/cache/testing/BUILD
+++ b/java/com/google/gerrit/server/cache/testing/BUILD
@@ -7,7 +7,7 @@
deps = [
"//lib:guava",
"//lib:protobuf",
- "//lib:truth",
"//lib/commons:lang3",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/server/group/testing/BUILD b/java/com/google/gerrit/server/group/testing/BUILD
index 134de78..8b8cd00 100644
--- a/java/com/google/gerrit/server/group/testing/BUILD
+++ b/java/com/google/gerrit/server/group/testing/BUILD
@@ -8,7 +8,8 @@
"//java/com/google/gerrit/common:server",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
- "//lib:truth",
+ "//lib:guava",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewers.java b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
index c344513..65c7db7 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewers.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewers.java
@@ -98,7 +98,6 @@
private final AccountLoader.Factory accountLoaderFactory;
private final Provider<ReviewDb> dbProvider;
private final ChangeData.Factory changeDataFactory;
- private final IdentifiedUser.GenericFactory identifiedUserFactory;
private final Config cfg;
private final ReviewerJson json;
private final NotesMigration migration;
@@ -118,7 +117,6 @@
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RetryHelper retryHelper,
- IdentifiedUser.GenericFactory identifiedUserFactory,
@GerritServerConfig Config cfg,
ReviewerJson json,
NotesMigration migration,
@@ -135,7 +133,6 @@
this.accountLoaderFactory = accountLoaderFactory;
this.dbProvider = db;
this.changeDataFactory = changeDataFactory;
- this.identifiedUserFactory = identifiedUserFactory;
this.cfg = cfg;
this.json = json;
this.migration = migration;
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index f2fe4c2..875d636 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -29,9 +29,10 @@
"//java/com/google/gerrit/server/cache/mem",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/schema",
+ "//lib:guava",
"//lib:gwtorm",
"//lib:h2",
- "//lib:truth",
+ "//lib:junit",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/guice",
@@ -39,5 +40,6 @@
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
"//lib/log:api",
+ "//lib/truth",
],
)
diff --git a/java/com/google/gerrit/truth/BUILD b/java/com/google/gerrit/truth/BUILD
index a0e2ee9..719ddce 100644
--- a/java/com/google/gerrit/truth/BUILD
+++ b/java/com/google/gerrit/truth/BUILD
@@ -4,6 +4,7 @@
srcs = glob(["**/*.java"]),
visibility = ["//visibility:public"],
deps = [
- "//lib:truth",
+ "//lib:guava",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/BUILD b/javatests/com/google/gerrit/acceptance/BUILD
index 234e4be..9246abb 100644
--- a/javatests/com/google/gerrit/acceptance/BUILD
+++ b/javatests/com/google/gerrit/acceptance/BUILD
@@ -6,7 +6,7 @@
deps = [
"//java/com/google/gerrit/acceptance:lib",
"//lib:guava",
- "//lib:truth",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/api/group/BUILD b/javatests/com/google/gerrit/acceptance/api/group/BUILD
index 21294f5..a0b70cc 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/BUILD
+++ b/javatests/com/google/gerrit/acceptance/api/group/BUILD
@@ -21,6 +21,6 @@
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
"//lib:gwtorm",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/rest/project/BUILD b/javatests/com/google/gerrit/acceptance/rest/project/BUILD
index 0720fb3..dad3ca9 100644
--- a/javatests/com/google/gerrit/acceptance/rest/project/BUILD
+++ b/javatests/com/google/gerrit/acceptance/rest/project/BUILD
@@ -18,7 +18,8 @@
],
deps = [
"//java/com/google/gerrit/extensions:api",
- "//lib:truth",
+ "//lib:guava",
+ "//lib/truth",
],
)
@@ -31,8 +32,9 @@
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
+ "//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/common/BUILD b/javatests/com/google/gerrit/common/BUILD
index ff19646..ba9a5bc 100644
--- a/javatests/com/google/gerrit/common/BUILD
+++ b/javatests/com/google/gerrit/common/BUILD
@@ -15,7 +15,7 @@
"//java/com/google/gerrit/common:client",
"//lib:guava",
"//lib:junit",
- "//lib:truth",
+ "//lib/truth",
],
)
@@ -28,8 +28,8 @@
"//java/com/google/gerrit/common:version",
"//java/com/google/gerrit/launcher",
"//lib:guava",
- "//lib:truth",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 70d7089..a2f5229 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -14,10 +14,10 @@
"//lib:gson",
"//lib:guava",
"//lib:junit",
- "//lib:truth",
"//lib/elasticsearch",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/extensions/BUILD b/javatests/com/google/gerrit/extensions/BUILD
index 2557750..069c915 100644
--- a/javatests/com/google/gerrit/extensions/BUILD
+++ b/javatests/com/google/gerrit/extensions/BUILD
@@ -7,7 +7,7 @@
deps = [
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/extensions/common/testing:common-test-util",
- "//lib:truth",
"//lib/guice",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/extensions/conditions/BUILD b/javatests/com/google/gerrit/extensions/conditions/BUILD
index aebe347..e2d5951 100644
--- a/javatests/com/google/gerrit/extensions/conditions/BUILD
+++ b/javatests/com/google/gerrit/extensions/conditions/BUILD
@@ -5,6 +5,6 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/extensions:lib",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/git/testing/BUILD b/javatests/com/google/gerrit/git/testing/BUILD
index 13eb5bf..56e9ec2 100644
--- a/javatests/com/google/gerrit/git/testing/BUILD
+++ b/javatests/com/google/gerrit/git/testing/BUILD
@@ -5,6 +5,6 @@
srcs = glob(["*.java"]),
deps = [
"//java/com/google/gerrit/git/testing",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/gpg/BUILD b/javatests/com/google/gerrit/gpg/BUILD
index 5cc9ae8..ab66f9a 100644
--- a/javatests/com/google/gerrit/gpg/BUILD
+++ b/javatests/com/google/gerrit/gpg/BUILD
@@ -20,7 +20,6 @@
"//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
"//lib/bouncycastle:bcpg",
"//lib/bouncycastle:bcpg-neverlink",
"//lib/bouncycastle:bcprov",
@@ -30,5 +29,6 @@
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
"//lib/log:api",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/httpd/BUILD b/javatests/com/google/gerrit/httpd/BUILD
index e2f2a45..ec2df15 100644
--- a/javatests/com/google/gerrit/httpd/BUILD
+++ b/javatests/com/google/gerrit/httpd/BUILD
@@ -19,11 +19,11 @@
"//lib:junit",
"//lib:servlet-api-3_1-without-neverlink",
"//lib:soy",
- "//lib:truth",
"//lib/easymock",
"//lib/guice",
"//lib/guice:guice-servlet",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/index/BUILD b/javatests/com/google/gerrit/index/BUILD
index bd79860..d905188 100644
--- a/javatests/com/google/gerrit/index/BUILD
+++ b/javatests/com/google/gerrit/index/BUILD
@@ -9,9 +9,10 @@
"//java/com/google/gerrit/index",
"//java/com/google/gerrit/index:query_exception",
"//java/com/google/gerrit/index:query_parser",
+ "//lib:guava",
"//lib:junit",
- "//lib:truth",
"//lib/antlr:java_runtime",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/metrics/proc/BUILD b/javatests/com/google/gerrit/metrics/proc/BUILD
index 8e50cf6..91e5cf6 100644
--- a/javatests/com/google/gerrit/metrics/proc/BUILD
+++ b/javatests/com/google/gerrit/metrics/proc/BUILD
@@ -9,8 +9,8 @@
"//java/com/google/gerrit/lifecycle",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/metrics/dropwizard",
- "//lib:truth",
"//lib/dropwizard:dropwizard-core",
"//lib/guice",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index af0bea6..e4afae2 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -13,11 +13,11 @@
"//java/com/google/gerrit/server",
"//lib:guava",
"//lib:junit",
- "//lib:truth",
"//lib/easymock",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/reviewdb/BUILD b/javatests/com/google/gerrit/reviewdb/BUILD
index a7b9b51..0fd140e 100644
--- a/javatests/com/google/gerrit/reviewdb/BUILD
+++ b/javatests/com/google/gerrit/reviewdb/BUILD
@@ -7,7 +7,8 @@
"//java/com/google/gerrit/reviewdb:client",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 1ab1124..1b11dd65 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -12,7 +12,8 @@
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/truth",
- "//lib:truth",
+ "//lib:guava",
+ "//lib/truth",
],
)
@@ -52,16 +53,18 @@
"//java/org/eclipse/jgit:server",
"//lib:grappa",
"//lib:gson",
+ "//lib:guava",
"//lib:guava-retrying",
"//lib:gwtorm",
"//lib:protobuf",
- "//lib:truth-java8-extension",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
"//lib/commons:codec",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
"//proto:cache_java_proto",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/BUILD b/javatests/com/google/gerrit/server/cache/BUILD
index b173957..278330b 100644
--- a/javatests/com/google/gerrit/server/cache/BUILD
+++ b/javatests/com/google/gerrit/server/cache/BUILD
@@ -9,8 +9,8 @@
"//lib:gwtorm",
"//lib:junit",
"//lib:protobuf",
- "//lib:truth",
"//lib/auto:auto-value",
"//lib/auto:auto-value-annotations",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/cache/h2/BUILD b/javatests/com/google/gerrit/server/cache/h2/BUILD
index e2b9257..63ae94b 100644
--- a/javatests/com/google/gerrit/server/cache/h2/BUILD
+++ b/javatests/com/google/gerrit/server/cache/h2/BUILD
@@ -9,7 +9,7 @@
"//lib:guava",
"//lib:h2",
"//lib:junit",
- "//lib:truth",
"//lib/guice",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/group/db/BUILD b/javatests/com/google/gerrit/server/group/db/BUILD
index 48e8d303..eee5529 100644
--- a/javatests/com/google/gerrit/server/group/db/BUILD
+++ b/javatests/com/google/gerrit/server/group/db/BUILD
@@ -16,9 +16,10 @@
"//java/com/google/gerrit/server/group/testing",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//java/com/google/gerrit/truth",
+ "//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/query/account/BUILD b/javatests/com/google/gerrit/server/query/account/BUILD
index c352f43..e6c631b 100644
--- a/javatests/com/google/gerrit/server/query/account/BUILD
+++ b/javatests/com/google/gerrit/server/query/account/BUILD
@@ -15,10 +15,11 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:truth",
- "//lib:truth-java8-extension",
+ "//lib:guava",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
"//prolog:gerrit-prolog-common",
],
)
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 66c825c..78ec176 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -19,11 +19,12 @@
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
@@ -41,10 +42,11 @@
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
+ "//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/query/group/BUILD b/javatests/com/google/gerrit/server/query/group/BUILD
index 01a54a3..0dd16cd 100644
--- a/javatests/com/google/gerrit/server/query/group/BUILD
+++ b/javatests/com/google/gerrit/server/query/group/BUILD
@@ -15,10 +15,11 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:truth",
- "//lib:truth-java8-extension",
+ "//lib:guava",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
],
)
diff --git a/javatests/com/google/gerrit/server/query/project/BUILD b/javatests/com/google/gerrit/server/query/project/BUILD
index ac2692b..eaa3df3 100644
--- a/javatests/com/google/gerrit/server/query/project/BUILD
+++ b/javatests/com/google/gerrit/server/query/project/BUILD
@@ -14,9 +14,10 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:truth",
+ "//lib:guava",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/server/rules/BUILD b/javatests/com/google/gerrit/server/rules/BUILD
index 04a6485..42452df 100644
--- a/javatests/com/google/gerrit/server/rules/BUILD
+++ b/javatests/com/google/gerrit/server/rules/BUILD
@@ -10,10 +10,10 @@
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:truth",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/prolog:runtime",
+ "//lib/truth",
"//prolog:gerrit-prolog-common",
],
)
diff --git a/javatests/com/google/gerrit/server/update/BUILD b/javatests/com/google/gerrit/server/update/BUILD
index 81e8b31..46820c7 100644
--- a/javatests/com/google/gerrit/server/update/BUILD
+++ b/javatests/com/google/gerrit/server/update/BUILD
@@ -12,9 +12,9 @@
"//java/com/google/gerrit/server",
"//lib:guava",
"//lib:junit",
- "//lib:truth",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
],
)
@@ -34,10 +34,10 @@
"//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
"//lib:gwtorm",
- "//lib:truth",
- "//lib:truth-java8-extension",
"//lib/guice",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/jgit/org.eclipse.jgit.junit:junit",
+ "//lib/truth",
+ "//lib/truth:truth-java8-extension",
],
)
diff --git a/javatests/com/google/gerrit/sshd/BUILD b/javatests/com/google/gerrit/sshd/BUILD
index c0eaedf..ad7d8a9 100644
--- a/javatests/com/google/gerrit/sshd/BUILD
+++ b/javatests/com/google/gerrit/sshd/BUILD
@@ -7,7 +7,7 @@
"//java/com/google/gerrit/extensions:api",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/sshd",
- "//lib:truth",
"//lib/mina:sshd",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/testing/BUILD b/javatests/com/google/gerrit/testing/BUILD
index 191e98f..5774707 100644
--- a/javatests/com/google/gerrit/testing/BUILD
+++ b/javatests/com/google/gerrit/testing/BUILD
@@ -7,6 +7,6 @@
deps = [
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/testing:gerrit-test-util",
- "//lib:truth",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gerrit/util/http/BUILD b/javatests/com/google/gerrit/util/http/BUILD
index 5755ca8..48b4339 100644
--- a/javatests/com/google/gerrit/util/http/BUILD
+++ b/javatests/com/google/gerrit/util/http/BUILD
@@ -8,7 +8,7 @@
"//javatests/com/google/gerrit/util/http/testutil",
"//lib:junit",
"//lib:servlet-api-3_1-without-neverlink",
- "//lib:truth",
"//lib/easymock",
+ "//lib/truth",
],
)
diff --git a/javatests/com/google/gwtexpui/safehtml/BUILD b/javatests/com/google/gwtexpui/safehtml/BUILD
index 4f75bdb..694f422 100644
--- a/javatests/com/google/gwtexpui/safehtml/BUILD
+++ b/javatests/com/google/gwtexpui/safehtml/BUILD
@@ -5,8 +5,9 @@
srcs = glob(["client/**/*.java"]),
deps = [
"//java/com/google/gwtexpui/safehtml",
- "//lib:truth",
+ "//lib:guava",
"//lib/gwt:dev",
"//lib/gwt:user",
+ "//lib/truth",
],
)
diff --git a/lib/BUILD b/lib/BUILD
index 5e391e9..c698afb 100644
--- a/lib/BUILD
+++ b/lib/BUILD
@@ -217,28 +217,6 @@
)
java_library(
- name = "truth",
- data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
- visibility = ["//visibility:public"],
- exports = [
- ":guava",
- ":junit",
- "@truth//jar",
- ],
-)
-
-java_library(
- name = "truth-java8-extension",
- data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
- visibility = ["//visibility:public"],
- exports = [
- ":guava",
- ":truth",
- "@truth-java8-extension//jar",
- ],
-)
-
-java_library(
name = "javassist",
data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
visibility = ["//visibility:public"],
diff --git a/lib/guava.bzl b/lib/guava.bzl
index db85900..e90c2b3 100644
--- a/lib/guava.bzl
+++ b/lib/guava.bzl
@@ -1,5 +1,5 @@
-GUAVA_VERSION = "24.1-jre"
+GUAVA_VERSION = "25.0-jre"
-GUAVA_BIN_SHA1 = "96c528475465aeb22cce60605d230a7e67cebd7b"
+GUAVA_BIN_SHA1 = "7319c34fa5866a85b6bad445adad69d402323129"
GUAVA_DOC_URL = "https://google.github.io/guava/releases/" + GUAVA_VERSION + "/api/docs/"
diff --git a/lib/truth/BUILD b/lib/truth/BUILD
new file mode 100644
index 0000000..cb17269
--- /dev/null
+++ b/lib/truth/BUILD
@@ -0,0 +1,21 @@
+java_library(
+ name = "truth",
+ data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
+ visibility = ["//visibility:public"],
+ exports = ["@truth//jar"],
+ runtime_deps = [
+ "//lib:guava",
+ "//lib:junit",
+ ],
+)
+
+java_library(
+ name = "truth-java8-extension",
+ data = ["//lib:LICENSE-DO_NOT_DISTRIBUTE"],
+ visibility = ["//visibility:public"],
+ exports = ["@truth-java8-extension//jar"],
+ runtime_deps = [
+ ":truth",
+ "//lib:guava",
+ ],
+)
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index b59742e..3f26628 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -283,6 +283,7 @@
as="file"
initial-count="[[fileListIncrement]]"
target-framerate="1">
+ [[_reportRenderedRow(index)]]
<div class="stickyArea">
<div class$="file-row row [[_computePathClass(file.__path, _expandedFilePaths.*)]]"
data-path$="[[file.__path]]" tabindex="-1">
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 0fa037c..83f1565 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -26,6 +26,8 @@
const SIZE_BAR_GAP_WIDTH = 1;
const SIZE_BAR_MIN_WIDTH = 1.5;
+ const RENDER_TIME = 'FileListRenderTime';
+
const FileStatus = {
A: 'Added',
C: 'Copied',
@@ -429,17 +431,21 @@
return GrCountStringFormatter.computeShortString(commentCount, 'c');
},
- _reviewFile(path) {
+ /**
+ * @param {string} path
+ * @param {boolean=} opt_reviewed
+ */
+ _reviewFile(path, opt_reviewed) {
if (this.editMode) { return; }
const index = this._files.findIndex(file => file.__path === path);
- const reviewed = this._files[index].isReviewed;
+ const reviewed = opt_reviewed || !this._files[index].isReviewed;
- this.set(['_files', index, 'isReviewed'], !reviewed);
+ this.set(['_files', index, 'isReviewed'], reviewed);
if (index < this._shownFiles.length) {
- this.set(['_shownFiles', index, 'isReviewed'], !reviewed);
+ this.set(['_shownFiles', index, 'isReviewed'], reviewed);
}
- this._saveReviewedState(path, !reviewed);
+ this._saveReviewedState(path, reviewed);
},
_saveReviewedState(path, reviewed) {
@@ -797,6 +803,12 @@
_computeFilesShown(numFilesShown, files) {
const filesShown = files.base.slice(0, numFilesShown);
this.fire('files-shown-changed', {length: filesShown.length});
+
+ // Start the timer for the rendering work hwere because this is where the
+ // _shownFiles property is being set, and _shownFiles is used in the
+ // dom-repeat binding.
+ this.$.reporting.time(RENDER_TIME);
+
return filesShown;
},
@@ -953,7 +965,7 @@
path, this.patchRange, this.projectConfig);
const promises = [diffElem.reload()];
if (this._loggedIn && !this.diffPrefs.manual_review) {
- promises.push(this._reviewFile(path));
+ promises.push(this._reviewFile(path, true));
}
return Promise.all(promises);
}).then(() => {
@@ -1175,5 +1187,21 @@
_noDiffsExpanded() {
return this.filesExpanded === GrFileListConstants.FilesExpandedState.NONE;
},
+
+ /**
+ * Method to call via binding when each file list row is rendered. This
+ * allows approximate detection of when the dom-repeat has completed
+ * rendering.
+ * @param {number} index The index of the row being rendered.
+ * @return {string} an empty string.
+ */
+ _reportRenderedRow(index) {
+ if (index === this._shownFiles.length - 1) {
+ this.async(() => {
+ this.$.reporting.timeEnd(RENDER_TIME);
+ }, 1);
+ }
+ return '';
+ },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 8541edf..3c90a1f 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -60,7 +60,6 @@
stub('gr-rest-api-interface', {
getLoggedIn() { return Promise.resolve(true); },
getPreferences() { return Promise.resolve({}); },
- fetchJSON() { return Promise.resolve({}); },
getDiffComments() { return Promise.resolve({}); },
getDiffRobotComments() { return Promise.resolve({}); },
getDiffDrafts() { return Promise.resolve({}); },
@@ -127,6 +126,19 @@
assert.isTrue(controlRow.classList.contains('invisible'));
});
+ test('rendering each row calls the _reportRenderedRow method', () => {
+ const renderedStub = sandbox.stub(element, '_reportRenderedRow');
+ element._filesByPath = _.range(10)
+ .reduce((_filesByPath, i) => {
+ _filesByPath['/file' + i] = {lines_inserted: 9};
+ return _filesByPath;
+ }, {});
+ flushAsynchronousOperations();
+ assert.equal(
+ Polymer.dom(element.root).querySelectorAll('.file-row').length, 10);
+ assert.equal(renderedStub.callCount, 10);
+ });
+
test('calculate totals for patch number', () => {
element._filesByPath = {
'/COMMIT_MSG': {
@@ -1023,6 +1035,7 @@
delete element.diffPrefs.manual_review;
return element._renderInOrder(['p'], diffs, 1).then(() => {
assert.isTrue(reviewStub.called);
+ assert.isTrue(reviewStub.calledWithExactly('p', true));
});
});
});
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js
new file mode 100644
index 0000000..28c46f4
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector.js
@@ -0,0 +1,61 @@
+/**
+ * @license
+ * Copyright (C) 2016 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.
+ */
+(function() {
+ 'use strict';
+
+ const JANK_SLEEP_TIME_MS = 1000;
+
+ const GrJankDetector = {
+ // Slowdowns counter.
+ jank: 0,
+ fps: 0,
+ _lastFrameTime: 0,
+
+ start() {
+ this._requestAnimationFrame(this._detect.bind(this));
+ },
+
+ _requestAnimationFrame(callback) {
+ window.requestAnimationFrame(callback);
+ },
+
+ _detect(now) {
+ if (this._lastFrameTime === 0) {
+ this._lastFrameTime = now;
+ this.fps = 0;
+ this._requestAnimationFrame(this._detect.bind(this));
+ return;
+ }
+ const fpsNow = 1000/(now - this._lastFrameTime);
+ this._lastFrameTime = now;
+ // Calculate moving average within last 3 measurements.
+ this.fps = this.fps === 0 ? fpsNow : ((this.fps * 2 + fpsNow) / 3);
+ if (this.fps > 10) {
+ this._requestAnimationFrame(this._detect.bind(this));
+ } else {
+ this.jank++;
+ console.warn('JANK', this.jank);
+ this._lastFrameTime = 0;
+ window.setTimeout(
+ () => this._requestAnimationFrame(this._detect.bind(this)),
+ JANK_SLEEP_TIME_MS);
+ }
+ },
+ };
+
+ window.GrJankDetector = GrJankDetector;
+})();
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html
new file mode 100644
index 0000000..6faeec1
--- /dev/null
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-jank-detector_test.html
@@ -0,0 +1,78 @@
+<!DOCTYPE html>
+<!--
+@license
+Copyright (C) 2018 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.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-jank-detector</title>
+
+<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
+<script src="../../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../../test/common-test-setup.html"/>
+
+<script src="gr-jank-detector.js"></script>
+
+<script>
+ suite('gr-jank-detector tests', () => {
+ let sandbox;
+ let clock;
+ let instance;
+
+ const NOW_TIME = 100;
+
+ setup(() => {
+ sandbox = sinon.sandbox.create();
+ clock = sinon.useFakeTimers(NOW_TIME);
+ instance = GrJankDetector;
+ instance._lastFrameTime = 0;
+ sandbox.stub(instance, '_requestAnimationFrame');
+ });
+
+ teardown(() => {
+ sandbox.restore();
+ });
+
+ test('start() installs frame callback', () => {
+ sandbox.stub(instance, '_detect');
+ instance._requestAnimationFrame.callsArg(0);
+ instance.start();
+ assert.isTrue(instance._detect.calledOnce);
+ });
+
+ test('measures fps', () => {
+ instance._detect(10);
+ instance._detect(30);
+ assert.equal(instance.fps, 50);
+ });
+
+ test('detects jank', () => {
+ let now = 10;
+ instance._detect(now);
+ const fastFrame = () => instance._detect(now += 20);
+ const slowFrame = () => instance._detect(now += 300);
+ fastFrame();
+ assert.equal(instance.jank, 0);
+ _.times(4, slowFrame);
+ assert.equal(instance.jank, 0);
+ instance._requestAnimationFrame.reset();
+ slowFrame();
+ assert.equal(instance.jank, 1);
+ assert.isFalse(instance._requestAnimationFrame.called);
+ clock.tick(1000);
+ assert.isTrue(instance._requestAnimationFrame.called);
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
index 2970a26..cbb2c09 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.html
@@ -19,5 +19,6 @@
<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<dom-module id="gr-reporting">
+ <script src="gr-jank-detector.js"></script>
<script src="gr-reporting.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
index 0db442f..ae67dac 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting.js
@@ -48,6 +48,14 @@
STARTED_HIDDEN: 'hidden',
};
+ // Frame rate related constants.
+ const JANK = {
+ TYPE: 'lifecycle',
+ CATEGORY: 'UI Latency',
+ // Reported events - alphabetize below.
+ COUNT: 'Jank count',
+ };
+
// Navigation reporting constants.
const NAVIGATION = {
TYPE: 'nav-report',
@@ -118,6 +126,8 @@
};
catchErrors();
+ GrJankDetector.start();
+
const GrReporting = Polymer({
is: 'gr-reporting',
@@ -206,6 +216,11 @@
},
beforeLocationChanged() {
+ if (GrJankDetector.jank > 0) {
+ this.reporter(
+ JANK.TYPE, JANK.CATEGORY, JANK.COUNT, GrJankDetector.jank);
+ GrJankDetector.jank = 0;
+ }
for (const prop of Object.keys(this._baselines)) {
delete this._baselines[prop];
}
diff --git a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
index bfb45f6..e2bb83d 100644
--- a/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
+++ b/polygerrit-ui/app/elements/core/gr-reporting/gr-reporting_test.html
@@ -93,7 +93,11 @@
test('beforeLocationChanged', () => {
element._baselines['garbage'] = 'monster';
sandbox.stub(element, 'time');
+ GrJankDetector.jank = 42;
element.beforeLocationChanged();
+ assert.equal(GrJankDetector.jank, 0);
+ assert.isTrue(element.reporter.calledWithExactly(
+ 'lifecycle', 'UI Latency', 'Jank count', 42));
assert.isTrue(element.time.calledWithExactly('DashboardDisplayed'));
assert.isTrue(element.time.calledWithExactly('ChangeDisplayed'));
assert.isTrue(element.time.calledWithExactly('DiffViewDisplayed'));
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
index 9dc51ba..9a5851b 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -27,6 +27,36 @@
*/
Defs.patchRange;
+ /**
+ * Object to describe a request for passing into _fetchJSON or _fetchRawJSON.
+ * - url is the URL for the request (excluding get params)
+ * - errFn is a function to invoke when the request fails.
+ * - cancelCondition is a function that, if provided and returns true, will
+ * cancel the response after it resolves.
+ * - params is a key-value hash to specify get params for the request URL.
+ * @typedef {{
+ * url: string,
+ * errFn: (function(?Response, string=)|null|undefined),
+ * cancelCondition: (function()|null|undefined),
+ * params: (Object|null|undefined),
+ * fetchOptions: (Object|null|undefined),
+ * }}
+ */
+ Defs.FetchJSONRequest;
+
+ /**
+ * @typedef {{
+ * changeNum: (string|number),
+ * endpoint: string,
+ * patchNum: (string|number|null|undefined),
+ * errFn: (function(?Response, string=)|null|undefined),
+ * cancelCondition: (function()|null|undefined),
+ * params: (Object|null|undefined),
+ * fetchOptions: (Object|null|undefined),
+ * }}
+ */
+ Defs.ChangeFetchRequest;
+
const DiffViewMode = {
SIDE_BY_SIDE: 'SIDE_BY_SIDE',
UNIFIED: 'UNIFIED_DIFF',
@@ -112,23 +142,17 @@
* Returns a Promise that resolves to a native Response.
* Doesn't do error checking. Supports cancel condition. Performs auth.
* Validates auth expiry errors.
- * @param {string} url
- * @param {?function(?Response, string=)=} opt_errFn
- * passed as null sometimes.
- * @param {?function()=} opt_cancelCondition
- * passed as null sometimes.
- * @param {?Object=} opt_params URL params, key-value hash.
- * @param {?Object=} opt_options Fetch options.
+ * @param {Defs.FetchJSONRequest} req
+ * @return {Promise}
*/
- _fetchRawJSON(url, opt_errFn, opt_cancelCondition, opt_params,
- opt_options) {
- const urlWithParams = this._urlWithParams(url, opt_params);
- return this._auth.fetch(urlWithParams, opt_options).then(response => {
- if (opt_cancelCondition && opt_cancelCondition()) {
- response.body.cancel();
+ _fetchRawJSON(req) {
+ const urlWithParams = this._urlWithParams(req.url, req.params);
+ return this._auth.fetch(urlWithParams, req.fetchOptions).then(res => {
+ if (req.cancelCondition && req.cancelCondition()) {
+ res.body.cancel();
return;
}
- return response;
+ return res;
}).catch(err => {
const isLoggedIn = !!this._cache['/accounts/self/detail'];
if (isLoggedIn && err && err.message === FAILED_TO_FETCH_ERROR) {
@@ -139,8 +163,8 @@
CHECK_SIGN_IN_DEBOUNCE_MS);
return;
}
- if (opt_errFn) {
- opt_errFn.call(undefined, null, err);
+ if (req.errFn) {
+ req.errFn.call(undefined, null, err);
} else {
this.fire('network-error', {error: err});
}
@@ -152,31 +176,23 @@
* Fetch JSON from url provided.
* Returns a Promise that resolves to a parsed response.
* Same as {@link _fetchRawJSON}, plus error handling.
- * @param {string} url
- * @param {?function(?Response, string=)=} opt_errFn
- * passed as null sometimes.
- * @param {?function()=} opt_cancelCondition
- * passed as null sometimes.
- * @param {?Object=} opt_params URL params, key-value hash.
- * @param {?Object=} opt_options Fetch options.
+ * @param {Defs.FetchJSONRequest} req
*/
- fetchJSON(url, opt_errFn, opt_cancelCondition, opt_params, opt_options) {
- return this._fetchRawJSON(
- url, opt_errFn, opt_cancelCondition, opt_params, opt_options)
- .then(response => {
- if (!response) {
- return;
- }
- if (!response.ok) {
- if (opt_errFn) {
- opt_errFn.call(null, response);
- return;
- }
- this.fire('server-error', {response});
- return;
- }
- return response && this.getResponseObject(response);
- });
+ _fetchJSON(req) {
+ return this._fetchRawJSON(req).then(response => {
+ if (!response) {
+ return;
+ }
+ if (!response.ok) {
+ if (req.errFn) {
+ req.errFn.call(null, response);
+ return;
+ }
+ this.fire('server-error', {response});
+ return;
+ }
+ return response && this.getResponseObject(response);
+ });
},
/**
@@ -236,39 +252,45 @@
getConfig(noCache) {
if (!noCache) {
- return this._fetchSharedCacheURL('/config/server/info');
+ return this._fetchSharedCacheURL({url: '/config/server/info'});
}
- return this.fetchJSON('/config/server/info');
+ return this._fetchJSON({url: '/config/server/info'});
},
getRepo(repo, opt_errFn) {
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchSharedCacheURL(
- '/projects/' + encodeURIComponent(repo), opt_errFn);
+ return this._fetchSharedCacheURL({
+ url: '/projects/' + encodeURIComponent(repo),
+ errFn: opt_errFn,
+ });
},
getProjectConfig(repo, opt_errFn) {
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchSharedCacheURL(
- '/projects/' + encodeURIComponent(repo) + '/config', opt_errFn);
+ return this._fetchSharedCacheURL({
+ url: '/projects/' + encodeURIComponent(repo) + '/config',
+ errFn: opt_errFn,
+ });
},
getRepoAccess(repo) {
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchSharedCacheURL(
- '/access/?project=' + encodeURIComponent(repo));
+ return this._fetchSharedCacheURL({
+ url: '/access/?project=' + encodeURIComponent(repo),
+ });
},
getRepoDashboards(repo, opt_errFn) {
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchSharedCacheURL(
- `/projects/${encodeURIComponent(repo)}/dashboards?inherited`,
- opt_errFn);
+ return this._fetchSharedCacheURL({
+ url: `/projects/${encodeURIComponent(repo)}/dashboards?inherited`,
+ errFn: opt_errFn,
+ });
},
saveRepoConfig(repo, config, opt_errFn, opt_ctx) {
@@ -315,8 +337,10 @@
},
getGroupConfig(group, opt_errFn) {
- const encodeName = encodeURIComponent(group);
- return this.fetchJSON(`/groups/${encodeName}/detail`, opt_errFn);
+ return this._fetchJSON({
+ url: `/groups/${encodeURIComponent(group)}/detail`,
+ errFn: opt_errFn,
+ });
},
/**
@@ -394,7 +418,7 @@
*/
getIsGroupOwner(groupName) {
const encodeName = encodeURIComponent(groupName);
- return this._fetchSharedCacheURL(`/groups/?owned&q=${encodeName}`)
+ return this._fetchSharedCacheURL({url: `/groups/?owned&q=${encodeName}`})
.then(configs => configs.hasOwnProperty(groupName));
},
@@ -432,8 +456,10 @@
},
getGroupAuditLog(group, opt_errFn) {
- return this._fetchSharedCacheURL(
- '/groups/' + group + '/log.audit', opt_errFn);
+ return this._fetchSharedCacheURL({
+ url: '/groups/' + group + '/log.audit',
+ errFn: opt_errFn,
+ });
},
saveGroupMembers(groupName, groupMembers) {
@@ -470,13 +496,15 @@
},
getVersion() {
- return this._fetchSharedCacheURL('/config/server/version');
+ return this._fetchSharedCacheURL({url: '/config/server/version'});
},
getDiffPreferences() {
return this.getLoggedIn().then(loggedIn => {
if (loggedIn) {
- return this._fetchSharedCacheURL('/accounts/self/preferences.diff');
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/preferences.diff',
+ });
}
// These defaults should match the defaults in
// java/com/google/gerrit/extensions/client/DiffPreferencesInfo.java
@@ -504,7 +532,9 @@
getEditPreferences() {
return this.getLoggedIn().then(loggedIn => {
if (loggedIn) {
- return this._fetchSharedCacheURL('/accounts/self/preferences.edit');
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/preferences.edit',
+ });
}
// These defaults should match the defaults in
// java/com/google/gerrit/extensions/client/EditPreferencesInfo.java
@@ -570,15 +600,18 @@
},
getAccount() {
- return this._fetchSharedCacheURL('/accounts/self/detail', resp => {
- if (!resp || resp.status === 403) {
- this._cache['/accounts/self/detail'] = null;
- }
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/detail',
+ errFn: resp => {
+ if (!resp || resp.status === 403) {
+ this._cache['/accounts/self/detail'] = null;
+ }
+ },
});
},
getExternalIds() {
- return this.fetchJSON('/accounts/self/external.ids');
+ return this._fetchJSON({url: '/accounts/self/external.ids'});
},
deleteAccountIdentity(id) {
@@ -591,11 +624,13 @@
* @return {!Promise<!Object>}
*/
getAccountDetails(userId) {
- return this.fetchJSON(`/accounts/${encodeURIComponent(userId)}/detail`);
+ return this._fetchJSON({
+ url: `/accounts/${encodeURIComponent(userId)}/detail`,
+ });
},
getAccountEmails() {
- return this._fetchSharedCacheURL('/accounts/self/emails');
+ return this._fetchSharedCacheURL({url: '/accounts/self/emails'});
},
/**
@@ -692,15 +727,17 @@
},
getAccountStatus(userId) {
- return this.fetchJSON(`/accounts/${encodeURIComponent(userId)}/status`);
+ return this._fetchJSON({
+ url: `/accounts/${encodeURIComponent(userId)}/status`,
+ });
},
getAccountGroups() {
- return this.fetchJSON('/accounts/self/groups');
+ return this._fetchJSON({url: '/accounts/self/groups'});
},
getAccountAgreements() {
- return this.fetchJSON('/accounts/self/agreements');
+ return this._fetchJSON({url: '/accounts/self/agreements'});
},
saveAccountAgreement(name) {
@@ -717,8 +754,9 @@
.map(param => { return encodeURIComponent(param); })
.join('&q=');
}
- return this._fetchSharedCacheURL('/accounts/self/capabilities' +
- queryString);
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/capabilities' + queryString,
+ });
},
getLoggedIn() {
@@ -741,31 +779,31 @@
checkCredentials() {
// Skip the REST response cache.
- return this._fetchRawJSON('/accounts/self/detail').then(response => {
- if (!response) { return; }
- if (response.status === 403) {
+ return this._fetchRawJSON({url: '/accounts/self/detail'}).then(res => {
+ if (!res) { return; }
+ if (res.status === 403) {
this.fire('auth-error');
this._cache['/accounts/self/detail'] = null;
- } else if (response.ok) {
- return this.getResponseObject(response);
+ } else if (res.ok) {
+ return this.getResponseObject(res);
}
- }).then(response => {
- if (response) {
- this._cache['/accounts/self/detail'] = response;
+ }).then(res => {
+ if (res) {
+ this._cache['/accounts/self/detail'] = res;
}
- return response;
+ return res;
});
},
getDefaultPreferences() {
- return this._fetchSharedCacheURL('/config/server/preferences');
+ return this._fetchSharedCacheURL({url: '/config/server/preferences'});
},
getPreferences() {
return this.getLoggedIn().then(loggedIn => {
if (loggedIn) {
- return this._fetchSharedCacheURL('/accounts/self/preferences').then(
- res => {
+ return this._fetchSharedCacheURL({url: '/accounts/self/preferences'})
+ .then(res => {
if (this._isNarrowScreen()) {
res.default_diff_view = DiffViewMode.UNIFIED;
} else {
@@ -786,7 +824,9 @@
},
getWatchedProjects() {
- return this._fetchSharedCacheURL('/accounts/self/watched.projects');
+ return this._fetchSharedCacheURL({
+ url: '/accounts/self/watched.projects',
+ });
},
/**
@@ -813,29 +853,28 @@
},
/**
- * @param {string} url
- * @param {function(?Response, string=)=} opt_errFn
+ * @param {Defs.FetchJSONRequest} req
*/
- _fetchSharedCacheURL(url, opt_errFn) {
- if (this._sharedFetchPromises[url]) {
- return this._sharedFetchPromises[url];
+ _fetchSharedCacheURL(req) {
+ if (this._sharedFetchPromises[req.url]) {
+ return this._sharedFetchPromises[req.url];
}
// TODO(andybons): Periodic cache invalidation.
- if (this._cache[url] !== undefined) {
- return Promise.resolve(this._cache[url]);
+ if (this._cache[req.url] !== undefined) {
+ return Promise.resolve(this._cache[req.url]);
}
- this._sharedFetchPromises[url] = this.fetchJSON(url, opt_errFn)
+ this._sharedFetchPromises[req.url] = this._fetchJSON(req)
.then(response => {
if (response !== undefined) {
- this._cache[url] = response;
+ this._cache[req.url] = response;
}
- this._sharedFetchPromises[url] = undefined;
+ this._sharedFetchPromises[req.url] = undefined;
return response;
}).catch(err => {
- this._sharedFetchPromises[url] = undefined;
+ this._sharedFetchPromises[req.url] = undefined;
throw err;
});
- return this._sharedFetchPromises[url];
+ return this._sharedFetchPromises[req.url];
},
_isNarrowScreen() {
@@ -848,8 +887,8 @@
* @param {number|string=} opt_offset
* @param {!Object=} opt_options
* @return {?Array<!Object>|?Array<!Array<!Object>>} If opt_query is an
- * array, fetchJSON will return an array of arrays of changeInfos. If it
- * is unspecified or a string, fetchJSON will return an array of
+ * array, _fetchJSON will return an array of arrays of changeInfos. If it
+ * is unspecified or a string, _fetchJSON will return an array of
* changeInfos.
*/
getChanges(opt_changesPerPage, opt_query, opt_offset, opt_options) {
@@ -874,7 +913,7 @@
this._maybeInsertInLookup(change);
}
};
- return this.fetchJSON('/changes/', null, null, params).then(response => {
+ return this._fetchJSON({url: '/changes/', params}).then(response => {
// Response may be an array of changes OR an array of arrays of
// changes.
if (opt_query instanceof Array) {
@@ -959,43 +998,43 @@
* @param {function(?Response, string=)=} opt_errFn
* @param {function()=} opt_cancelCondition
*/
- _getChangeDetail(changeNum, params, opt_errFn,
- opt_cancelCondition) {
+ _getChangeDetail(changeNum, params, opt_errFn, opt_cancelCondition) {
return this.getChangeActionURL(changeNum, null, '/detail').then(url => {
const urlWithParams = this._urlWithParams(url, params);
- return this._fetchRawJSON(
- url,
- opt_errFn,
- opt_cancelCondition,
- {O: params},
- this._etags.getOptions(urlWithParams))
- .then(response => {
- if (response && response.status === 304) {
- return Promise.resolve(this._parsePrefixedJSON(
- this._etags.getCachedPayload(urlWithParams)));
- }
+ const req = {
+ url,
+ errFn: opt_errFn,
+ cancelCondition: opt_cancelCondition,
+ params: {O: params},
+ fetchOptions: this._etags.getOptions(urlWithParams),
+ };
+ return this._fetchRawJSON(req).then(response => {
+ if (response && response.status === 304) {
+ return Promise.resolve(this._parsePrefixedJSON(
+ this._etags.getCachedPayload(urlWithParams)));
+ }
- if (response && !response.ok) {
- if (opt_errFn) {
- opt_errFn.call(null, response);
- } else {
- this.fire('server-error', {response});
- }
- return;
- }
+ if (response && !response.ok) {
+ if (opt_errFn) {
+ opt_errFn.call(null, response);
+ } else {
+ this.fire('server-error', {response});
+ }
+ return;
+ }
- const payloadPromise = response ?
- this._readResponsePayload(response) :
- Promise.resolve(null);
+ const payloadPromise = response ?
+ this._readResponsePayload(response) :
+ Promise.resolve(null);
- return payloadPromise.then(payload => {
- if (!payload) { return null; }
- this._etags.collect(urlWithParams, response, payload.raw);
- this._maybeInsertInLookup(payload.parsed);
+ return payloadPromise.then(payload => {
+ if (!payload) { return null; }
+ this._etags.collect(urlWithParams, response, payload.raw);
+ this._maybeInsertInLookup(payload.parsed);
- return payload.parsed;
- });
- });
+ return payload.parsed;
+ });
+ });
});
},
@@ -1004,7 +1043,11 @@
* @param {number|string} patchNum
*/
getChangeCommitInfo(changeNum, patchNum) {
- return this._getChangeURLAndFetch(changeNum, '/commit?links', patchNum);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/commit?links',
+ patchNum,
+ });
},
/**
@@ -1019,8 +1062,12 @@
} else if (!this.patchNumEquals(patchRange.basePatchNum, 'PARENT')) {
params = {base: patchRange.basePatchNum};
}
- return this._getChangeURLAndFetch(changeNum, '/files',
- patchRange.patchNum, undefined, undefined, params);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/files',
+ patchNum: patchRange.patchNum,
+ params,
+ });
},
/**
@@ -1032,7 +1079,7 @@
if (patchRange.basePatchNum !== 'PARENT') {
endpoint += '&base=' + encodeURIComponent(patchRange.basePatchNum + '');
}
- return this._getChangeURLAndFetch(changeNum, endpoint);
+ return this._getChangeURLAndFetch({changeNum, endpoint});
},
/**
@@ -1042,8 +1089,11 @@
* @return {!Promise<!Object>}
*/
queryChangeFiles(changeNum, patchNum, query) {
- return this._getChangeURLAndFetch(changeNum,
- `/files?q=${encodeURIComponent(query)}`, patchNum);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: `/files?q=${encodeURIComponent(query)}`,
+ patchNum,
+ });
},
/**
@@ -1071,16 +1121,16 @@
},
getChangeRevisionActions(changeNum, patchNum) {
- return this._getChangeURLAndFetch(changeNum, '/actions', patchNum)
- .then(revisionActions => {
- // The rebase button on change screen is always enabled.
- if (revisionActions.rebase) {
- revisionActions.rebase.rebaseOnCurrent =
- !!revisionActions.rebase.enabled;
- revisionActions.rebase.enabled = true;
- }
- return revisionActions;
- });
+ const req = {changeNum, endpoint: '/actions', patchNum};
+ return this._getChangeURLAndFetch(req).then(revisionActions => {
+ // The rebase button on change screen is always enabled.
+ if (revisionActions.rebase) {
+ revisionActions.rebase.rebaseOnCurrent =
+ !!revisionActions.rebase.enabled;
+ revisionActions.rebase.enabled = true;
+ }
+ return revisionActions;
+ });
},
/**
@@ -1091,15 +1141,19 @@
getChangeSuggestedReviewers(changeNum, inputVal, opt_errFn) {
const params = {n: 10};
if (inputVal) { params.q = inputVal; }
- return this._getChangeURLAndFetch(changeNum, '/suggest_reviewers', null,
- opt_errFn, null, params);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/suggest_reviewers',
+ errFn: opt_errFn,
+ params,
+ });
},
/**
* @param {number|string} changeNum
*/
getChangeIncludedIn(changeNum) {
- return this._getChangeURLAndFetch(changeNum, '/in', null);
+ return this._getChangeURLAndFetch({changeNum, endpoint: '/in'});
},
_computeFilter(filter) {
@@ -1122,10 +1176,10 @@
getGroups(filter, groupsPerPage, opt_offset) {
const offset = opt_offset || 0;
- return this._fetchSharedCacheURL(
- `/groups/?n=${groupsPerPage + 1}&S=${offset}` +
- this._computeFilter(filter)
- );
+ return this._fetchSharedCacheURL({
+ url: `/groups/?n=${groupsPerPage + 1}&S=${offset}` +
+ this._computeFilter(filter),
+ });
},
/**
@@ -1139,10 +1193,10 @@
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this._fetchSharedCacheURL(
- `/projects/?d&n=${reposPerPage + 1}&S=${offset}` +
- this._computeFilter(filter)
- );
+ return this._fetchSharedCacheURL({
+ url: `/projects/?d&n=${reposPerPage + 1}&S=${offset}` +
+ this._computeFilter(filter),
+ });
},
setRepoHead(repo, ref) {
@@ -1162,15 +1216,13 @@
*/
getRepoBranches(filter, repo, reposBranchesPerPage, opt_offset, opt_errFn) {
const offset = opt_offset || 0;
-
+ const count = reposBranchesPerPage + 1;
+ filter = this._computeFilter(filter);
+ repo = encodeURIComponent(repo);
+ const url = `/projects/${repo}/branches?n=${count}&S=${offset}${filter}`;
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this.fetchJSON(
- `/projects/${encodeURIComponent(repo)}/branches` +
- `?n=${reposBranchesPerPage + 1}&S=${offset}` +
- this._computeFilter(filter),
- opt_errFn
- );
+ return this._fetchJSON({url, errFn: opt_errFn});
},
/**
@@ -1183,15 +1235,14 @@
*/
getRepoTags(filter, repo, reposTagsPerPage, opt_offset, opt_errFn) {
const offset = opt_offset || 0;
-
+ const encodedRepo = encodeURIComponent(repo);
+ const n = reposTagsPerPage + 1;
+ const encodedFilter = this._computeFilter(filter);
+ const url = `/projects/${encodedRepo}/tags` + `?n=${n}&S=${offset}` +
+ encodedFilter;
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this.fetchJSON(
- `/projects/${encodeURIComponent(repo)}/tags` +
- `?n=${reposTagsPerPage + 1}&S=${offset}` +
- this._computeFilter(filter),
- opt_errFn
- );
+ return this._fetchJSON({url, errFn: opt_errFn});
},
/**
@@ -1203,21 +1254,19 @@
*/
getPlugins(filter, pluginsPerPage, opt_offset, opt_errFn) {
const offset = opt_offset || 0;
-
- return this.fetchJSON(
- `/plugins/?all&n=${pluginsPerPage + 1}&S=${offset}` +
- this._computeFilter(filter),
- opt_errFn
- );
+ const encodedFilter = this._computeFilter(filter);
+ const n = pluginsPerPage + 1;
+ const url = `/plugins/?all&n=${n}&S=${offset}${encodedFilter}`;
+ return this._fetchJSON({url, errFn: opt_errFn});
},
getRepoAccessRights(repoName, opt_errFn) {
// TODO(kaspern): Rename rest api from /projects/ to /repos/ once backend
// supports it.
- return this.fetchJSON(
- `/projects/${encodeURIComponent(repoName)}/access`,
- opt_errFn
- );
+ return this._fetchJSON({
+ url: `/projects/${encodeURIComponent(repoName)}/access`,
+ errFn: opt_errFn,
+ });
},
setRepoAccessRights(repoName, repoInfo) {
@@ -1243,7 +1292,12 @@
getSuggestedGroups(inputVal, opt_n, opt_errFn, opt_ctx) {
const params = {s: inputVal};
if (opt_n) { params.n = opt_n; }
- return this.fetchJSON('/groups/', opt_errFn, opt_ctx, params);
+ return this._fetchJSON({
+ url: '/groups/',
+ errFn: opt_errFn,
+ cancelCondition: opt_ctx,
+ params,
+ });
},
/**
@@ -1259,7 +1313,12 @@
type: 'ALL',
};
if (opt_n) { params.n = opt_n; }
- return this.fetchJSON('/projects/', opt_errFn, opt_ctx, params);
+ return this._fetchJSON({
+ url: '/projects/',
+ errFn: opt_errFn,
+ cancelCondition: opt_ctx,
+ params,
+ });
},
/**
@@ -1274,7 +1333,12 @@
}
const params = {suggest: null, q: inputVal};
if (opt_n) { params.n = opt_n; }
- return this.fetchJSON('/accounts/', opt_errFn, opt_ctx, params);
+ return this._fetchJSON({
+ url: '/accounts/',
+ errFn: opt_errFn,
+ cancelCondition: opt_ctx,
+ params,
+ });
},
addChangeReviewer(changeNum, reviewerID) {
@@ -1305,11 +1369,18 @@
},
getRelatedChanges(changeNum, patchNum) {
- return this._getChangeURLAndFetch(changeNum, '/related', patchNum);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/related',
+ patchNum,
+ });
},
getChangesSubmittedTogether(changeNum) {
- return this._getChangeURLAndFetch(changeNum, '/submitted_together', null);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/submitted_together',
+ });
},
getChangeConflicts(changeNum) {
@@ -1321,7 +1392,7 @@
O: options,
q: 'status:open is:mergeable conflicts:' + changeNum,
};
- return this.fetchJSON('/changes/', null, null, params);
+ return this._fetchJSON({url: '/changes/', params});
},
getChangeCherryPicks(project, changeID, changeNum) {
@@ -1339,7 +1410,7 @@
O: options,
q: query,
};
- return this.fetchJSON('/changes/', null, null, params);
+ return this._fetchJSON({url: '/changes/', params});
},
getChangesWithSameTopic(topic) {
@@ -1353,11 +1424,15 @@
O: options,
q: 'status:open topic:' + topic,
};
- return this.fetchJSON('/changes/', null, null, params);
+ return this._fetchJSON({url: '/changes/', params});
},
getReviewedFiles(changeNum, patchNum) {
- return this._getChangeURLAndFetch(changeNum, '/files?reviewed', patchNum);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/files?reviewed',
+ patchNum,
+ });
},
/**
@@ -1395,10 +1470,12 @@
getChangeEdit(changeNum, opt_download_commands) {
const params = opt_download_commands ? {'download-commands': true} : null;
return this.getLoggedIn().then(loggedIn => {
- return loggedIn ?
- this._getChangeURLAndFetch(changeNum, '/edit/', null, null, null,
- params) :
- false;
+ if (!loggedIn) { return false; }
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: '/edit/',
+ params,
+ });
});
},
@@ -1607,8 +1684,14 @@
}
const endpoint = `/files/${encodeURIComponent(path)}/diff`;
- return this._getChangeURLAndFetch(changeNum, endpoint, patchNum,
- opt_errFn, opt_cancelCondition, params);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint,
+ patchNum,
+ errFn: opt_errFn,
+ cancelCondition: opt_cancelCondition,
+ params,
+ });
},
/**
@@ -1695,7 +1778,11 @@
* @return {!Promise<!Object>} Diff comments response.
*/
const fetchComments = opt_patchNum => {
- return this._getChangeURLAndFetch(changeNum, endpoint, opt_patchNum);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint,
+ patchNum: opt_patchNum,
+ });
};
if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
@@ -1809,9 +1896,10 @@
},
getCommitInfo(project, commit) {
- return this.fetchJSON(
- '/projects/' + encodeURIComponent(project) +
- '/commits/' + encodeURIComponent(commit));
+ return this._fetchJSON({
+ url: '/projects/' + encodeURIComponent(project) +
+ '/commits/' + encodeURIComponent(commit),
+ });
},
_fetchB64File(url) {
@@ -1940,7 +2028,7 @@
},
getAccountSSHKeys() {
- return this._fetchSharedCacheURL('/accounts/self/sshkeys');
+ return this._fetchSharedCacheURL({url: '/accounts/self/sshkeys'});
},
addAccountSSHKey(key) {
@@ -1963,7 +2051,7 @@
},
getAccountGPGKeys() {
- return this.fetchJSON('/accounts/self/gpgkeys');
+ return this._fetchJSON({url: '/accounts/self/gpgkeys'});
},
addAccountGPGKey(key) {
@@ -2006,7 +2094,10 @@
},
getCapabilities(token, opt_errFn) {
- return this.fetchJSON('/config/server/capabilities', opt_errFn);
+ return this._fetchJSON({
+ url: '/config/server/capabilities',
+ errFn: opt_errFn,
+ });
},
setAssignee(changeNum, assignee) {
@@ -2073,11 +2164,13 @@
*/
getChange(changeNum, opt_errFn) {
// Cannot use _changeBaseURL, as this function is used by _projectLookup.
- return this.fetchJSON(`/changes/?q=change:${changeNum}`, opt_errFn)
- .then(res => {
- if (!res || !res.length) { return null; }
- return res[0];
- });
+ return this._fetchJSON({
+ url: `/changes/?q=change:${changeNum}`,
+ errFn: opt_errFn,
+ }).then(res => {
+ if (!res || !res.length) { return null; }
+ return res[0];
+ });
},
/**
@@ -2140,23 +2233,20 @@
});
},
- /**
- * Alias for _changeBaseURL.then(fetchJSON).
- * @todo(beckysiegel) clean up comments
- * @param {string|number} changeNum
- * @param {string} endpoint
- * @param {?string|number=} opt_patchNum gets passed as null.
- * @param {?function(?Response, string=)=} opt_errFn gets passed as null.
- * @param {?function()=} opt_cancelCondition gets passed as null.
- * @param {?Object=} opt_params gets passed as null.
- * @param {!Object=} opt_options
- * @return {!Promise<!Object>}
- */
- _getChangeURLAndFetch(changeNum, endpoint, opt_patchNum, opt_errFn,
- opt_cancelCondition, opt_params, opt_options) {
- return this._changeBaseURL(changeNum, opt_patchNum).then(url => {
- return this.fetchJSON(url + endpoint, opt_errFn, opt_cancelCondition,
- opt_params, opt_options);
+ /**
+ * Alias for _changeBaseURL.then(_fetchJSON).
+ * @param {Defs.ChangeFetchRequest} req
+ * @return {!Promise<!Object>}
+ */
+ _getChangeURLAndFetch(req) {
+ return this._changeBaseURL(req.changeNum, req.patchNum).then(url => {
+ return this._fetchJSON({
+ url: url + req.endpoint,
+ errFn: req.errFn,
+ cancelCondition: req.cancelCondition,
+ params: req.params,
+ fetchOptions: req.fetchOptions,
+ });
});
},
@@ -2171,9 +2261,12 @@
*/
getBlame(changeNum, patchNum, path, opt_base) {
const encodedPath = encodeURIComponent(path);
- return this._getChangeURLAndFetch(changeNum,
- `/files/${encodedPath}/blame`, patchNum, undefined, undefined,
- opt_base ? {base: 't'} : undefined);
+ return this._getChangeURLAndFetch({
+ changeNum,
+ endpoint: `/files/${encodedPath}/blame`,
+ patchNum,
+ params: opt_base ? {base: 't'} : undefined,
+ });
},
/**
@@ -2217,7 +2310,7 @@
getDashboard(project, dashboard, opt_errFn) {
const url = '/projects/' + encodeURIComponent(project) + '/dashboards/' +
encodeURIComponent(dashboard);
- return this._fetchSharedCacheURL(url, opt_errFn);
+ return this._fetchSharedCacheURL({url, errFn: opt_errFn});
},
getMergeable(changeNum) {
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
index fb20da4..7e71efa 100644
--- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -58,7 +58,7 @@
});
test('JSON prefix is properly removed', done => {
- element.fetchJSON('/dummy/url').then(obj => {
+ element._fetchJSON('/dummy/url').then(obj => {
assert.deepEqual(obj, {hello: 'bonjour'});
done();
});
@@ -66,7 +66,7 @@
test('cached results', done => {
let n = 0;
- sandbox.stub(element, 'fetchJSON', () => {
+ sandbox.stub(element, '_fetchJSON', () => {
return Promise.resolve(++n);
});
const promises = [];
@@ -86,7 +86,7 @@
test('cached promise', done => {
const promise = Promise.reject('foo');
element._cache['/foo'] = promise;
- element._fetchSharedCacheURL('/foo').catch(p => {
+ element._fetchSharedCacheURL({url: '/foo'}).catch(p => {
assert.equal(p, 'foo');
done();
});
@@ -120,7 +120,8 @@
cancel() { cancelCalled = true; },
},
}));
- element.fetchJSON('/dummy/url', null, () => { return true; }).then(
+ const cancelCondition = () => { return true; };
+ element._fetchJSON({url: '/dummy/url', cancelCondition}).then(
obj => {
assert.isUndefined(obj);
assert.isTrue(cancelCalled);
@@ -129,7 +130,7 @@
});
test('parent diff comments are properly grouped', done => {
- sandbox.stub(element, 'fetchJSON', () => {
+ sandbox.stub(element, '_fetchJSON', () => {
return Promise.resolve({
'/COMMIT_MSG': [],
'sieve.go': [
@@ -272,7 +273,8 @@
test('differing patch diff comments are properly grouped', done => {
sandbox.stub(element, 'getFromProjectLookup')
.returns(Promise.resolve('test'));
- sandbox.stub(element, 'fetchJSON', url => {
+ sandbox.stub(element, '_fetchJSON', request => {
+ const url = request.url;
if (url === '/changes/test~42/revisions/1') {
return Promise.resolve({
'/COMMIT_MSG': [],
@@ -386,11 +388,11 @@
});
suite('rebase action', () => {
- let resolveFetchJSON;
+ let resolve_fetchJSON;
setup(() => {
- sandbox.stub(element, 'fetchJSON').returns(
+ sandbox.stub(element, '_fetchJSON').returns(
new Promise(resolve => {
- resolveFetchJSON = resolve;
+ resolve_fetchJSON = resolve;
}));
});
@@ -401,7 +403,7 @@
assert.isFalse(response.rebase.rebaseOnCurrent);
done();
});
- resolveFetchJSON({rebase: {}});
+ resolve_fetchJSON({rebase: {}});
});
test('rebase on current', done => {
@@ -411,7 +413,7 @@
assert.isTrue(response.rebase.rebaseOnCurrent);
done();
});
- resolveFetchJSON({rebase: {enabled: true}});
+ resolve_fetchJSON({rebase: {enabled: true}});
});
});
@@ -423,7 +425,7 @@
element.addEventListener('server-error', resolve);
});
- element.fetchJSON().then(response => {
+ element._fetchJSON({}).then(response => {
assert.isUndefined(response);
assert.isTrue(getResponseObjectStub.notCalled);
serverErrorEventPromise.then(() => done());
@@ -444,7 +446,7 @@
element.addEventListener('server-error', serverErrorStub);
const authErrorStub = sandbox.stub();
element.addEventListener('auth-error', authErrorStub);
- element.fetchJSON('/bar').then(r => {
+ element._fetchJSON('/bar').then(r => {
flush(() => {
assert.isTrue(authErrorStub.called);
assert.isFalse(serverErrorStub.called);
@@ -484,10 +486,10 @@
});
test('legacy n,z key in change url is replaced', () => {
- const stub = sandbox.stub(element, 'fetchJSON')
+ const stub = sandbox.stub(element, '_fetchJSON')
.returns(Promise.resolve([]));
element.getChanges(1, null, 'n,z');
- assert.equal(stub.args[0][3].S, 0);
+ assert.equal(stub.lastCall.args[0].params.S, 0);
});
test('saveDiffPreferences invalidates cache line', () => {
@@ -512,7 +514,7 @@
});
element._cache[cacheKey] = 'fake cache';
- stub.callArg(1);
+ stub.lastCall.args[0].errFn();
});
test('getAccount does not add to the cache when resp.status is 403',
@@ -527,7 +529,7 @@
done();
});
element._cache[cacheKey] = 'fake cache';
- stub.callArgWith(1, {status: 403});
+ stub.lastCall.args[0].errFn({status: 403});
});
test('getAccount when resp is successful', done => {
@@ -541,7 +543,8 @@
done();
});
element._cache[cacheKey] = 'fake cache';
- stub.callArg(1, {});
+
+ stub.lastCall.args[0].errFn({});
});
const preferenceSetup = function(testJSON, loggedIn, smallScreen) {
@@ -872,66 +875,69 @@
const fetchStub = sandbox.stub(element, '_getChangeURLAndFetch')
.returns(Promise.resolve());
return element.queryChangeFiles('42', 'edit', 'test/path.js').then(() => {
- assert.deepEqual(fetchStub.lastCall.args,
- ['42', '/files?q=test%2Fpath.js', 'edit']);
+ assert.deepEqual(fetchStub.lastCall.args[0], {
+ changeNum: '42',
+ endpoint: '/files?q=test%2Fpath.js',
+ patchNum: 'edit',
+ });
});
});
test('getRepos', () => {
sandbox.stub(element, '_fetchSharedCacheURL');
element.getRepos('test', 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/projects/?d&n=26&S=0&m=test'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?d&n=26&S=0&m=test');
element.getRepos(null, 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/projects/?d&n=26&S=0'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?d&n=26&S=0');
element.getRepos('test', 25, 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/projects/?d&n=26&S=25&m=test'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?d&n=26&S=25&m=test');
});
test('getRepos filter', () => {
sandbox.stub(element, '_fetchSharedCacheURL');
element.getRepos('test/test/test', 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/projects/?d&n=26&S=0&m=test%2Ftest%2Ftest'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?d&n=26&S=0&m=test%2Ftest%2Ftest');
});
test('getRepos filter regex', () => {
sandbox.stub(element, '_fetchSharedCacheURL');
element.getRepos('^test.*', 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/projects/?d&n=26&S=0&r=%5Etest.*'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/projects/?d&n=26&S=0&r=%5Etest.*');
});
test('getGroups filter regex', () => {
sandbox.stub(element, '_fetchSharedCacheURL');
element.getGroups('^test.*', 25);
- assert.isTrue(element._fetchSharedCacheURL.lastCall
- .calledWithExactly('/groups/?n=26&S=0&r=%5Etest.*'));
+ assert.equal(element._fetchSharedCacheURL.lastCall.args[0].url,
+ '/groups/?n=26&S=0&r=%5Etest.*');
});
test('gerrit auth is used', () => {
sandbox.stub(Gerrit.Auth, 'fetch').returns(Promise.resolve());
- element.fetchJSON('foo');
+ element._fetchJSON('foo');
assert(Gerrit.Auth.fetch.called);
});
- test('getSuggestedAccounts does not return fetchJSON', () => {
- const fetchJSONSpy = sandbox.spy(element, 'fetchJSON');
+ test('getSuggestedAccounts does not return _fetchJSON', () => {
+ const _fetchJSONSpy = sandbox.spy(element, '_fetchJSON');
return element.getSuggestedAccounts().then(accts => {
- assert.isFalse(fetchJSONSpy.called);
+ assert.isFalse(_fetchJSONSpy.called);
assert.equal(accts.length, 0);
});
});
- test('fetchJSON gets called by getSuggestedAccounts', () => {
- const fetchJSONStub = sandbox.stub(element, 'fetchJSON',
+ test('_fetchJSON gets called by getSuggestedAccounts', () => {
+ const _fetchJSONStub = sandbox.stub(element, '_fetchJSON',
() => Promise.resolve());
return element.getSuggestedAccounts('own').then(() => {
- assert.deepEqual(fetchJSONStub.lastCall.args[3], {
+ assert.deepEqual(_fetchJSONStub.lastCall.args[0].params, {
q: 'own',
suggest: null,
});
@@ -1064,7 +1070,7 @@
suite('getChanges populates _projectLookup', () => {
test('multiple queries', () => {
- sandbox.stub(element, 'fetchJSON')
+ sandbox.stub(element, '_fetchJSON')
.returns(Promise.resolve([
[
{_number: 1, project: 'test'},
@@ -1073,7 +1079,7 @@
{_number: 3, project: 'test/test'},
],
]));
- // When opt_query instanceof Array, fetchJSON returns
+ // When opt_query instanceof Array, _fetchJSON returns
// Array<Array<Object>>.
return element.getChanges(null, []).then(() => {
assert.equal(Object.keys(element._projectLookup).length, 3);
@@ -1084,14 +1090,14 @@
});
test('no query', () => {
- sandbox.stub(element, 'fetchJSON')
+ sandbox.stub(element, '_fetchJSON')
.returns(Promise.resolve([
{_number: 1, project: 'test'},
{_number: 2, project: 'test'},
{_number: 3, project: 'test/test'},
]));
- // When opt_query !instanceof Array, fetchJSON returns
+ // When opt_query !instanceof Array, _fetchJSON returns
// Array<Object>.
return element.getChanges().then(() => {
assert.equal(Object.keys(element._projectLookup).length, 3);
@@ -1104,10 +1110,12 @@
test('_getChangeURLAndFetch', () => {
element._projectLookup = {1: 'test'};
- const fetchStub = sandbox.stub(element, 'fetchJSON')
+ const fetchStub = sandbox.stub(element, '_fetchJSON')
.returns(Promise.resolve());
- return element._getChangeURLAndFetch(1, '/test', 1).then(() => {
- assert.isTrue(fetchStub.calledWith('/changes/test~1/revisions/1/test'));
+ const req = {changeNum: 1, endpoint: '/test', patchNum: 1};
+ return element._getChangeURLAndFetch(req).then(() => {
+ assert.equal(fetchStub.lastCall.args[0].url,
+ '/changes/test~1/revisions/1/test');
});
});
@@ -1170,8 +1178,8 @@
const range = {basePatchNum: 'PARENT', patchNum: 2};
return element.getChangeFiles(123, range).then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 2);
- assert.isNotOk(fetchStub.lastCall.args[5]);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 2);
+ assert.isNotOk(fetchStub.lastCall.args[0].params);
});
});
@@ -1181,10 +1189,10 @@
const range = {basePatchNum: 4, patchNum: 5};
return element.getChangeFiles(123, range).then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 5);
- assert.isOk(fetchStub.lastCall.args[5]);
- assert.equal(fetchStub.lastCall.args[5].base, 4);
- assert.isNotOk(fetchStub.lastCall.args[5].parent);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 5);
+ assert.isOk(fetchStub.lastCall.args[0].params);
+ assert.equal(fetchStub.lastCall.args[0].params.base, 4);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.parent);
});
});
@@ -1194,10 +1202,10 @@
const range = {basePatchNum: -3, patchNum: 5};
return element.getChangeFiles(123, range).then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 5);
- assert.isOk(fetchStub.lastCall.args[5]);
- assert.isNotOk(fetchStub.lastCall.args[5].base);
- assert.equal(fetchStub.lastCall.args[5].parent, 3);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 5);
+ assert.isOk(fetchStub.lastCall.args[0].params);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.base);
+ assert.equal(fetchStub.lastCall.args[0].params.parent, 3);
});
});
});
@@ -1208,10 +1216,10 @@
.returns(Promise.resolve());
return element.getDiff(123, 'PARENT', 2, 'foo/bar.baz').then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 2);
- assert.isOk(fetchStub.lastCall.args[5]);
- assert.isNotOk(fetchStub.lastCall.args[5].parent);
- assert.isNotOk(fetchStub.lastCall.args[5].base);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 2);
+ assert.isOk(fetchStub.lastCall.args[0].params);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.parent);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.base);
});
});
@@ -1220,10 +1228,10 @@
.returns(Promise.resolve());
return element.getDiff(123, 4, 5, 'foo/bar.baz').then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 5);
- assert.isOk(fetchStub.lastCall.args[5]);
- assert.isNotOk(fetchStub.lastCall.args[5].parent);
- assert.equal(fetchStub.lastCall.args[5].base, 4);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 5);
+ assert.isOk(fetchStub.lastCall.args[0].params);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.parent);
+ assert.equal(fetchStub.lastCall.args[0].params.base, 4);
});
});
@@ -1232,10 +1240,10 @@
.returns(Promise.resolve());
return element.getDiff(123, -3, 5, 'foo/bar.baz').then(() => {
assert.isTrue(fetchStub.calledOnce);
- assert.equal(fetchStub.lastCall.args[2], 5);
- assert.isOk(fetchStub.lastCall.args[5]);
- assert.isNotOk(fetchStub.lastCall.args[5].base);
- assert.equal(fetchStub.lastCall.args[5].parent, 3);
+ assert.equal(fetchStub.lastCall.args[0].patchNum, 5);
+ assert.isOk(fetchStub.lastCall.args[0].params);
+ assert.isNotOk(fetchStub.lastCall.args[0].params.base);
+ assert.equal(fetchStub.lastCall.args[0].params.parent, 3);
});
});
});
@@ -1245,7 +1253,7 @@
element.getDashboard('gerrit/project', 'default:main');
assert.isTrue(fetchStub.calledOnce);
assert.equal(
- fetchStub.lastCall.args[0],
+ fetchStub.lastCall.args[0].url,
'/projects/gerrit%2Fproject/dashboards/default%3Amain');
});
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 6cf674a..6a562fc 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -88,6 +88,7 @@
'core/gr-error-manager/gr-error-manager_test.html',
'core/gr-main-header/gr-main-header_test.html',
'core/gr-navigation/gr-navigation_test.html',
+ 'core/gr-reporting/gr-jank-detector_test.html',
'core/gr-reporting/gr-reporting_test.html',
'core/gr-router/gr-router_test.html',
'core/gr-search-bar/gr-search-bar_test.html',
