diff --git a/Documentation/cmd-ls-groups.txt b/Documentation/cmd-ls-groups.txt index a0c7c344c4..651cebe325 100644 --- a/Documentation/cmd-ls-groups.txt +++ b/Documentation/cmd-ls-groups.txt @@ -10,7 +10,6 @@ gerrit ls-groups - List groups visible to caller [--user | -u ] [--owned] [--visible-to-all] - [--type {internal | system}] [-q ] [--verbose | -v] -- @@ -67,15 +66,6 @@ This option can't be used together with the '--project' option. (groups that are explicitly marked as visible to all registered users). ---type:: - Display only groups of the specified type. If not specified, - groups of all types are displayed. Supported types: -+ --- -`internal`:: Any group defined within Gerrit. -`system`:: Any system defined and managed group. --- - -q:: Group that should be inspected. The `-q` option can be specified multiple times to define several groups to be inspected. If diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 3dda56cfce..ec73a32f3f 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -1098,7 +1098,7 @@ how the replacement is displayed to the user. ---- [commentlink "changeid"] match = (I[0-9a-f]{8,40}) - link = "#q,$1" + link = "#/q/$1" [commentlink "bugzilla"] match = "(bug\\s+#?)(\\d+)" @@ -2405,10 +2405,14 @@ Defaults to no limit. + Maximum number of leaf terms to allow in a query. Too-large queries may perform poorly, so setting this option causes query parsing to fail fast -before attempting to send them to the secondary index. Set to 0 for no -limit. +before attempting to send them to the secondary index. Should this limit +be reached, database is used instead of index as applicable. + -Defaults to 500. +When the index type is `LUCENE`, also sets the maximum number of clauses +permitted per BooleanQuery. This is so that all enforced query limits +are the same. ++ +Defaults to 1024. ==== Lucene configuration @@ -2417,14 +2421,6 @@ Open and closed changes are indexed in separate indexes named The following settings are only used when the index type is `LUCENE`. -[[index.defaultMaxClauseCount]]index.defaultMaxClauseCount:: -+ -Only used when the type is `LUCENE`. -+ -Sets the maximum number of clauses permitted per BooleanQuery. -+ -Defaults to 1024. - [[index.name.ramBufferSize]]index.name.ramBufferSize:: + Determines the amount of RAM that may be used for buffering added documents @@ -2468,7 +2464,6 @@ Sample Lucene index configuration: ---- [index] type = LUCENE - defaultMaxClauseCount = 2048 [index "changes_open"] ramBufferSize = 60 m diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index ac4d9a0d76..408e46be9f 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -496,6 +496,9 @@ For the development mode email addresses are directly added without confirmation. A Gerrit administrator may add an email address without confirmation by setting `no_confirmation` in the link:#email-input[EmailInput]. +If link:config-gerrit.html#sendemail.allowrcpt[sendemail.allowrcpt] is +configured, the added email address must belong to a domain that is +allowed, unless `no_confirmation` is set. In the request body additional data for the email address can be provided as link:#email-input[EmailInput]. diff --git a/ReleaseNotes/ReleaseNotes-2.11.6.txt b/ReleaseNotes/ReleaseNotes-2.11.6.txt new file mode 100644 index 0000000000..d6f939f3c5 --- /dev/null +++ b/ReleaseNotes/ReleaseNotes-2.11.6.txt @@ -0,0 +1,130 @@ +Release notes for Gerrit 2.11.6 +=============================== + +Gerrit 2.11.6 is now available: + +link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.6.war[ +https://gerrit-releases.storage.googleapis.com/gerrit-2.11.6.war] + +There are no schema changes from link:ReleaseNotes-2.11.5.html[2.11.5]. + +Bug Fixes +--------- + +General +~~~~~~~ + +* link:https://code.google.com/p/gerrit/issues/detail?id=3742[Issue 3742]: +Use merge strategy for mergeability testing on 'Rebase if Necessary' strategy. ++ +When pushing several interdependent commits to a project with the +'Rebase if Necessary' strategy, all the commits except the first one were +marked as 'Cannot merge'. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3762[Issue 3762]: +Fix server error when querying changes with the `query` ssh command. + +* Fix server error when listing annotated/signed tag that has no tagger info. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3698[Issue 3698]: +Fix creation of the administrator user on databases with pre-allocated +auto-increment column values. ++ +When using a database configuration where auto-increment column values are +pre-allocated, it was possible that the 'Administrators' group was created +with an ID other than `1`. In this case, the created admin user was not added +to the correct group, and did not have the correct admin permissions. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3018[Issue 3018]: +Fix query for changes using a label with a group operator. ++ +The `group` operator was being ignored when searching for changes with labels +because the search index does not contain group information. + +* Fix online reindexing of changes that don't already exist in the index. ++ +Changes are now always reloaded from the database during online reindex. + +* Fix reviewer suggestion for accounts containing upper case letters. ++ +When an email for an account contained upper-case letter(s), this account +couldn't be added as a reviewer by selecting it from the suggested list of +accounts. + +Authentication +~~~~~~~~~~~~~~ + +* Fix handling of lowercase HTTP username. ++ +When `auth.userNameToLowerCase` is set to true the HTTP-provided username +should be converted to lowercase as it is done on all the other authentication +mechanisms. + +* Don't create new account when claimed OAuth identity is unknown. ++ +The Claimed Identity feature was enabled to support old Google OpenID accounts, +that cannot be activated anymore. In some corner cases, when for example the URL +is not from the production Gerrit site, for example on a staging instance, the +OpenID identity may deviate from the original one. In case of mismatch, the lookup +of the user for the claimed identity would fail, causing a new account to be +created. + +UI +~~ + +* link:https://code.google.com/p/gerrit/issues/detail?id=3714[Issue 3714]: +Improve visibility of comments on dark themes. + +* Fix highlighting of search results and trailing whitespaces in intraline +diff chunks. + +Plugins +~~~~~~~ + +* link:https://code.google.com/p/gerrit/issues/detail?id=3768[Issue 3768]: +Fix usage of `EqualsFilePredicate` in plugins. + +* Suggest to upgrade installed plugins per default during site initialization +to new Gerrit version. ++ +The default was 'No' which resulted in some sites not upgrading core +plugins and running the wrong versions. + +* Fix reading of plugin documentation. ++ +Under some circumstances it was possible to fail with an IO error. + +* Replication + +** Recursively include parent groups of groups specified in `authGroup`. ++ +An `authGroup` could be included in other groups and should be granted the +same permission as its parents. + +** Put back erroneously removed documentation of `remote.NAME.timeout`. + +** Add logging of cancelled replication events. + +* API + +** Allow to use `CurrentSchemaVersion`. + +** Allow to use `InternalChangeQuery.query()`. + +** Allow to use `JdbcUtil.port()`. + +** Allow to use GWTORM `Key` classes. + +Documentation Updates +--------------------- + +* link:https://code.google.com/p/gerrit/issues/detail?id=412[Issue 412]: +Update documentation of `commentlink.match` regular expression to clarify +that the expression is applied to the rendered HTML. + +* Remove warning about unstable change edit REST API endpoints. ++ +These endpoints should be considered stable since version 2.11. + +* Document that `ldap.groupBase` and `ldap.accountBase` are repeatable. + diff --git a/ReleaseNotes/ReleaseNotes-2.11.7.txt b/ReleaseNotes/ReleaseNotes-2.11.7.txt new file mode 100644 index 0000000000..7a0de2db29 --- /dev/null +++ b/ReleaseNotes/ReleaseNotes-2.11.7.txt @@ -0,0 +1,44 @@ +Release notes for Gerrit 2.11.7 +=============================== + +Gerrit 2.11.7 is now available: + +link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war[ +https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war] + +There are no schema changes from link:ReleaseNotes-2.11.6.html[2.11.6]. + +Bug Fixes +--------- + +* link:https://code.google.com/p/gerrit/issues/detail?id=3882[Issue 3882]: +Fix 'No user on email thread' exception when label with group parameter is +used in watched projects predicate. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3877[Issue 3877]: +Include files in output when using `gerrit query` with combination of +search operators. ++ +A regression introduced in version 2.11.6 caused files to be omitted +in the output. + +* Include comments in output when using `gerrit query` with the +`--current-patch-set` option. ++ +Comments were added at the change level but were not added at the +patch set level. + +* Honor the `sendemail.allowrcpt` setting when adding new email address. ++ +When adding a new email address via the UI or REST API, it was possible for +the user to add an address that does not belong to a domain allowed by the +`sendemail.allowrcpt` configuration. However, when sending the verification +email, the recipient address was (correctly) dropped, and the email had no +recipients. This resulted in an error from the SMTP server and an 'Internal +server error' message to the user. + +* Remove unnecessary log messages. ++ +The messages 'Assuming empty group membership' and 'Skipping delivery of +email' do not add any value and were filling up the error log. + diff --git a/ReleaseNotes/ReleaseNotes-2.12.txt b/ReleaseNotes/ReleaseNotes-2.12.txt index d6c33f6164..797a13854a 100644 --- a/ReleaseNotes/ReleaseNotes-2.12.txt +++ b/ReleaseNotes/ReleaseNotes-2.12.txt @@ -376,6 +376,16 @@ menu on the change screen. Plugins can refer to groups so that when they are renamed, the project config will also be updated in this section. +* API + +** Allow to use `CurrentSchemaVersion`. + +** Allow to use `InternalChangeQuery.query()`. + +** Allow to use `JdbcUtil.port()`. + +** Allow to use GWTORM `Key` classes. + Other ~~~~~ @@ -480,6 +490,65 @@ Users who are unable to upgrade the plugin may instead change the trigger's branch configuration to type `Path` with a pattern like `refs/*/master` instead of `Plain` and `master`. +* link:https://code.google.com/p/gerrit/issues/detail?id=3714[Issue 3714]: +Improve visibility of comments on dark themes. + +* Fix highlighting of search results and trailing whitespaces in intraline +diff chunks. + +* Fix server error when listing annotated/signed tag that has no tagger info. + +* Don't create new account when claimed OAuth identity is unknown. ++ +The Claimed Identity feature was enabled to support old Google OpenID accounts, +that cannot be activated anymore. In some corner cases, when for example the URL +is not from the production Gerrit site, for example on a staging instance, the +OpenID identity may deviate from the original one. In case of mismatch, the lookup +of the user for the claimed identity would fail, causing a new account to be +created. + +* Suggest to upgrade installed plugins per default during site initialization +to new Gerrit version. ++ +The default was 'No' which resulted in some sites not upgrading core +plugins and running the wrong versions. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3698[Issue 3698]: +Fix creation of the administrator user on databases with pre-allocated +auto-increment column values. ++ +When using a database configuration where auto-increment column values are +pre-allocated, it was possible that the 'Administrators' group was created +with an ID other than `1`. In this case, the created admin user was not added +to the correct group, and did not have the correct admin permissions. + +* link:https://code.google.com/p/gerrit/issues/detail?id=3018[Issue 3018]: +Fix query for changes using a label with a group operator. ++ +The `group` operator was being ignored when searching for changes with labels +because the search index does not contain group information. + +* Fix online reindexing of changes that don't already exist in the index. ++ +Changes are now always reloaded from the database during online reindex. + +* Fix reading of plugin documentation. ++ +Under some circumstances it was possible to fail with an IO error. + +Documentation Updates +--------------------- + +* link:https://code.google.com/p/gerrit/issues/detail?id=412[Issue 412]: +Update documentation of `commentlink.match` regular expression to clarify +that the expression is applied to the rendered HTML. + +* Remove warning about unstable change edit REST API endpoints. ++ +These endpoints should be considered stable since version 2.11. + +* Document that `ldap.groupBase` and `ldap.accountBase` are repeatable. + Upgrades -------- diff --git a/ReleaseNotes/index.txt b/ReleaseNotes/index.txt index e15a845a5d..fa57dab590 100644 --- a/ReleaseNotes/index.txt +++ b/ReleaseNotes/index.txt @@ -14,6 +14,8 @@ Version 2.12.x [[2_11]] Version 2.11.x -------------- +* link:ReleaseNotes-2.11.7.html[2.11.7] +* link:ReleaseNotes-2.11.6.html[2.11.6] * link:ReleaseNotes-2.11.5.html[2.11.5] * link:ReleaseNotes-2.11.4.html[2.11.4] * link:ReleaseNotes-2.11.3.html[2.11.3] diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java index 00bb78cda7..c2b32d656a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -159,7 +159,7 @@ public class ChangeEditIT extends AbstractDaemonTest { assertChangeMessages(change, ImmutableList.of("Uploaded patch set 1.", "Uploaded patch set 2.", - "Patch set 3: Published edit on patch set 2.")); + "Patch Set 3: Published edit on patch set 2.")); } @Test @@ -179,7 +179,7 @@ public class ChangeEditIT extends AbstractDaemonTest { assertChangeMessages(change, ImmutableList.of("Uploaded patch set 1.", "Uploaded patch set 2.", - "Patch set 3: Published edit on patch set 2.")); + "Patch Set 3: Published edit on patch set 2.")); } @Test @@ -357,7 +357,7 @@ public class ChangeEditIT extends AbstractDaemonTest { assertChangeMessages(change, ImmutableList.of("Uploaded patch set 1.", "Uploaded patch set 2.", - "Patch set 3: Commit message was updated.")); + "Patch Set 3: Commit message was updated.")); } @Test @@ -384,7 +384,7 @@ public class ChangeEditIT extends AbstractDaemonTest { assertChangeMessages(change, ImmutableList.of("Uploaded patch set 1.", "Uploaded patch set 2.", - "Patch set 3: Commit message was updated.")); + "Patch Set 3: Commit message was updated.")); } @Test diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java index ed6740ae76..7d2930a2a3 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java @@ -55,6 +55,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest { private TestAccount user1; private TestAccount user2; private TestAccount user3; + private TestAccount user4; @Before public void setUp() throws Exception { @@ -65,6 +66,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest { user1 = user("user1", "First1 Last1", group1); user2 = user("user2", "First2 Last2", group2); user3 = user("user3", "First3 Last3", group1, group2); + user4 = user("jdoe", "John Doe", "JDOE"); } @Test @@ -205,14 +207,18 @@ public class SuggestReviewersIT extends AbstractDaemonTest { reviewers = suggestReviewers(changeId, user1.username, 2); assertThat(reviewers).hasSize(1); - reviewers = suggestReviewers(changeId, "example.com", 6); - assertThat(reviewers).hasSize(5); + reviewers = suggestReviewers(changeId, "example.com", 7); + assertThat(reviewers).hasSize(6); reviewers = suggestReviewers(changeId, user1.email, 2); assertThat(reviewers).hasSize(1); reviewers = suggestReviewers(changeId, user1.username + " example", 2); assertThat(reviewers).hasSize(1); + + reviewers = suggestReviewers(changeId, user4.email.toLowerCase(), 2); + assertThat(reviewers).hasSize(1); + assertThat(reviewers.get(0).account.email).isEqualTo(user4.email); } @Test @@ -254,9 +260,8 @@ public class SuggestReviewersIT extends AbstractDaemonTest { return GroupDescriptions.toAccountGroup(d); } - private TestAccount user(String name, String fullName, AccountGroup... groups) - throws Exception { - name = name(name); + private TestAccount user(String name, String fullName, String emailName, + AccountGroup... groups) throws Exception { String[] groupNames = FluentIterable.from(Arrays.asList(groups)) .transform(new Function() { @Override @@ -264,6 +269,12 @@ public class SuggestReviewersIT extends AbstractDaemonTest { return in.getName(); } }).toArray(String.class); - return accounts.create(name, name + "@example.com", fullName, groupNames); + return accounts.create(name(name), name(emailName) + "@example.com", + fullName, groupNames); + } + + private TestAccount user(String name, String fullName, AccountGroup... groups) + throws Exception { + return user(name, fullName, name, groups); } } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java new file mode 100644 index 0000000000..066d3628d7 --- /dev/null +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java @@ -0,0 +1,320 @@ +// 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. + +package com.google.gerrit.acceptance.ssh; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assert_; + +import com.google.common.collect.Lists; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.NoHttpd; +import com.google.gerrit.acceptance.PushOneCommit; +import com.google.gerrit.extensions.api.changes.AddReviewerInput; +import com.google.gerrit.extensions.api.changes.ReviewInput; +import com.google.gerrit.extensions.client.Side; +import com.google.gerrit.server.data.ChangeAttribute; +import com.google.gson.Gson; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; + +@NoHttpd +public class QueryIT extends AbstractDaemonTest { + + private static Gson gson = new Gson(); + + @Test + public void testBasicQueryJSON() throws Exception { + String changeId1 = createChange().getChangeId(); + String changeId2 = createChange().getChangeId(); + + List changes = executeSuccessfulQuery("1234"); + assertThat(changes.size()).isEqualTo(0); + + changes = executeSuccessfulQuery(changeId1); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).project).isEqualTo(project.toString()); + assertThat(changes.get(0).id).isEqualTo(changeId1); + + changes = executeSuccessfulQuery(changeId1 + " OR " + changeId2); + assertThat(changes.size()).isEqualTo(2); + assertThat(changes.get(0).project).isEqualTo(project.toString()); + assertThat(changes.get(0).id).isEqualTo(changeId2); + assertThat(changes.get(1).project).isEqualTo(project.toString()); + assertThat(changes.get(1).id).isEqualTo(changeId1); + + changes = + executeSuccessfulQuery("--start=1 " + changeId1 + " OR " + changeId2); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).project).isEqualTo(project.toString()); + assertThat(changes.get(0).id).isEqualTo(changeId1); + } + + @Test + public void testAllApprovalsOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + gApi.changes().id(changeId).current().review(ReviewInput.approve()); + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets).isNull(); + + changes = executeSuccessfulQuery("--all-approvals " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1); + } + + @Test + public void testAllReviewersOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + AddReviewerInput in = new AddReviewerInput(); + in.reviewer = user.email; + gApi.changes().id(changeId).addReviewer(in); + + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).allReviewers).isNull(); + + changes = executeSuccessfulQuery("--all-reviewers " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).allReviewers).isNotNull(); + assertThat(changes.get(0).allReviewers.size()).isEqualTo(1); + } + + @Test + public void testCommitMessageOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + List changes = + executeSuccessfulQuery("--commit-message " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).commitMessage).isNotNull(); + assertThat(changes.get(0).commitMessage).contains(PushOneCommit.SUBJECT); + } + + @Test + public void testCurrentPatchSetOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + amendChange(changeId); + + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet).isNull(); + + changes = executeSuccessfulQuery("--current-patch-set " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet).isNotNull(); + assertThat(changes.get(0).currentPatchSet.number).isEqualTo("2"); + + gApi.changes().id(changeId).current().review(ReviewInput.approve()); + changes = executeSuccessfulQuery("--current-patch-set " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet).isNotNull(); + assertThat(changes.get(0).currentPatchSet.approvals).isNotNull(); + assertThat(changes.get(0).currentPatchSet.approvals.size()).isEqualTo(1); + + } + + @Test + public void testPatchSetsOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + amendChange(changeId); + amendChange(changeId); + + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets).isNull(); + + changes = executeSuccessfulQuery("--patch-sets " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets).isNotNull(); + assertThat(changes.get(0).patchSets.size()).isEqualTo(3); + } + + @Test + public void shouldFailWithFilesWithoutPatchSetsOrCurrentPatchSetsOption() + throws Exception { + String changeId = createChange().getChangeId(); + sshSession.exec("gerrit query --files " + changeId); + assertThat(sshSession.hasError()).isTrue(); + assertThat(sshSession.getError()).contains( + "needs --patch-sets or --current-patch-set"); + } + + @Test + public void testFileOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + + List changes = + executeSuccessfulQuery("--current-patch-set --files " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet.files).isNotNull(); + assertThat(changes.get(0).currentPatchSet.files.size()).isEqualTo(2); + + changes = executeSuccessfulQuery("--patch-sets --files " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).files).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2); + + gApi.changes().id(changeId).current().review(ReviewInput.approve()); + changes = + executeSuccessfulQuery("--patch-sets --files --all-approvals " + + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).files).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2); + assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1); + } + + @Test + public void testCommentOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).comments).isNull(); + + changes = executeSuccessfulQuery("--comments " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).comments).isNotNull(); + assertThat(changes.get(0).comments.size()).isEqualTo(1); + } + + @Test + public void testCommentOptionsInCurrentPatchSetJSON() throws Exception { + String changeId = createChange().getChangeId(); + + ReviewInput review = new ReviewInput(); + ReviewInput.CommentInput comment = new ReviewInput.CommentInput(); + comment.path = PushOneCommit.FILE_NAME; + comment.side = Side.REVISION; + comment.message = "comment 1"; + review.comments = new HashMap<>(); + review.comments.put(comment.path, Lists.newArrayList(comment)); + gApi.changes().id(changeId).current().review(review); + + List changes = + executeSuccessfulQuery("--current-patch-set " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet.comments).isNull(); + + changes = + executeSuccessfulQuery("--current-patch-set --comments " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).currentPatchSet.comments).isNotNull(); + assertThat(changes.get(0).currentPatchSet.comments.size()).isEqualTo(1); + } + + @Test + public void testCommentOptionInPatchSetsJSON() throws Exception { + String changeId = createChange().getChangeId(); + + ReviewInput review = new ReviewInput(); + ReviewInput.CommentInput comment = new ReviewInput.CommentInput(); + comment.path = PushOneCommit.FILE_NAME; + comment.side = Side.REVISION; + comment.message = "comment 1"; + review.comments = new HashMap<>(); + review.comments.put(comment.path, Lists.newArrayList(comment)); + gApi.changes().id(changeId).current().review(review); + + List changes = + executeSuccessfulQuery("--patch-sets " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).comments).isNull(); + + changes = executeSuccessfulQuery("--patch-sets --comments " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).comments).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1); + + changes = + executeSuccessfulQuery("--patch-sets --comments --files " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).comments).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).files).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2); + + gApi.changes().id(changeId).current().review(ReviewInput.approve()); + changes = + executeSuccessfulQuery("--patch-sets --comments --files --all-approvals " + + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).comments).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1); + assertThat(changes.get(0).patchSets.get(0).files).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2); + assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull(); + assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1); + } + + @Test + public void testDependenciesOptionJSON() throws Exception { + String changeId1 = createChange().getChangeId(); + String changeId2 = createChange().getChangeId(); + List changes = executeSuccessfulQuery(changeId1); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).dependsOn).isNull(); + + changes = executeSuccessfulQuery("--dependencies " + changeId1); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).dependsOn).isNull(); + + changes = executeSuccessfulQuery(changeId2); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).dependsOn).isNull(); + + changes = executeSuccessfulQuery("--dependencies " + changeId2); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).dependsOn).isNotNull(); + assertThat(changes.get(0).dependsOn.size()).isEqualTo(1); + } + + @Test + public void testSubmitRecordsOptionJSON() throws Exception { + String changeId = createChange().getChangeId(); + List changes = executeSuccessfulQuery(changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).submitRecords).isNull(); + + changes = executeSuccessfulQuery("--submit-records " + changeId); + assertThat(changes.size()).isEqualTo(1); + assertThat(changes.get(0).submitRecords).isNotNull(); + assertThat(changes.get(0).submitRecords.size()).isEqualTo(1); + } + + private List executeSuccessfulQuery(String params) + throws Exception { + String rawResponse = + sshSession.exec("gerrit query --format=JSON " + params); + assert_().withFailureMessage(sshSession.getError()) + .that(sshSession.hasError()).isFalse(); + return getChanges(rawResponse); + } + + private static List getChanges(String rawResponse) { + String[] lines = rawResponse.split("\\n"); + List changes = new ArrayList<>(lines.length - 1); + for (int i = 0; i < lines.length - 1; i++) { + changes.add(gson.fromJson(lines[i], ChangeAttribute.class)); + } + return changes; + } +} diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java index 1b5d2e89e1..2db0eff537 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectTagsScreen.java @@ -109,12 +109,12 @@ public class ProjectTagsScreen extends PaginatedProjectScreen { TagsTable() { table.setWidth(""); - table.setText(0, 0, Util.C.columnTagName()); - table.setText(0, 1, Util.C.columnBranchRevision()); + table.setText(0, 1, Util.C.columnTagName()); + table.setText(0, 2, Util.C.columnBranchRevision()); FlexCellFormatter fmt = table.getFlexCellFormatter(); - fmt.addStyleName(0, 0, Gerrit.RESOURCES.css().dataHeader()); fmt.addStyleName(0, 1, Gerrit.RESOURCES.css().dataHeader()); + fmt.addStyleName(0, 2, Gerrit.RESOURCES.css().dataHeader()); } void display(List tags) { @@ -135,18 +135,18 @@ public class ProjectTagsScreen extends PaginatedProjectScreen { } void populate(int row, TagInfo k) { - table.setWidget(row, 0, new InlineHTML(highlight(k.getShortName(), match))); + table.setWidget(row, 1, new InlineHTML(highlight(k.getShortName(), match))); if (k.revision() != null) { - table.setText(row, 1, k.revision()); + table.setText(row, 2, k.revision()); } else { - table.setText(row, 1, ""); + table.setText(row, 2, ""); } FlexCellFormatter fmt = table.getFlexCellFormatter(); String dataCellStyle = Gerrit.RESOURCES.css().dataCell(); - fmt.addStyleName(row, 0, dataCellStyle); fmt.addStyleName(row, 1, dataCellStyle); + fmt.addStyleName(row, 2, dataCellStyle); setRowItem(row, k); } diff --git a/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java b/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java index fa4be77caa..c439e109b3 100644 --- a/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java +++ b/gerrit-gwtui/src/main/java/net/codemirror/mode/ModeInfo.java @@ -74,6 +74,7 @@ public class ModeInfo extends JavaScriptObject { Modes.I.sql(), Modes.I.stex(), Modes.I.swift(), + Modes.I.tcl(), Modes.I.velocity(), Modes.I.verilog(), Modes.I.vhdl(), diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/DirectoryDocServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/DirectoryDocServlet.java new file mode 100644 index 0000000000..826ff95006 --- /dev/null +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/DirectoryDocServlet.java @@ -0,0 +1,35 @@ +// 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. + +package com.google.gerrit.httpd.raw; + +import com.google.common.cache.Cache; + +import java.nio.file.Path; + +class DirectoryDocServlet extends ResourceServlet { + private static final long serialVersionUID = 1L; + + private final Path doc; + + DirectoryDocServlet(Cache cache, Path unpackedWar) { + super(cache, true); + this.doc = unpackedWar.resolve("Documentation"); + } + + @Override + protected Path getResourcePath(String pathInfo) { + return doc.resolve(pathInfo); + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java index 3feb6bb6ae..4256a9f16b 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/raw/StaticModule.java @@ -120,6 +120,8 @@ public class StaticModule extends ServletModule { Paths p = getPaths(); if (p.warFs != null) { return new WarDocServlet(cache, p.warFs); + } else if (p.unpackedWar != null && !p.isDev()) { + return new DirectoryDocServlet(cache, p.unpackedWar); } else { return new HttpServlet() { private static final long serialVersionUID = 1L; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java index 0da66bfe26..63ec0754d6 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountServiceImpl.java @@ -55,7 +55,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements private final Provider currentUser; private final ProjectControl.Factory projectControlFactory; private final AgreementInfoFactory.Factory agreementInfoFactory; - private final ChangeQueryBuilder queryBuilder; + private final Provider queryBuilder; private final SetDiffPreferences setDiff; @Inject @@ -63,7 +63,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements final Provider identifiedUser, final ProjectControl.Factory projectControlFactory, final AgreementInfoFactory.Factory agreementInfoFactory, - final ChangeQueryBuilder queryBuilder, + final Provider queryBuilder, SetDiffPreferences setDiff) { super(schema, identifiedUser); this.currentUser = identifiedUser; @@ -147,7 +147,7 @@ class AccountServiceImpl extends BaseServiceImplementation implements if (filter != null) { try { - queryBuilder.parse(filter); + queryBuilder.get().parse(filter); } catch (QueryParseException badFilter) { throw new InvalidQueryException(badFilter.getMessage(), filter); } diff --git a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java index 22b1aee3ef..1334bcb05b 100644 --- a/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java +++ b/gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneChangeIndex.java @@ -230,7 +230,7 @@ public class LuceneChangeIndex implements ChangeIndex { CUSTOM_CHAR_MAPPING); queryBuilder = new QueryBuilder(analyzer); - BooleanQuery.setMaxClauseCount(cfg.getInt("index", "defaultMaxClauseCount", + BooleanQuery.setMaxClauseCount(cfg.getInt("index", "maxTerms", BooleanQuery.getMaxClauseCount())); GerritIndexWriterConfig openConfig = diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java index 5c4b9cdc30..a78c23cacb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/CreateEmail.java @@ -103,7 +103,8 @@ public class CreateEmail implements RestModifyView public Response apply(IdentifiedUser user, EmailInput input) throws AuthException, BadRequestException, ResourceConflictException, - ResourceNotFoundException, OrmException, EmailException { + ResourceNotFoundException, OrmException, EmailException, + MethodNotAllowedException { if (input.email != null && !email.equals(input.email)) { throw new BadRequestException("email address must match URL"); } @@ -126,7 +127,11 @@ public class CreateEmail implements RestModifyView } } else { try { - registerNewEmailFactory.create(email).send(); + RegisterNewEmailSender sender = registerNewEmailFactory.create(email); + if (!sender.isAllowed()) { + throw new MethodNotAllowedException("Not allowed to add email address " + email); + } + sender.send(); info.pendingConfirmation = true; } catch (EmailException | RuntimeException e) { log.error("Cannot send email verification message to " + email, e); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java index e830cfc7ad..b3ddae07c2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/Helper.java @@ -225,8 +225,6 @@ import javax.security.auth.login.LoginException; try { account = findAccount(schema, ctx, username, false); } catch (AccountException e) { - LdapRealm.log.warn("Account " + username + - " not found, assuming empty group membership"); return Collections.emptySet(); } } @@ -248,8 +246,6 @@ import javax.security.auth.login.LoginException; try { account = findAccount(schema, ctx, username, true); } catch (AccountException e) { - LdapRealm.log.warn("Account " + username + - " not found, assuming empty group membership"); return Collections.emptySet(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java index dd499ec76b..3f333af4c2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java @@ -17,12 +17,12 @@ package com.google.gerrit.server.change; import static com.google.gerrit.server.PatchLineCommentsUtil.PLC_ORDER; import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling; -import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.SendEmailExecutor; import com.google.gerrit.server.mail.CommentSender; import com.google.gerrit.server.notedb.ChangeNotes; @@ -32,7 +32,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; -import com.google.inject.OutOfScopeException; import com.google.inject.Provider; import com.google.inject.ProvisionException; import com.google.inject.assistedinject.Assisted; @@ -51,7 +50,7 @@ public class EmailReviewComments implements Runnable, RequestContext { NotifyHandling notify, ChangeNotes notes, PatchSet patchSet, - Account.Id authorId, + IdentifiedUser user, ChangeMessage message, List comments); } @@ -65,13 +64,13 @@ public class EmailReviewComments implements Runnable, RequestContext { private final NotifyHandling notify; private final ChangeNotes notes; private final PatchSet patchSet; - private final Account.Id authorId; + private final IdentifiedUser user; private final ChangeMessage message; private List comments; private ReviewDb db; @Inject - EmailReviewComments ( + EmailReviewComments( @SendEmailExecutor ExecutorService executor, PatchSetInfoFactory patchSetInfoFactory, CommentSender.Factory commentSenderFactory, @@ -80,7 +79,7 @@ public class EmailReviewComments implements Runnable, RequestContext { @Assisted NotifyHandling notify, @Assisted ChangeNotes notes, @Assisted PatchSet patchSet, - @Assisted Account.Id authorId, + @Assisted IdentifiedUser user, @Assisted ChangeMessage message, @Assisted List comments) { this.sendEmailsExecutor = executor; @@ -91,7 +90,7 @@ public class EmailReviewComments implements Runnable, RequestContext { this.notify = notify; this.notes = notes; this.patchSet = patchSet; - this.authorId = authorId; + this.user = user; this.message = message; this.comments = PLC_ORDER.sortedCopy(comments); } @@ -107,7 +106,7 @@ public class EmailReviewComments implements Runnable, RequestContext { CommentSender cm = commentSenderFactory.create(notes.getProjectName(), notes.getChangeId()); - cm.setFrom(authorId); + cm.setFrom(user.getAccountId()); cm.setPatchSet(patchSet, patchSetInfoFactory.get(notes.getProjectName(), patchSet)); cm.setChangeMessage(message); @@ -132,7 +131,7 @@ public class EmailReviewComments implements Runnable, RequestContext { @Override public CurrentUser getUser() { - throw new OutOfScopeException("No user on email thread"); + return user.getRealUser(); } @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index bc83237842..fda92d2eed 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -384,7 +384,7 @@ public class PostReview implements RestModifyView in.notify, notes, ps, - user.getAccountId(), + user, message, comments).sendAsync(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java index 5802c155c8..ffb63f626c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerSuggestionCache.java @@ -172,9 +172,9 @@ public class ReviewerSuggestionCache { doc.add(new TextField(NAME, a.getFullName(), Store.YES)); } if (a.getPreferredEmail() != null) { + doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES)); doc.add(new StringField(EMAIL, a.getPreferredEmail().toLowerCase(), Store.YES)); - doc.add(new TextField(EMAIL, a.getPreferredEmail(), Store.YES)); } AccountExternalIdAccess extIdAccess = db.get().accountExternalIds(); String username = AccountState.getUserName( diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 97a26b1687..695b6bb287 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -247,7 +247,7 @@ public class ChangeEditUtil { PatchSetInserter inserter = patchSetInserterFactory.create(ctl, psId, squashed); - StringBuilder message = new StringBuilder("Patch set ") + StringBuilder message = new StringBuilder("Patch Set ") .append(inserter.getPatchSetId().get()) .append(": "); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java index 2e993bd9f9..f90b72ca10 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/EmailMerge.java @@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.mail.MergedSender; import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.ThreadLocalRequestContext; @@ -48,6 +49,7 @@ public class EmailMerge implements Runnable, RequestContext { private final MergedSender.Factory mergedSenderFactory; private final SchemaFactory schemaFactory; private final ThreadLocalRequestContext requestContext; + private final IdentifiedUser.GenericFactory identifiedUserFactory; private final Project.NameKey project; private final Change.Id changeId; @@ -59,6 +61,7 @@ public class EmailMerge implements Runnable, RequestContext { MergedSender.Factory mergedSenderFactory, SchemaFactory schemaFactory, ThreadLocalRequestContext requestContext, + IdentifiedUser.GenericFactory identifiedUserFactory, @Assisted Project.NameKey project, @Assisted Change.Id changeId, @Assisted @Nullable Account.Id submitter) { @@ -66,6 +69,7 @@ public class EmailMerge implements Runnable, RequestContext { this.mergedSenderFactory = mergedSenderFactory; this.schemaFactory = schemaFactory; this.requestContext = requestContext; + this.identifiedUserFactory = identifiedUserFactory; this.project = project; this.changeId = changeId; this.submitter = submitter; @@ -102,6 +106,10 @@ public class EmailMerge implements Runnable, RequestContext { @Override public CurrentUser getUser() { + if (submitter != null) { + return identifiedUserFactory.create( + getReviewDbProvider(), submitter).getRealUser(); + } throw new OutOfScopeException("No user on email thread"); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 392e43df3d..af0ef9ebc9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -798,7 +798,7 @@ public class ReceiveCommits { } catch (RestApiException err) { reject(replace.inputCommand, "internal server error"); log.error(String.format( - "Cannot add patch set to %d of %s", + "Cannot add patch set to change %d in project %s", e.getKey().get(), project.getName()), err); } } else if (replace.inputCommand.getResult() == NOT_ATTEMPTED) { @@ -2169,6 +2169,9 @@ public class ReceiveCommits { try (RequestState state = requestState(caller)) { return insertPatchSet(state); } + } catch (OrmException | IOException e) { + log.error("Failed to insert patch set", e); + throw e; } finally { synchronizedIncrement(replaceProgress); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java index d861d32e19..bea5ade094 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java @@ -216,8 +216,10 @@ public class RebaseIfNecessary extends SubmitStrategy { static boolean dryRun(SubmitDryRun.Arguments args, CodeReviewCommit mergeTip, CodeReviewCommit toMerge) throws IntegrationException { + // Test for merge instead of cherry pick to avoid false negatives + // on commit chains. return !args.mergeUtil.hasMissingDependencies(args.mergeSorter, toMerge) - && args.mergeUtil.canCherryPick(args.mergeSorter, args.repo, mergeTip, - args.rw, toMerge); + && args.mergeUtil.canMerge(args.mergeSorter, args.repo, mergeTip, + toMerge); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java index d992c0ef40..2c2019b7a1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/IndexConfig.java @@ -29,7 +29,7 @@ import org.eclipse.jgit.lib.Config; */ @AutoValue public abstract class IndexConfig { - private static final int DEFAULT_MAX_TERMS = 500; + private static final int DEFAULT_MAX_TERMS = 1024; public static IndexConfig createDefault() { return create(0, 0, DEFAULT_MAX_TERMS); @@ -47,7 +47,7 @@ public abstract class IndexConfig { return new AutoValue_IndexConfig( checkLimit(maxLimit, "maxLimit", Integer.MAX_VALUE), checkLimit(maxPages, "maxPages", Integer.MAX_VALUE), - checkLimit(maxTerms, "maxTerms", Integer.MAX_VALUE)); + checkLimit(maxTerms, "maxTerms", DEFAULT_MAX_TERMS)); } private static int checkLimit(int limit, String name, int defaultValue) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java index 1b65673364..6d7b506a0e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/OutgoingEmail.java @@ -29,6 +29,7 @@ import com.google.gerrit.server.validators.ValidationException; import com.google.gwtorm.server.OrmException; import org.apache.commons.lang.StringUtils; +import org.apache.commons.validator.routines.EmailValidator; import org.apache.velocity.Template; import org.apache.velocity.VelocityContext; import org.apache.velocity.context.InternalContextAdapterImpl; @@ -336,7 +337,6 @@ public abstract class OutgoingEmail { protected boolean shouldSendMessage() { if (body.length() == 0) { // If we have no message body, don't send. - log.warn("Skipping delivery of email with no body"); return false; } @@ -344,7 +344,6 @@ public abstract class OutgoingEmail { // If we have nobody to send this message to, then all of our // selection filters previously for this type of message were // unable to match a destination. Don't bother sending it. - log.warn("Skipping delivery of email with no recipients"); return false; } @@ -394,21 +393,21 @@ public abstract class OutgoingEmail { /** Schedule delivery of this message to the given account. */ protected void add(final RecipientType rt, final Address addr) { if (addr != null && addr.email != null && addr.email.length() > 0) { - if (args.emailSender.canEmail(addr.email)) { - if (smtpRcptTo.add(addr)) { - switch (rt) { - case TO: - ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr); - break; - case CC: - ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr); - break; - case BCC: - break; - } - } - } else { + if (!EmailValidator.getInstance().isValid(addr.email)) { + log.warn("Not emailing " + addr.email + " (invalid email address)"); + } else if (!args.emailSender.canEmail(addr.email)) { log.warn("Not emailing " + addr.email + " (prohibited by allowrcpt)"); + } else if (smtpRcptTo.add(addr)) { + switch (rt) { + case TO: + ((EmailHeader.AddressList) headers.get(HDR_TO)).add(addr); + break; + case CC: + ((EmailHeader.AddressList) headers.get(HDR_CC)).add(addr); + break; + case BCC: + break; + } } } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java index c5d56b84e1..cfdeb8f390 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/RegisterNewEmailSender.java @@ -49,11 +49,6 @@ public class RegisterNewEmailSender extends OutgoingEmail { add(RecipientType.TO, new Address(addr)); } - @Override - protected boolean shouldSendMessage() { - return true; - } - @Override protected void format() throws EmailException { appendText(velocifyFile("RegisterNewEmail.vm")); @@ -70,4 +65,8 @@ public class RegisterNewEmailSender extends OutgoingEmail { } return emailToken; } + + public boolean isAllowed() { + return args.emailSender.canEmail(addr); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java index 8bd66334c3..aa572473f5 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/SmtpEmailSender.java @@ -249,16 +249,19 @@ public class SmtpEmailSender implements EmailSender { client.enableSSL(sslVerify); } + client.setConnectTimeout(connectTimeout); try { - client.setConnectTimeout(connectTimeout); client.connect(smtpHost, smtpPort); - if (!SMTPReply.isPositiveCompletion(client.getReplyCode())) { - throw new EmailException("SMTP server rejected connection"); + int replyCode = client.getReplyCode(); + String replyString = client.getReplyString(); + if (!SMTPReply.isPositiveCompletion(replyCode)) { + throw new EmailException( + String.format("SMTP server rejected connection: %d: %s", + replyCode, replyString)); } if (!client.login()) { - String e = client.getReplyString(); throw new EmailException( - "SMTP server rejected HELO/EHLO greeting: " + e); + "SMTP server rejected HELO/EHLO greeting: " + replyString); } if (smtpEncryption == Encryption.TLS) { @@ -266,16 +269,15 @@ public class SmtpEmailSender implements EmailSender { throw new EmailException("SMTP server does not support TLS"); } if (!client.login()) { - String e = client.getReplyString(); - throw new EmailException("SMTP server rejected login: " + e); + throw new EmailException("SMTP server rejected login: " + replyString); } } if (smtpUser != null && !client.auth(smtpUser, smtpPass)) { - String e = client.getReplyString(); - throw new EmailException("SMTP server rejected auth: " + e); + throw new EmailException("SMTP server rejected auth: " + replyString); } - } catch (IOException e) { + return client; + } catch (IOException | EmailException e) { if (client.isConnected()) { try { client.disconnect(); @@ -283,17 +285,10 @@ public class SmtpEmailSender implements EmailSender { //Ignored } } - throw new EmailException(e.getMessage(), e); - } catch (EmailException e) { - if (client.isConnected()) { - try { - client.disconnect(); - } catch (IOException e2) { - // Ignored - } + if (e instanceof EmailException) { + throw (EmailException) e; } - throw e; + throw new EmailException(e.getMessage(), e); } - return client; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java index c2b28cb14c..588bc6d71b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/Plugin.java @@ -14,8 +14,6 @@ package com.google.gerrit.server.plugins; -import static com.google.gerrit.common.FileUtil.lastModified; - import com.google.common.base.Strings; import com.google.common.collect.Lists; import com.google.gerrit.common.Nullable; @@ -171,6 +169,6 @@ public abstract class Plugin { protected abstract boolean canReload(); boolean isModified(Path jar) { - return snapshot.lastModified() != lastModified(jar); + return snapshot.isModified(jar.toFile()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java index 5df364fb0d..87eca8b274 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/plugins/PluginGuiceEnvironment.java @@ -564,7 +564,9 @@ public class PluginGuiceEnvironment { return false; } Class type = key.getTypeLiteral().getRawType(); - if (LifecycleListener.class.isAssignableFrom(type)) { + if (LifecycleListener.class.isAssignableFrom(type) + // This is needed for secondary index to work from plugin listeners + && !is("com.google.gerrit.server.index.IndexCollection", type)) { return false; } if (StartPluginListener.class.isAssignableFrom(type)) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java index cac62e24fd..cbd4cd745b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java @@ -293,6 +293,10 @@ public class OutputStreamQuery { eventFactory.addPatchSetFileNames(c.currentPatchSet, d.change(), d.currentPatchSet()); } + if (includeComments) { + eventFactory.addPatchSetComments(c.currentPatchSet, + d.publishedComments()); + } } } @@ -301,7 +305,7 @@ public class OutputStreamQuery { if (includePatchSets) { eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(), includeApprovals ? d.approvals().asMap() : null, - labelTypes); + includeFiles, d.notes(), labelTypes); for (PatchSetAttribute attribute : c.patchSets) { eventFactory.addPatchSetComments( attribute, d.publishedComments()); diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties index c4cca1b8b0..823455d1f3 100644 --- a/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/mime/mime-types.properties @@ -47,6 +47,7 @@ soy = text/x-soy st = text/x-stsrc stex = text/x-stex swift = text/x-swift +tcl = text/x-tcl v = text/x-verilog vert = x-shader/x-vertex vh = text/x-verilog