New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 13 users

Issue metadata

Status: Released
Owner: ----
Closed: Jan 2016
Cc:



Sign in to add a comment
link

Issue 3768: [reviewers-plugin] NullPointerException in EqualsFilePredicate

Reported by jhsc...@infer.com, Jan 12 2016

Issue description

Affected Version:
2.11.2

What steps will reproduce the problem?
1. Install reviewers plugin on gerrit 2.11.2
1a. git clone https://gerrit.googlesource.com/plugins/reviewers
1b. git checkout -B stable-2.11 origin/stable-2.11
1c. mvn clean package
1d. place in plugins dir of gerrit install
2. Add default reviewer with a file filter through the Gerrit Web UI
2a. load https://GERRIT_INSTALL/#/x/reviewers/p/projectName
2b. set filter to "file:some/full/path/to/real/file.txt", reviewer to a real user
3. Push a commit which matches the filter

What is the expected output? What do you see instead?

Expected: reviewer is automatically added as a reviewer
Actual: reviewer not added, error in gerrit server logs ($PATH here is some/file/in/my/repo/f.txt).


[2016-01-11 17:01:28,671] ERROR com.googlesource.gerrit.plugins.reviewers.ChangeEventListener : Error in operator file:$PATH
com.google.gerrit.server.query.QueryParseException: Error in operator file:$PATH
	at com.google.gerrit.server.query.QueryBuilder.error(QueryBuilder.java:338)
	at com.google.gerrit.server.query.QueryBuilder$ReflectionFactory.create(QueryBuilder.java:376)
	at com.google.gerrit.server.query.QueryBuilder.operator(QueryBuilder.java:283)
	at com.google.gerrit.server.query.QueryBuilder.operator(QueryBuilder.java:268)
	at com.google.gerrit.server.query.QueryBuilder.toPredicate(QueryBuilder.java:223)
	at com.google.gerrit.server.query.QueryBuilder.parse(QueryBuilder.java:185)
	at com.googlesource.gerrit.plugins.reviewers.ChangeEventListener.filterMatch(ChangeEventListener.java:250)
	at com.googlesource.gerrit.plugins.reviewers.ChangeEventListener.findReviewerSections(ChangeEventListener.java:239)
	at com.googlesource.gerrit.plugins.reviewers.ChangeEventListener.findReviewers(ChangeEventListener.java:223)
	at com.googlesource.gerrit.plugins.reviewers.ChangeEventListener.onEvent(ChangeEventListener.java:160)
	at com.google.gerrit.common.ChangeHookRunner.fireEventForUnrestrictedListeners(ChangeHookRunner.java:706)
	at com.google.gerrit.common.ChangeHookRunner.fireEvent(ChangeHookRunner.java:718)
	at com.google.gerrit.common.ChangeHookRunner.doPatchsetCreatedHook(ChangeHookRunner.java:364)
	at com.google.gerrit.server.change.ChangeInserter.insert(ChangeInserter.java:275)
	at com.google.gerrit.server.git.ReceiveCommits$CreateRequest.insertChange(ReceiveCommits.java:1721)
	at com.google.gerrit.server.git.ReceiveCommits$CreateRequest.access$900(ReceiveCommits.java:1653)
	at com.google.gerrit.server.git.ReceiveCommits$CreateRequest$1.call(ReceiveCommits.java:1684)
	at com.google.gerrit.server.git.ReceiveCommits$CreateRequest$1.call(ReceiveCommits.java:1680)
	at com.google.gerrit.server.util.RequestScopePropagator$1.call(RequestScopePropagator.java:96)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	at com.google.common.util.concurrent.MoreExecutors$DirectExecutorService.execute(MoreExecutors.java:299)
	at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:132)
	at com.google.common.util.concurrent.AbstractListeningExecutorService.submit(AbstractListeningExecutorService.java:58)
	at com.google.gerrit.server.git.ReceiveCommits$CreateRequest.insertChange(ReceiveCommits.java:1679)
	at com.google.gerrit.server.git.ReceiveCommits.insertChangesAndPatchSets(ReceiveCommits.java:799)
	at com.google.gerrit.server.git.ReceiveCommits.processCommands(ReceiveCommits.java:588)
	at com.google.gerrit.server.git.AsyncReceiveCommits$Worker.run(AsyncReceiveCommits.java:89)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at com.google.gerrit.server.util.RequestScopePropagator$5.call(RequestScopePropagator.java:222)
	at com.google.gerrit.server.util.RequestScopePropagator$4.call(RequestScopePropagator.java:201)
	at com.google.gerrit.server.util.ThreadLocalRequestScopePropagator$1.call(ThreadLocalRequestScopePropagator.java:55)
	at com.google.gerrit.server.util.RequestScopePropagator$1.call(RequestScopePropagator.java:98)
	at com.google.gerrit.server.util.RequestScopePropagator$2.run(RequestScopePropagator.java:131)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:471)
	at java.util.concurrent.FutureTask.run(FutureTask.java:262)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:178)
	at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:292)
	at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:379)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1145)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:615)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.NullPointerException
	at com.google.gerrit.server.query.change.EqualsFilePredicate.create(EqualsFilePredicate.java:27)
	at com.google.gerrit.server.query.change.ChangeQueryBuilder.file(ChangeQueryBuilder.java:487)
	at sun.reflect.GeneratedMethodAccessor346.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:606)
	at com.google.gerrit.server.query.QueryBuilder$ReflectionFactory.create(QueryBuilder.java:367)
	... 39 more
Please provide any additional information below.
 

Comment 1 by jhsc...@infer.com, Jan 12 2016

Note that I was directed to this issue tracker from GitHub: https://github.com/davido/gerrit-reviewers-plugin/issues/2

Thanks in advance!

Comment 2 by david.os...@gmail.com, Jan 12 2016

Project Member
Caused by: java.lang.NullPointerException
	at com.google.gerrit.server.query.change.EqualsFilePredicate.create(EqualsFilePredicate.java:27)

  if (!args.indexes.getSearchIndex().getSchema().getFields().containsKey( // <== line 27
       ChangeField.FILE_PART.getName())) {
    return eqPath;
  }

Comment 3 by david.os...@gmail.com, Jan 14 2016

Project Member
Status: AwaitingInformation
NPE in this line is really strange. I just tested on master and it works as epected. This code wasn't changed sinc 2.11. Also note that the schema is induced from the active Lucene version, and this code is in Gerrit core.

    Version search = null;
    List<Version> write = Lists.newArrayListWithCapacity(2);
    for (Version v : versions.descendingMap().values()) {
      if (v.schema == null) {
        continue;
      }
      if (write.isEmpty() && onlineUpgrade) {
        write.add(v);
      }
      if (v.ready) {
        search = v;
        if (!write.contains(v)) {
          write.add(v);
        }
        break;
      }
    }
    if (search == null) {
      throw new ProvisionException(runReindex);
    }

    markNotReady(cfg, versions.values(), write);
    LuceneChangeIndex searchIndex = indexFactory.create(search.schema, null);

Can it be that Lucene index is not correctly working on your site,
and this issue is unrelated to the reviewers plugin?
Can you search for: "file:^build/README.md" in gerrit UI in search box?
Or just enter this URL:

http://host:port/#/q/file:%255Ebuild/README.md

Does this work for you?

Comment 4 by jhsc...@infer.com, Jan 14 2016

I searched file:^build/README.md in the search box and got no results (but no errors).

I searched for file:^pod/util.py and got results back (pod/util.py is a file that exists in my repository).

I don't know anything about gerrit internals, but is it possible that args or args.indexes could be null in EqualsFilePredicate (perhaps due to bugs in wiring or dependency injection?)

I'll try creating a brand new project in gerrit today and reproing there. If there are non-destructive "rebuild" steps I can take (like a Lucene reindex), I'm happy to try those as well.

Thanks for your help!

Comment 5 by jhsc...@infer.com, Jan 14 2016

I created a new project, committed/submitted a single file (foo.txt). I then added a review with query "file:foo.txt", edited foo.txt, commited/submitted, and saw the same exception/stack trace.

So this is possibly instance (e.g., my gerrit install) specific, but not project-specific.

Let me know if there's anything else it would be helpful for me to try.

Thanks!

Comment 6 by david.os...@gmail.com, Jan 15 2016

Project Member
Can you test on master, 2.12 and 2.11 by installing Gerrit
and the reviewers plugin from the scratch? I suspect something is
wrong on your site. You can find the right versions of reviewers
plugin and gerrit on Gerritforge CI service: [1].

* [1] https://gerrit-ci.gerritforge.com/view/Gerrit

Comment 7 by jhsc...@infer.com, Jan 15 2016

Yes, although it will probably take me a few days to get time to do it.

Also, I tried replacing my locally built version of reviewers plugin with one from gerritforge, as well as reindexing, but neither of those helped.

Thanks! I'll post back with the results of a from-scratch install/test.

Comment 8 by david.os...@gmail.com, Jan 15 2016

Project Member
Status: Accepted
I installed stable 2.11 and reviewers plugin and I'm nw getting exactly the same stack trace as your:

Caused by: java.lang.NullPointerException
        at com.google.gerrit.server.query.change.EqualsFilePredicate.create(EqualsFilePredicate.java:27)
        at com.google.gerrit.server.query.change.ChangeQueryBuilder.file(ChangeQueryBuilder.java:502)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:606)
        at com.google.gerrit.server.query.QueryBuilder$ReflectionFactory.create(QueryBuilder.java:367)
        ... 39 more

Investigating...

Comment 9 by david.os...@gmail.com, Jan 15 2016

Project Member

Comment 10 by jhsc...@infer.com, Jan 15 2016

I'm happy/eager to test out the new war once everything goes through. Thanks!

Comment 11 by david.os...@gmail.com, Jan 15 2016

Project Member
If someone wonders, why this breakage doesn't happen on stable-2.12 and master, this change fixed it: [1].

* [1] https://gerrit-review.googlesource.com/68132

Comment 12 by david.os...@gmail.com, Jan 15 2016

Project Member
So, I cherry-picked [1] and uploaded: [2].

* [2] https://gerrit-review.googlesource.com/73878

Comment 13 by david.os...@gmail.com, Jan 21 2016

Project Member
All good things come in threes ;-)

https://gerrit-review.googlesource.com/74130

Comment 14 by jhsc...@infer.com, Jan 21 2016

Look forward to testing out the fix thanks!

Comment 15 by david.os...@gmail.com, Jan 21 2016

Project Member
I'm building stable-2.11 branch atm with this fix applied, for you to test.

Comment 16 by jhsc...@infer.com, Jan 21 2016

awesome!  point me to a binary and I should be able to test tomorrow  (Thursday, Pacific).

Comment 17 by david.os...@gmail.com, Jan 21 2016

Project Member
Patched gerrit release.war with documentation and plugins:

  http://ostrovsky.org/gerrit/gerrit-v2.11.5-30-gf86bcb5.war

$ md5sum gerrit-v2.11.5-30-gf86bcb5.war
50e82409480d87650a3f7e3383d887bf  gerrit-v2.11.5-30-gf86bcb5.war

Comment 18 by jhsc...@infer.com, Jan 21 2016

Success! Reviewer was added and no errors in log.

Let me know when an official build is available from gerritforge?

Thanks for tracking this down!

Comment 19 by david.os...@gmail.com, Jan 21 2016

Project Member
Thank you for the re-test.

> Let me know when an official build is available from gerritforge?

First, this change has to be merged. Can you consider to comment on this change:

  https://gerrit-review.googlesource.com/74130

I think you can't vote verify+1 (missing ACL), but you could comment on it and mention, that you have tested this diff on your site.

Comment 20 by jhsc...@infer.com, Jan 21 2016

done!

Comment 21 by jhsc...@infer.com, Jan 25 2016

Any ETA on when this might merge? Thanks again!

Comment 22 by david.os...@gmail.com, Jan 25 2016

Project Member
Cc: david.pu...@sonymobile.com

Comment 23 by jhsc...@infer.com, Jan 27 2016

Noticed the commit merged, triggered a build on gerritforge and installed it. Working so far!

Comment 24 by david.pu...@sonymobile.com, Jan 27 2016

Labels: FixedIn-2.11.6
Status: Submitted

Comment 25 by david.pu...@gmail.com, Feb 27 2018

Status: Released (was: Submitted)

Sign in to add a comment