First customer will be the find-owners plugin to get
plugin config parameters in project.config and gerrit.config.
Change-Id: I6bbc3cbad93b31aa8dc36b711ad6d74acef05424
With this and previous commits, code in "CreateChange"
is reorganised for better readability. Each method is
relatively small and mainly serves for one
functionality.
Change-Id: I893e38ee726f245706a5bb266b7c291de40bfbf6
This check is required for the input and can be done before
lots of the above code being executed. Moving it to
rejected early.
Change-Id: I62ed7b18bfa441dc6d7addb06a105069932acd9d
This commit groups those permission checks to a dedicated
method. It also renames "rsrc" to "projectResource" to make
it more readable.
Change-Id: I7b9ed0f9aa3d4af5e3fb1a9d0b9de96455d605a8
The existing #clean method is merged into this new method. More
code for preprocessing will be merged into this method in the
followup methods.
Change-Id: I943708898b2e47c62912fad739fefe8dccdd36fd
filterTagsSeparately was an option to make refs computation faster. It
was used from parts in Gerrit that didn't need this option and wanted
a faster computation. Now that we have restricted tags to be reachable
from refs/heads/* we can remove that extra complexity.
I764e16d introduced this originally.
Change-Id: I6c4a01db77ff9dad93439788100e8a4b090d946a
With the current 'extension' search operator it's possible to find
a) changes that contain at least one file with the given extension,
e.g.: extension:txt
b) changes that contain no file with the given extension,
e.g.: -extension:txt
However sometimes you want to match changes which only contain files of
a given extension, e.g. changes that only touch txt files. This can now
be done with the new 'only_extensions' search operator, e.g.:
only_extensions:txt
It is also possible to specify multiple file extensions. E.g. matching
changes that only touch txt and jpg files can be done by:
only_extensions:jpg,txt
By reversing the 'only_extensions' search operator it is possible to
match changes that not only touch files with certain extensions, e.g.:
-only_extensions:jpg,txt
The order and the case in which the extensions are provided to the
'only_extensions' operator don't matter.
Also extensions can be specified with or without leading '.' (same as
for the 'extension' search operator).
Changes that contain files without file extension can be matched by
including an empty file extension into the file extension list, e.g.:
* changes that only include txt files and files without extension:
only_extensions:,txt
* changes that only contain files without extension:
only_extensions:,
only_extensions:""
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I3d2b978ed455f835d1dad2daa920be0b0ec2ae36
The races here are non-obvious. First, in general, HTTP audit event
dispatching looks synchronous in GitOverHttpServlet, but is actually
async since it happens after writing data over an HTTP connection.
There's no way to know whether the thread on the other end has read the
data and continued executing before #doFilter returns.
More specifically in the test environment, GitOverHttpServletIT performs
some Git-over-HTTP operations during setup, namely cloning the repo into
an InMemoryRepository. We don't know whether the audit events generated
by this operation are processed before or after the @Test method starts
executing. As a result, we can't use the cleaner alternative of having a
test set up its own AuditListener binding, because the DynamicSet
addition may or may not be visible when GitOverHttpServlet calls the
audit listener.
Fundamentally, we can't avoid waiting an arbitrarily long amount of time
for the audit events to appear, because the JVM provides no time-bounded
guarantee about when the HTTP thread will continue executing after the
test thread has finished reading the HTTP response. So ultimately, this
change boils down to increasing the timeout we wait for audit events to
appear. (Also note that in the pessimal JVM thread scheduling case, we
are also subject to timeouts in HTTP communication, so even if we
somehow got rid of timeouts for audit event processing it's not like we
would be free from timeouts.)
That said, this change also improves the code to be more reliable and
easy to reason about in several ways. First, use BlockingQueue over
wait/notify, which is a better abstraction for pumping messages between
threads.
Second, we now keep track in GitOverHttpServlet of how many requests
were started, and this AtomicLong increment operation happens before any
HTTP response data is written. This means that we know exactly how many
audit events to expect from the BlockingQueue, and callers don't have to
either specify the expected count in advance. This reduces the risk of
a caller miscounting and waiting for an event that will never arrive. It
also means that test methods can easily drain an arbitrary number of
events from the setup code with a single call at the top of the method.
The remainder of events are generated from entirely within the method.
Now that we more reliably dequeue all events, it's clear that we
actually do expect two HTTP requests in both the upload-pack and
receive-pack cases. The previous code expecting 1 event in the
upload-pack case was wrong and only passed due to flakiness.
It's possible that the timeouts may still need to be adjusted to work in
other environments (slow laptops, CI). In the worst case, we can use
BlockingQueue#take(), which has no timeout at all.
As a side effect, move FakeGroupAuditService to the acceptance package,
since it needs a new dependency that we don't really need to add to the
testing package. Like other classes in the acceptance package, it's used
during acceptance test setup, not any smaller tests.
Considered the following alternatives to introducing a new counter in
GitOverHttpServlet:
* Changing the GroupAuditService interface to explicitly record the
start of requests. Getting the interface right is nontrivial and
would require changes to multiple implementations, with no benefit
outside of tests.
* Hooking into existing HTTP request metrics, which doesn't currently
work due to the bug identified by Ic7b38a01.
Verified that this reduces flakiness to <0.5% on my workstation with:
$ bazel test --runs_per_test=200 --test_filter='GitOverHttpServletIT#.*AuditEventLog' //javatests/com/google/gerrit/acceptance/git:git
Change-Id: Idd57cc7f5e0b6504403b10bd3676e78d9f1cd792
All methods in classes derived from AbstractPushForReview are cloning
test project twice:
1. with the inProcessProtocol
2. with SSH or HTTP protocol
Add new Annotation: SkipProjectClone and annotate the base class with
it and avoid double clone of projects in that case, so that the project
only cloned with the right protocol.
Change-Id: I6f3590cf47975d23293b113aa4a998b5c619e9b1
This simplifies the implementation, and underscores the fact that
the only new behavior introduced for tests is recording events in a side
list.
Change-Id: Ie2e5e7c9b22373ad8421f0d4dbd844edac0a047a
SearchSuggestOracle was only used by the GWT UI and does no longer
exist. For PolyGerrit the list of search suggestions is defined in
gr-search-bar.js.
Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: I24c6d7608242ab0d94f403fb8436eed34b0699d6
Currently, it's possible for a user to create a change on a branch
they can't see. When this happens, the created change is not
accessible by the user (WAI) but this gives the user a way to probe
whether a branch exists or not.
This commit fixes this issue by adding a "READ" permisssion check for
the input branch and verifies that if the user doesn't have permission
to see the target branch, we always return 404 no matter the branch
exists or not.
BTW, the existing #createChangeOnInvisibleBranchFails test failed
to catch up this because the 404 it verifies is thrown by
the java API ChangesImpl#create when it tries to create a ChangeInfo
for the created change.
Change-Id: I5c8e3334e37b7215c86c08c5172a8e3b4a69d0c6
Both classes only contain one static method each, and are used only
in a few places.
Remove the one in c.g.g.common and move its method to the other one.
Change-Id: I8ae9affc75c8abe6d3341287b604f5d2fd752caf
* changes:
Move default AllProjects configs to a utility class for tests
AllProjectsCreator: allow to initialize with only project description
AllProjectsCreator: add an 'initDefaultAcls' boolean field
AllProjectsCreator: move ACL initialization into separate methods
Provide 'projectDescription' and 'booleanProjectConfig' in 'AllProjectsInput'
Define 'AllProjectsInput' for 'AllProjectsCreator'
Add a test for "AllProjectsCreator"
Computing mergeability of open changes becomes very expensive for hosts
that have fast-moving branches and a lot of open changes. This commit
scales down this problem by adding a config to remove the bit from
ChangeInfo.
We will consult the mailing list to see if we can remove this bit from
ChangeInfo and the change index completely or add a new MERGEABLE list
option.
Change-Id: I28e90e17abaf369d03ff9d3c0d491903ecf30bf9
Iteration over the project states in ListProjects#display() method is
the main logic to render the project list. A number of check is
performed whether or not a specific project should be included in the
result or not. After a number of checks, including git repository
checks, the loop is bailed out if skip or limit is set and reached.
Move the check for skip and limit to earlier in the loop to avoid the
resource and time consuming checks in vain. As the side effect of this
change ProjectInfo is stopped to be populated with the data, that would
be eventually discarded anyway.
Change-Id: I4982800143b617f0d6e9d2a704002b2368249b4e
The ListProject.display() method has deep nesting and very
complicated logic inside. Break it down into separate steps
to make it more readable and allow easier review of its
improvement and changes.
Change-Id: Ic98bd6d6d4ba16ecfbdfccda4a99f4a51b993292
* stable-2.16:
PG: Don't get gitweb weblink from ServerInfo
Set version to 2.15.9
ListProjects: Refactor to avoid excessive heap usage
ListProjectsIT: Add test for parent candidates option
Fix support for deleting branches (if you have can_delete)
Remove redundant release notes
FileApi: Add a method to set a file's "reviewed" flag
Fix support for deleting branches (if you have can_delete)
Change-Id: I2e256821671efd0949ab85efe28aebfc9c184b34
The for loop had a premature return after the first result, when the
intent of the surrounding code (from I87e58dda) was to OR together
predicates when there are multiple matches.
Change-Id: I9cc852439846bbbfbb3b05343ee73be8ca6cdf0f
This emphasizes to callers that there is no performance benefit to
creating an Account instance. It would be unnecessarily verbose at best
and counterproductive at worst if a caller had an Account.Id available
and went out of their way to convert this to an Account just for the
purposes of calling a different override of canSee.
Change-Id: I90151a9ab5e5d7465eb836bebf39f9e8e72033c1
The fields that are used for sorting must be added to the document. This
means the documents in the index must be all recomputed, hence we need
new index schema versions for the affected indexes.
The sorting that is done by Lucene for the account index must match the
sorting of accounts that is already done in QueryAccounts before
returning query results to clients. QueryAccounts sorts results by
fullname, preferred email and account ID. If Lucene would do a different
sorting limited queries may return wrong results. E.g. if there are 3
accounts, Foo (ID=1), Bar (ID=2) and Baz (ID=3), which are all matched
by a query for which the results are limited to 2, callers of
QueryAccounts expect Bar and Baz to be returned since QueryAccounts
promises sorting by fullname. If now Lucene would sort by account ID,
Lucene would find Foo and Bar, Baz would be skipped since it's over the
limit. These results are then handed over to QueryAccounts which does
sorting by name so that Bar and Foo are returned to the client. This
would be wrong since the caller expects Bar and Baz. It's a bit unclear
how this worked when Lucene was not doing proper sorting at all, maybe
it was just luck that the withLimit test succeeded before. As Lucene
does the sorting properly now, the sorting in QueryAccounts could be
removed, but since Lucene is not the only index implementation and other
index implementation may still have sorting issues we keep that sorting
in QueryAccounts for now.
Bug: Issue 10210
Change-Id: Ic59e4d330fe8c7198023ddbc2fa946cf5db80b63
Signed-off-by: Edwin Kempin <ekempin@google.com>
(cherry picked from commit 714f7d3c3f6540b6d52bc4a426476f692eae612c)
