8556 Commits

Author SHA1 Message Date
David Pursehouse
42ace3c4f4 Format java files with google-java-format
google-java-format was not run after squashing string concatenations
in change I7348413ae.

Change-Id: I30afb262cca64e7638657d59ebb81938c22f0242
2017-02-17 13:21:04 +09:00
Dave Borowitz
34f51034c9 Merge changes from topic 'reviewdb-primary'
* changes:
  Support migrating PrimaryStorage from NoteDb -> ReviewDb
  Support read-only lease in NoteDb meta graph
  AbstractChangeUpdate: Clarify when notes can be null
  ChangeRebuilder: Add method to rebuild NoteDb -> ReviewDb
2017-02-17 02:08:33 +00:00
Dave Borowitz
6ff2aef6f8 Support migrating PrimaryStorage from NoteDb -> ReviewDb
Change-Id: I14fb7be482b0cfbbdbb6eefdde535f37941fd85b
2017-02-16 20:33:35 -05:00
Dave Borowitz
cfb8e840ae Support read-only lease in NoteDb meta graph
To support reverse migration from NoteDb -> ReviewDb, we need to do a
similar thing to what we did with NoteDbChangeState, except in this case
NoteDb is the source of truth, so we need to record the lease in NoteDb.

Let a determined caller clear a lease by setting the readOnlyUntil to
the special timestamp of 0. There is no assurance that the caller is the
same one that holds the lease, but this code is going to be used in
exactly one place.

This does pollute the meta graph with implementation details of the
migration, which we didn't really want to do. We can erase our tracks
later by rebuilding the change again once ReviewDb is primary again.

Change-Id: Ia019fb47e1c1edf719354d0da6bbfe22429f3eb1
2017-02-16 20:26:06 -05:00
David Pursehouse
1111684821 Squash redundant string concatenations introduced by google-java-format
Reformatting the code with google-java-format tool in change Id5f3c6de9
resulted in wrapped lines being unwrapped to the new 100 columns limit.

In cases where the wrapped line was wrapped on a string concatenation,
the resulting long line now includes redundant concatenations. See the
upstream issue [1] for an example of this.

Squash the redundant concatenations with:

  git ls-files | grep java$ | xargs sed -i '' 's/" + "//g'

(note: this also resulted in a couple of unwanted changes; those are
manually undone.)

[1] https://github.com/google/google-java-format/issues/122

Change-Id: I7348413ae460c8c7a0b0c72dab0a1ae7275a2ec3
2017-02-17 08:57:46 +09:00
Dave Borowitz
24d3106b00 AbstractChangeUpdate: Clarify when notes can be null
Change-Id: I3330e91f66d9fd60e1e6f3f424c76db28735036c
2017-02-16 17:24:04 -05:00
Dave Borowitz
ef3fb11948 ChangeRebuilder: Add method to rebuild NoteDb -> ReviewDb
Change-Id: Iad57de65f54b5ac176576748972fe404c92f6e85
2017-02-16 17:24:03 -05:00
Dave Borowitz
3ae681fd6e Merge "CreateNewChangeForAllNotInTarget: Fix working with change series" 2017-02-16 15:18:07 +00:00
Han-Wen Nienhuys
795c384e4c Wrap IOExceptions in OrmException rather than IllegalStateException.
This avoids transient I/O problems from tripping up exception
monitoring.

Change-Id: I3563af3395bbe98a779be9a9ada6a4275d8216b7
2017-02-16 14:55:11 +01:00
ekempin
076d6e4704 Merge "Fix logger format across gerrit" 2017-02-16 07:31:51 +00:00
David Pursehouse
d83af77d18 ValidatorTest: Remove unnecessary import
OutgoingEmailValidator is in the same package, so it does not need to
be explicitly imported.

Change-Id: Ie0fceef827cc3fd67e86f7f46ff6590acc1e1e19
2017-02-16 10:46:46 +09:00
Wyatt Allen
2f341a9d37 Merge "Better email wording when updating changes the user does not own" 2017-02-15 21:59:25 +00:00
Wyatt Allen
2f7c135416 Better email wording when updating changes the user does not own
When a user updates a change that they are not the owner of (for
example, when providing assistance with a new patch set or when updating
the commit message on the owner's behalf) the generated email can give
the impression that the user has created an entirely new change.

With this change, ReplacePatchSet emails check whether the user who
modifies a change differs from the owner of that change. In this case it
appends a note regarding the owner.

For example, the sentence in the replace patch set emails ...

  User One has uploaded patch set #123 to this change.

... becomes ...

  User One has uploaded patch set #123 to the change originally created
  by User Two.

Bug: Issue 4805
Change-Id: Ia9aefaaf6fd3e5f0485675f30e048ac54c94a1b9
2017-02-15 13:49:41 -08:00
Zhen Chen
22b69a079e Fix logger format across gerrit
Did a scan for Logger instances through the whole project, fix all
malformed Loggers.

Change-Id: Ie126ed263a7d7bbf8688ba02d58bcd6f32107cfc
2017-02-15 10:35:38 -08:00
ekempin
8bfd99be80 Merge "Insert two newline chars when generating change message" 2017-02-15 15:18:32 +00:00
Patrick Hiesel
1c226ac737 Insert two newline chars when generating change message
This adds a newline char when parsing change messages for inbound
emails and fixes a bug where notifications sent out for these messages
would not contain the change message.

Change-Id: Ifa720236c1ded0207e5f4cf499a0879fa4ccff21
2017-02-15 12:55:45 +00:00
David Pursehouse
341ad81d9c ValidatorTest: Move to correct package
The class under test, OutgoingEmailValidator, is in the package:

  com.google.gerrit.server.mail.send

Move the test class to the same package.

Change-Id: I2d071a692aaf5c2b2bc1f09dd68e64110a646554
2017-02-15 10:51:56 +00:00
Wyatt Allen
0aeceaa5be Revert "Add a User Preference to Receive only Plaintext Emails"
This reverts commit 39777363a63c603cfa68ca52d2f3c99eb04f2f46.

Reason for revert: the frontend part of this change doesn't work.

Change-Id: I53138e939a1b98578787457e48743fdc8abf658c
2017-02-15 00:49:11 +00:00
David Pursehouse
ab879094c0 Merge "Add a User Preference to Receive only Plaintext Emails" 2017-02-14 23:18:27 +00:00
Edwin Kempin
1cd8f603a5 CreateNewChangeForAllNotInTarget: Fix working with change series
If a change is pushed that depends on an open change and both changes
are targeted to the same destination branch, we don't want to recreate
the base change for the same branch if the
CreateNewChangeForAllNotInTarget project option is set to true. We also
don't want to do any validation of the base change as it is not touched
by the upload of a successor change.

At the moment trying to push a change that depends on an open change of
another user fails with "invalid committer" if the uploading user
doesn't have the Forge Committer access right. This is because
CreateNewChangeForAllNotInTarget wrongly attempts to create a new change
for the base change, and hence does validation for the commit of the
base change. The error message for the base change is:

invalid committer

ERROR:  In commit 683c42e4ba60ffa68c820de9064b6c36238600c4
ERROR:  committer email address other.user@example.com
ERROR:  does not match your user account.
ERROR:
ERROR:  The following addresses are currently registered:
ERROR:    user@example.com
ERROR:
ERROR:  To register an email address, please visit:
ERROR:  http://localhost:0/#/settings/contact

This is confusing to the user who only uploaded a successor change but
didn't touch the base change.

Bug: Issue 5478
Change-Id: I04e25b5eb09e0c9540652c93eb1408d2fab39f6b
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-14 17:12:46 +01:00
Dave Borowitz
491170364a Merge "Do not invoke SubmoduleOp when no direct push in ReceiveCommits" 2017-02-14 15:36:47 +00:00
Dave Borowitz
b5a4bbe888 Merge "Support to query groups by owner group name" 2017-02-14 15:30:58 +00:00
Patrick Hiesel
39777363a6 Add a User Preference to Receive only Plaintext Emails
This adds a user preference such that we only send plaintext emails to
this user. It also adds docs and tests.

For the user, this comes at the cost of not being in the same email
thread as the users who receive HTML emails. We decided to send out
plaintext emails on a one-by-one basis instead of making one thread (as
we do with HTML) as this would just create more confusion.

When admins enable inbound email for Gerrit, the aforementioned caveat
isn't a problem anymore as users would email Gerrit anyway instead of
using a notification email as messaging thread.

Change-Id: I3d0fc920c9d81e1a7b707ae5f3f4600a62b8a93b
2017-02-14 15:32:52 +01:00
Changcheng Xiao
81c4809ac3 Store the number of unresolved comments in change index
When changes are queried, we want to return the number of unresolved
comments with each change. This number should be pre-counted rather than
loading all the comments and counting each time.

We also add new search operators, which allow user to query by
"has:unresolved" and "unresolved:<RELATION><NUMBER>".

The 'unresolvedCommentCount' field of 'ChangeData' will be null if
'lazyLoad' is false.

Change-Id: I97c6aeb47db48f56f94c5d184dfa36c8d7868cc8
2017-02-13 11:31:49 +01:00
Dave Borowitz
051e68013b Merge "Make implicit reviewers feature actually work on GWT UI" 2017-02-11 11:56:42 +00:00
Dave Borowitz
39bc108a39 Make implicit reviewers feature actually work on GWT UI
This was presumably fixed in Ia5e6b9791 and in I8b735db4f. It turns
out, that labels map is not null in GWT UI, as it is the case in PG,
but also non empty, even in non-voting case: Code-Review: 0.

Fix the optimization check to account for zero votes.

Bug: Issue 4638
Change-Id: I6d9a2cc42ec51e6b1df13b96cf4bcdd082c87f60
2017-02-11 06:19:54 +00:00
David Pursehouse
b480c2ecfa HtmlParser: Remove extra space in statement
Found by running google-java-format v1.3

Change-Id: I1439d93423cd569deb49841cf24a1f8d454a5ec3
2017-02-11 03:45:15 +00:00
Dave Borowitz
f1e305f296 Merge changes Ic218eaf2,I1700a68c
* changes:
  ChangeJson: Do not scan all starred changes refs of the change
  ChangeData: Remove deprecated and unused starredByUser field and methods
2017-02-10 14:51:44 +00:00
ekempin
3fd75a41ab Merge "Skip empty lines in emails when using TextParser" 2017-02-10 14:30:16 +00:00
ekempin
aaea0d7575 Merge "Improve Quotation Line Sanitizer and add Tests" 2017-02-10 14:17:56 +00:00
Patrick Hiesel
74a9edbd1d Skip empty lines in emails when using TextParser
Change-Id: I76a5de4fa9ac93cf971586f377a5d6ad4426be93
2017-02-10 14:57:29 +01:00
Patrick Hiesel
f1161f0ecd Improve Quotation Line Sanitizer and add Tests
Change-Id: I8d65c0015e6862e01a38ab2d7a6c50f51edf853c
2017-02-10 14:19:23 +01:00
Edwin Kempin
df1b48976d ChangeJson: Do not scan all starred changes refs of the change
When change details are loaded, the stars of the calling users are
included into the ChangeInfo. The stars are stored in
refs/starred-changes/<sharded-changeid>/<user-id> refs in the All-Users
repository and must be loaded from there. At the moment we load and
parse all starred changes ref of the change (all refs that start with
'refs/starred-changes/<sharded-changeid>/') when change details are
requested. Since we need only the stars of the calling user it is better
to only load and parse the single starred changes ref of the calling
user.

Change-Id: Ic218eaf2ce0e2b4d94c5c37136a86c20ee658dbd
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-10 14:10:36 +01:00
Edwin Kempin
bbe7ba18c7 ChangeData: Remove deprecated and unused starredByUser field and methods
Change-Id: I1700a68c8d23fb4534b7ae4f822526baa0287581
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-10 13:29:29 +01:00
Patrick Hiesel
baaacbead9 Replace non-breaking spaces in HtmlParser
Previously, trim() would not catch non-breaking spaces and they would
get added to the comments. Later on, non-breaking spaces get converted
to regular spaces yielding empty comments and comments that are not
trimmed correctly.

Change-Id: Ie1cd4e37a3f946e16a51e9db5d36ef1e0ee3bf5c
2017-02-10 11:39:51 +01:00
ekempin
9204602888 Merge changes I886a3844,I4b7661c1
* changes:
  Add test for deleting emails of other users
  Support deletion of emails in arbitrary external ID schemes
2017-02-10 08:45:00 +00:00
Edwin Kempin
dad613ef3e Support to query groups by owner group name
Change-Id: If514ec35caafe6fb359f82ff43a9e8efc8f520f2
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-10 09:21:55 +01:00
David Pursehouse
144a07d079 Format code with google-java-formatter version 1.3
Upgrade to version 1.3 which is the version in which the Eclipse
plugin was included [1].

The only change in formatting caused by this version is removal of
spaces after <li> tags in javadoc comments [2].

[1] https://github.com/google/google-java-format/pull/118
[2] bbb742ea49

Change-Id: I91ad790b586c9ea7e9af1b61016ea4276b90576d
2017-02-10 08:24:29 +01:00
Zhen Chen
acb51551fd Do not invoke SubmoduleOp when no direct push in ReceiveCommits
Most of the time, user pushes to refs/for/... for creating a change, do
not bother to get into the update superproject process.

Change-Id: I57390e0d5698e5e14d2566d8802167940d14cf1c
2017-02-09 11:42:49 -08:00
Edwin Kempin
ace2f9183d Support deletion of emails in arbitrary external ID schemes
The Delete Email REST endpoint assumed that all emails would be stored
as external ID in the "mailto:" scheme, but also external IDs in other
schemes can contain emails. Change the Delete Email REST endpoint so
that it deletes all external IDs of the user that are connected with the
given email address.

Bug: Issue 5423
Change-Id: I4b7661c19a5402013ed63ce0446de9146490304c
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-09 17:06:05 +01:00
Dave Borowitz
088c422caa ReceiveCommitsAdvertiseRefsHook: Request reviewer field
This is necessary to check draft visibility. It should have been added
at the same time as I177109ed9c.

Change-Id: I04574080793255df18eff1323203195ca9d1d17e
2017-02-09 10:45:30 -05:00
Patrick Hiesel
1fce31c863 Send email notifications and fire Gerrit event if comments are posted via email
Change-Id: I4121b8d8b13f9a675fc227a81f6543501b3084f6
2017-02-09 09:09:48 +01:00
David Ostrovsky
cf426c7207 Merge "Preferences: Fix overwriting configured default with original default" 2017-02-09 07:32:37 +00:00
Edwin Kempin
b4559f3cbd Preferences: Fix overwriting configured default with original default
If a user didn't set a value for a preference then the hard-coded
default value is used, unless another default value is configured in the
refs/users/default branch in All-Users.

If a default value was configured in refs/users/default then overwriting
this configured default with the original hard-coded default didn't
work. Only overwriting it with any other value worked.

This didn't work and the effective user setting was 80:
* hard-coded default: 100
* refs/users/default:  80
* user setting:       100

This worked:
* hard-coded default: 100
* refs/users/default:  80
* user setting:       101 (and any value other than 100)

This is the reason why setting lineLength ("Columns") in the diff
preferences on gerrit-review to 100 is not working, as in the
refs/users/default branch we have configured a default lineLength of 80
and 100 is the hard-coded default value. Please note that setting
lineLength to 100 works transiently until the page is reloaded because
the client stores the updated diff preferences locally.

Change-Id: I5f63f9d62f639c755b39d753d8b0b30832c8c9f6
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-09 08:21:29 +01:00
Saša Živkov
6978762844 Merge changes from topic 'diff-summary-cache-2'
* changes:
  Change indexing: avoid computing current file paths too early
  Store insertions/deletions in DiffSummary cache
2017-02-08 21:04:38 +00:00
Dave Borowitz
db4927841e CommentTimestampAdapter: Truncate milliseconds
googlesource.com uses an older version of Gson that is unable to parse
millseconds out of timestamps using the default type adapter[1]. Since
the point of this adapter is to work with existing googlesource.com
code, change it to support that.

[1] Specifically, it doesn't contain
    bcd52a1fdc

Change-Id: I5c129a29fa097d48f4aff002f9c85704e039e68f
2017-02-08 14:56:15 +00:00
Saša Živkov
43b50f87fb Change indexing: avoid computing current file paths too early
The currentFilePaths used to be computed very early in the reindexing
code. However, we still needed to go through the diff_summary cache
for changed lines. The diff_summary cache contains also the
currentFilePaths so we can simply get these paths from the cache.

This avoids expensive diff computations in JGit.

Change-Id: I30781f6b918ed2ca0cf0137bbfb21d92e0ba13af
2017-02-08 15:55:19 +01:00
Dave Borowitz
6fd1b6f1f6 Store insertions/deletions in DiffSummary cache
The goal is to use this cache for everything that is needed to
reindex a change, to avoid computing the full PatchList for each
change. Having a completely standalone cache for insertions/deletions
is overkill, but since we already have this cache, it's not too much
work to store a few more bytes in it.

This change is an alternative to I5a0bab3e.

Change-Id: I52ef20916ddf03f3e8692f1f443d09b89a98b2f1
2017-02-08 15:55:19 +01:00
Dave Borowitz
bf4224bc2f Merge "Assign unused Future return value to a variable"
* submodules:
* Update plugins/reviewnotes from branch 'master'
  - Assign unused Future return value to a variable
    
    Error Prone plans to make this pattern into a compile error; this avoids
    future breakage.
    
    Change-Id: I6a023f22f472941854a6729f6450b9b70a020069
2017-02-08 13:58:39 +00:00
Edwin Kempin
e1f9dd1120 AccountApi: Add methods to get emails and delete email
Also add a test for email deletion and add asserts for the addEmail()
test.

Bug: Issue 5385
Change-Id: I3e192638fdaac58d0cdaa535430bb968d7d32abb
Signed-off-by: Edwin Kempin <ekempin@google.com>
2017-02-08 13:42:46 +01:00