opendev/gerrit
Code Issues Proposed changes

Merge branch 'stable-3.1' into stable-3.2

Browse Source 
* stable-3.1:
  Add metric monitoring Java deadlocks
  Fix wrong status returned when auth backend couldn't be reached
  Add debug logs for copying approvals to new patch sets
  Revert "ApprovalCopier: Add debug logging of logic in canCopy"
  ChangeKindCacheImpl: Add debug logging of detected change kind
  ApprovalCopier: Add debug logging of logic in canCopy
  Documentation: Fix plugins install REST API & curl example
  Fix minor typo in WORKSPACE
  Fix `parent` in `gr-change-metadata` when switching patchsets
  Update git submodules
  Update git submodules
  Revert "Fix edit diff url"
  Upgrade testcontainers to 1.14.2
  Update git submodules
  Use UUID rather than group_id for links in gr-admin-group-list
  Fix Postgresql JDBC driver leaking memory

Change-Id: Ic01d45b4002889d2abb1e4813672052d5ab120ed
This commit is contained in:
David Pursehouse
2020-05-21 14:28:11 +09:00
parent 637c967c3f 7b6db6b5e5
commit f7d7df41ee
15 changed files with 123 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
Documentation/metrics.txt
View File

@@ -54,6 +54,8 @@ objects needing finalization.
* `proc/jvm/thread/num_daemon_live`: Current live daemon threads count.
* `proc/jvm/thread/num_peak_live`: Peak live thread count since the Java virtual machine started or peak was reset.
* `proc/jvm/thread/num_total_started`: Total number of threads created and also started since the Java virtual machine started.
* `proc/jvm/thread/num_deadlocked_threads`: Number of threads that are deadlocked waiting for object monitors or ownable synchronizers.
If deadlocks waiting for ownable synchronizers can be monitored depends on the capabilities of the used JVM.
=== Caches

13
Documentation/rest-api-plugins.txt
View File

@@ -246,7 +246,7 @@ List all plugins that match substring `project`:
[[install-plugin]]
=== Install Plugin
--
'PUT /plugins/link:#plugin-id[\{plugin-id\}]'
'PUT /plugins/link:#plugin-id[\{plugin-id\}].jar'
--
Installs a new plugin on the Gerrit server. If a plugin with the
@@ -260,7 +260,7 @@ a link:#plugin-input[PluginInput] entity.
.Request
----
PUT /plugins/delete-project HTTP/1.0
PUT /plugins/delete-project.jar HTTP/1.0
Content-Type: application/json; charset=UTF-8
{
@@ -272,7 +272,7 @@ To provide the plugin jar as binary data in the request body the
following curl command can be used:
----
curl --user admin:TNNuLkWsIV8w -X PUT --data-binary @delete-project-2.8.jar 'http://gerrit:8080/a/plugins/delete-project'
curl --user admin:TNNuLkWsIV8w -X PUT -H "Content-Type:application/octet-stream" --data-binary @delete-project.jar 'http://gerrit:8080/a/plugins/delete-project.jar'
----
As response a link:#plugin-info[PluginInfo] entity is returned that
@@ -282,12 +282,15 @@ describes the plugin.
----
HTTP/1.1 201 Created
Content-Disposition: attachment
Content-Type: application/json; charset=UTF-8
Content-Type: application/json;charset=utf-8
Content-Length: 150
)]}'
{
"id": "delete-project",
"version": "2.8"
"version": "v2.16-221-g35bb8bbac4",
"index_url": "plugins/delete-project/",
"filename": "delete-project.jar"
}
----

2
WORKSPACE
View File

@@ -267,7 +267,7 @@ maven_jar(
CAFFEINE_GUAVA_SHA256 = "3a66ee3ec70971dee0bae6e56bda7b8742bc4bedd7489161bfbbaaf7137d89e1"
# TODO(davido): Rename guava.jar to caffeine-guava.jar on fetch to prevent potential
# naming collision between caffeine guava adapater and guava library itself.
# naming collision between caffeine guava adapter and guava library itself.
# Remove this renaming procedure, once this upstream issue is fixed:
# https://github.com/ben-manes/caffeine/issues/364.
http_file(

6
java/com/google/gerrit/httpd/ProjectBasicAuthFilter.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.httpd;
import static java.nio.charset.StandardCharsets.UTF_8;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import static javax.servlet.http.HttpServletResponse.SC_UNAUTHORIZED;
import com.google.common.base.MoreObjects;
@@ -33,6 +34,7 @@ import com.google.gerrit.server.account.AuthRequest;
import com.google.gerrit.server.account.AuthResult;
import com.google.gerrit.server.account.AuthenticationFailedException;
import com.google.gerrit.server.account.externalids.PasswordVerifier;
import com.google.gerrit.server.auth.AuthenticationUnavailableException;
import com.google.gerrit.server.auth.NoSuchUserException;
import com.google.gerrit.server.config.AuthConfig;
import com.google.inject.Inject;
@@ -170,6 +172,10 @@ class ProjectBasicAuthFilter implements Filter {
logger.atWarning().log(authenticationFailedMsg(username, req) + ": %s", e.getMessage());
rsp.sendError(SC_UNAUTHORIZED);
return false;
} catch (AuthenticationUnavailableException e) {
logger.atSevere().withCause(e).log("could not reach authentication backend");
rsp.sendError(SC_SERVICE_UNAVAILABLE);
return false;
} catch (AccountException e) {
logger.atWarning().withCause(e).log(authenticationFailedMsg(username, req));
rsp.sendError(SC_UNAUTHORIZED);

31
java/com/google/gerrit/metrics/proc/ProcMetricModule.java
View File

@@ -234,5 +234,36 @@ public class ProcMetricModule extends MetricModule {
.setGauge()
.setUnit("threads"),
thread::getTotalStartedThreadCount);
if (thread.isSynchronizerUsageSupported()) {
metrics.newCallbackMetric(
"proc/jvm/thread/num_deadlocked_threads",
Integer.class,
new Description(
"number of threads that are deadlocked waiting for object monitors or ownable synchronizers")
.setGauge()
.setUnit("threads"),
() -> {
long[] deadlocked = thread.findDeadlockedThreads();
if (deadlocked == null) {
return 0;
}
return deadlocked.length;
});
} else {
metrics.newCallbackMetric(
"proc/jvm/thread/num_deadlocked_threads",
Integer.class,
new Description(
"number of threads that are deadlocked waiting to acquire object monitors")
.setGauge()
.setUnit("threads"),
() -> {
long[] deadlocked = thread.findMonitorDeadlockedThreads();
if (deadlocked == null) {
return 0;
}
return deadlocked.length;
});
}
}
}

10
java/com/google/gerrit/server/change/ChangeKindCacheImpl.java
View File

@@ -344,10 +344,12 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
ObjectId next) {
try {
Key key = Key.create(prior, next, useRecursiveMerge);
return cache.get(key, new Loader(key, repoManager, project, rw, repoConfig));
ChangeKind kind = cache.get(key, new Loader(key, repoManager, project, rw, repoConfig));
logger.atFine().log("Change kind of new patch set %s in %s: %s", next.name(), project, kind);
return kind;
} catch (ExecutionException e) {
logger.atWarning().withCause(e).log(
"Cannot check trivial rebase of new patch set %s in %s", next.name(), project);
"Cannot check change kind of new patch set %s in %s", next.name(), project);
return ChangeKind.REWORK;
}
}
@@ -400,6 +402,8 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
patch.number(), change.getId());
}
}
logger.atFine().log(
"Change kind for patchSet %s of change %s: %s", patch.number(), change.getId(), kind);
return kind;
}
@@ -426,6 +430,8 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
patch.number(), change.getChangeId());
}
}
logger.atFine().log(
"Change kind for patchSet %s of change %s: %s", patch.number(), change.getChangeId(), kind);
return kind;
}
}

7
polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list.js
View File

@@ -123,8 +123,13 @@ class GrAdminGroupList extends mixinBehaviors( [
}
}
/**
* Generates groups link (/admin/groups/<uuid>)
*
* @param {string} id
*/
_computeGroupUrl(id) {
return GerritNav.getUrlForGroup(id);
return GerritNav.getUrlForGroup(decodeURIComponent(id));
}
_getCreateGroupCapability() {

2
polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_html.js
View File

@@ -48,7 +48,7 @@ export const htmlTemplate = html`
<template is="dom-repeat" items="[[_shownGroups]]">
<tr class="table">
<td class="name">
<a href$="[[_computeGroupUrl(item.group_id)]]">[[item.name]]</a>
<a href$="[[_computeGroupUrl(item.id)]]">[[item.name]]</a>
</td>
<td class="description">[[item.description]]</td>
<td class="visibleToAll">[[_visibleToAll(item)]]</td>

25
polygerrit-ui/app/elements/admin/gr-admin-group-list/gr-admin-group-list_test.html
View File

@@ -35,6 +35,7 @@ limitations under the License.
<script type="module">
import '../../../test/common-test-setup.js';
import './gr-admin-group-list.js';
import {GerritNav} from '../../core/gr-navigation/gr-navigation.js';
let counter = 0;
const groupGenerator = () => {
return {
@@ -66,6 +67,30 @@ suite('gr-admin-group-list tests', () => {
sandbox.restore();
});
test('_computeGroupUrl', () => {
let urlStub = sandbox.stub(GerritNav, 'getUrlForGroup',
() => '/admin/groups/e2cd66f88a2db4d391ac068a92d987effbe872f5');
let group = {
id: 'e2cd66f88a2db4d391ac068a92d987effbe872f5',
};
assert.equal(element._computeGroupUrl(group),
'/admin/groups/e2cd66f88a2db4d391ac068a92d987effbe872f5');
urlStub.restore();
urlStub = sandbox.stub(GerritNav, 'getUrlForGroup',
() => '/admin/groups/user/test');
group = {
id: 'user%2Ftest',
};
assert.equal(element._computeGroupUrl(group),
'/admin/groups/user/test');
urlStub.restore();
});
suite('list with groups', () => {
setup(done => {
groups = _.times(26, groupGenerator);

10
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
View File

@@ -152,7 +152,7 @@ class GrChangeMetadata extends mixinBehaviors( [
_currentParents: {
type: Array,
computed: '_computeParents(change)',
computed: '_computeParents(revision)',
},
/** @type {?} */
@@ -494,13 +494,11 @@ class GrChangeMetadata extends mixinBehaviors( [
return null;
}
_computeParents(change) {
if (!change || !change.current_revision ||
!change.revisions[change.current_revision] ||
!change.revisions[change.current_revision].commit) {
_computeParents(revision) {
if (!revision || !revision.commit) {
return undefined;
}
return change.revisions[change.current_revision].commit.parents;
return revision.commit.parents;
}
_computeParentsLabel(parents) {

22
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
View File

@@ -426,14 +426,20 @@ suite('gr-change-metadata tests', () => {
});
test('_computeParents', () => {
const parents = [{commit: '123', subject: 'abc'}];
assert.isUndefined(element._computeParents(
{revisions: {456: {commit: {parents}}}}));
assert.isUndefined(element._computeParents(
{current_revision: '789', revisions: {456: {commit: {parents}}}}));
assert.equal(element._computeParents(
{current_revision: '456', revisions: {456: {commit: {parents}}}}),
parents);
const revision = {commit: {parents: [{commit: '123', subject: 'abc'}]}};
assert.isUndefined(element._computeParents({}));
assert.equal(element._computeParents(revision), revision.commit.parents);
});
test('_currentParents', () => {
element.revision = {
commit: {parents: [{commit: '123', subject: 'abc'}]},
};
assert.equal(element._currentParents[0].commit, '123');
element.revision = {
commit: {parents: [{commit: '12345', subject: 'abc'}]},
};
assert.equal(element._currentParents[0].commit, '12345');
});
test('_computeParentsLabel', () => {

3
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
View File

@@ -646,7 +646,8 @@ class GrDiffView extends mixinBehaviors( [
_goToEditFile() {
// TODO(taoalpha): add a shortcut for editing
const editUrl = GerritNav.getEditUrlForDiff(this._change, this._path);
const editUrl = GerritNav.getEditUrlForDiff(
this._change, this._path, this._patchRange.patchNum);
return GerritNav.navigateToRelativeUrl(editUrl);
}

3
polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.html
View File

@@ -403,7 +403,6 @@ suite('gr-diff-view tests', () => {
};
element._change = {
_number: 42,
project: 'gerrit',
status: 'NEW',
revisions: {
a: {_number: 1, commit: {parents: []}},
@@ -417,8 +416,6 @@ suite('gr-diff-view tests', () => {
assert.isTrue(!!editBtn);
MockInteractions.tap(editBtn);
assert.isTrue(redirectStub.called);
assert.isTrue(redirectStub.lastCall.calledWithExactly(
GerritNav.getEditUrlForDiff(element._change, element._path)));
done();
});
});

15
polygerrit-ui/app/elements/settings/gr-group-list/gr-group-list_test.html
View File

@@ -95,21 +95,30 @@ suite('gr-group-list tests', () => {
});
test('_computeGroupPath', () => {
sandbox.stub(GerritNav, 'getUrlForGroup',
let urlStub = sandbox.stub(GerritNav, 'getUrlForGroup',
() => '/admin/groups/e2cd66f88a2db4d391ac068a92d987effbe872f5');
let group = {
id: 'e2cd66f88a2db4d391ac068a92d987effbe872f5',
};
assert.equal(element._computeGroupPath(group),
'/admin/groups/e2cd66f88a2db4d391ac068a92d987effbe872f5');
group = {
name: 'admin',
};
assert.isUndefined(element._computeGroupPath(group));
urlStub.restore();
urlStub = sandbox.stub(GerritNav, 'getUrlForGroup',
() => '/admin/groups/user/test');
group = {
id: 'user%2Ftest',
};
assert.equal(element._computeGroupPath(group),
'/admin/groups/user/test');
});
});
</script>

6
tools/nongoogle.bzl
View File

@@ -120,18 +120,18 @@ def declare_nongoogle_deps():
sha1 = "dc13ae4faca6df981fc7aeb5a522d9db446d5d50",
)
TESTCONTAINERS_VERSION = "1.14.1"
TESTCONTAINERS_VERSION = "1.14.2"
maven_jar(
name = "testcontainers",
artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
sha1 = "defd04ff6ffc93e1ff988024048e8ba5bd298df3",
sha1 = "d74bc045fb5f30988b0adff20244412972a9f564",
)
maven_jar(
name = "testcontainers-elasticsearch",
artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
sha1 = "d682965bbf1334ef40720b4ad2eda2c12bf0b440",
sha1 = "66e1a6da0362beee83673b877c9c2e0536d6912c",
)
maven_jar(