We want to reduce QPS to Gerrit where easily possible. The framework for
this is already in place: On every Response, we can set caching.
We don't want caching to impact users by showing them stale data that is
in the critical path of their work. For admins, this is slightly more
acceptable.
To start with, we cache /server/version (30s) and /server/top-menus
(300s). These only change if Gerrit gets upgraded or plugins that
contribute top-menus reload.
Change-Id: I33ef5792bffba79bde5d7e7dda6b4291f2db9cc7
Allow to grant the ACL to arbitrary users using new permission to
control who is able to flip the Work In Progress bit in addition to
three different groups allowed to toggle Work In Progress state: change
owner, server administrators and project owners.
Feature: Issue 10385
Change-Id: I66f6078936c4fae20f3a12dbd3a5d9a244eb75cb
This commit updates some code introduced by I977a77286f
to improve its readability and also honor some java best
practices, e.g.:
* explicitly prohibit inheritance if a class isn't
intended for that (Effective Java Item 19's advice:
design and document for inheritance or else prohibit it).
* Write unit tests in three sections, aka "arrange",
"act", "assert".
Change-Id: I8939370f16c2465c7ba102afd3877a31babd0c89
Iad564dd47 was problematic in the error case because it returned
Predicate.not(Predicate.any()). The "any" predicate is not an index
predicate, which produces at least two problems:
* When used in a with another non-index predicate, it
might throw "no ChangeDataSource" from AndSource/OrSource.
* More dangerously, when combined with an index predicate, it is used as
a post-filtering predicate. A post-filtering predicate that matches no
results means it gets applied to every change in the index before
returning an empty set, which is bad.
Instead, reuse a handy existing index predicate that is designed to
return no results efficiently from the index:
ChangeStatusPredicate.NONE. Expose this from a more general location,
ChangeIndexPredicate#none().
Change-Id: Ic9a7e277070cff7768ec91c1972cf8e9f6deacb1
The Gerrit Plugin injector and the RestApiServlet currently have
different bugs. This commit papers over the most pressing ones and adds
tests so that plugins can bind child collections and offer modifications
directly on the collections (e.g. postOnCollection).
When binding rest views from plugins, the plugin name is incorrectly set
to "gerrit" which is what we use for core Gerrit views. This does not
apply to collections though. These are bound with the correct plugin
name "plugin-name".
In RestApiServlet, this leads to a situation where the collection thinks
it belongs to a different part of Gerrit. For operations that modify the
collection (post, delete) this means we have to use "gerrit" as the
component name when looking up child views. For root collections, we
already do this and this is why it works for existing plugin root
collections. For subcollections, however, we did not - which is what
this commit changes. Now the two usages in RestApiServlet are
consistent.
To be clear: They are now consistently wrong across the different parts
of Gerrit, but they are consistent which means they work.
As future work, we want to fix this long standing issue by binding the
correct plugin names from the beginning. This is non-trivial though
because the DynamicMapProvider has no context on the plugin that is
performing the binding.
This commit adds the same logic that we have for core-collection
delegation to plugin-collections in RestApiServlet.
Change-Id: I954105155c4a354b6c577d8cc246d00998e33f57
This commit makes it possible for plugins to declare
project permissions like what they are doing for
capabilities.
In the access section of a "project.config" file,
a plugin delcared permission is stored in a format
like "plugin-{pluginName}-{permission}" so that we
can tell if a config name could possibly be a plugin
permission with some confidence. This is helpful
when we load/save "ProjectConfig" where listing
plugin defined permissions is not trivial, e.g.
DynamicMap doesn't present when a test site is
initialized.
Note plugin declared capabilities have to keep the
old format "{plugin-name}-capability" before a data
migration is done.
In the child commit, we are going to extend
the "PermissionBackend" to accept project permissions
declared by plugins.
Change-Id: I977a77286f56214f90a5ff6a5b482e2701cf9916
When the conflicts operator fails, the result is not actionable, and the
UX is bad in the PolyGerrit UI, producing a modal error dialog. The
conflicts operator is already known to be subject to false positives and
false negatives; this just replaces some errors with silent false
negatives.
This code is in a bit of an odd situation where the actual stack trace
is useful[1], but it's extremely long and noisy to log every time. Log a
single line most of the time, and a stack trace every once in a while.
[1] It's how I found https://git.eclipse.org/r/137886
Change-Id: Iad564dd4708080e8dabc88e9ad1ad7521ec6b603
Instead of saying "a non-open change", use the actual status of the
change, i.e.:
cannot set abandoned change to private
cannot set merged change to private
Adjust the setMergedChangePrivate test accordingly, and also add a
new test to cover the abandoned change use case.
Change-Id: I896f20b68e82edf1341c898ab2c0ba680cde1d69
There seem to be an reoccurring mistake of using String.replaceAll
instead of the String.replace. The method names are misleading and one
may think that the replace method replaces only the first occurrence and
the replaceAll method replaces all occurrences.
Actually, both methods replace all occurrences. However, the replace
method interpretes the first parameter as the plain string while the
replaceAll method interpretes it as a regex. Using the replace method
when regex support is not needed improves both performance and
readability.
Change-Id: Iffed6aae4d7b541e4dc76e4b5bd20dcd8d5bad49
* stable-2.16:
ChangeCleanupConfig: Remove unnecessary usage of regex in URL replacement
AbandonIT: Add tests for changeCleanup.abandonMessage
UrlFormatter#getChangeViewUrl: Remove @Nullable and simplify default implementation
UrlFormatter#getSettingsUrl: Annotate section parameter as @Nullable
MergeUtil: Include project name in "Reviewed-On" URL
ChangeCleanupConfig: Inject UrlFormatter via DynamicItem
Adjust more classes to inject UrlFormatter via DynamicItem
Set version to 2.15.12-SNAPSHOT
Set version to 2.15.11
Allow LFS-over-SSH created auth pass through ContainerAuthFilter
Upgrade elasticsearch-rest-client to 6.6.1
ElasticContainer: Bump the test server version to 5.6.15
Change-Id: Ie90d450ddb16d165ed2d5ec91964d35b735214dd
RevisionNote is currently only used with lists. For the checks plugin we
want to use it to represent a map. Therefore, offer a getOnlyEntity
method to make sure we always just deal with a single entity.
Change-Id: I15b93e2517ce2dd3d8a6c421d5d8e7ac04c28220
There are no longer any callers in core that pass null project to
this method. Remove the @Nullable annotation and simplify the default
implementation.
Change-Id: I4359527e0167712a9d68efe08eb86392daa38f13
The method UrlFormatter#getChangeViewUrl has the @Nullable annotation
on the project argument, allowing the project name to be omitted from
the URL.
The URL used in the Reviewed-On footer, which is added to the detailed
commit message, is the only place that passes null for that parameter,
and as a result this is the only place in Gerrit where the change URL
does not include the project name.
Bug: Issue 10512
Change-Id: Ib6737af0a72adb7706caa20991780be71617dd24
Since change I375245647 the UrlFormatter is bound as a DynamicItem,
so it should also be injected as such.
For this class we can't simply change the constructor parameter since
the UrlFormatter is not stored as a member but is used to initialise
the abandon message, which may have a $URL placeholder which is used
to replace with the actual URL provided by the UrlFormatter.
Change it so that the abandon message is initialized in the construcor
as before, but the replacement of the $URL placeholder is only done at
the time the message is needed.
Bug: Issue 10500
Change-Id: If4299276ec94402e8d2b806ad179e185e307d1ab
Since change I375245647 the UrlFormatter is bound as a DynamicItem,
so it should also be injected as such.
Adjust most remaining classes to do that. The only one remaining is
the ChangeCleanupConfig which requires slightly more invasive changes
and will be done in a separate commit.
Bug: Issue 10500
Change-Id: Ie9eeef11bd5ce63168bb339e22ee3062482ddd25
* stable-2.15:
Set version to 2.15.12-SNAPSHOT
Set version to 2.15.11
Allow LFS-over-SSH created auth pass through ContainerAuthFilter
Upgrade elasticsearch-rest-client to 6.6.1
ElasticContainer: Bump the test server version to 5.6.15
Change-Id: I6a54f5b233cf9fa6053241b729cdd300f83dfdc9
For now, just tests a top-level binding under /plugins/X. Future tests
could check views bound in a RestApiModule under an existing collection,
but I wasn't able to get those working in this change.
These tests are in a separate class from the old
PluginsRestApiBindingsIT because they are really testing a different
type of endpoint: the former is about the remote admin endpoints, which
is different from actual plugin-provided endpoints. Rename
PluginsRestApiBindingsIT accordingly.
Change-Id: I71b82a14ded11cc77baf1313d82bc714f8602f67
* changes:
Warn the user about private changes on submit
Publish private changes when they get submitted through a Git push
Move private changes tests from ChangeIT to PrivateChangeIT
Inject ChangeMessageUtil into SetPrivateOp directly
Publish private changes on submission
Using reverse DNS lookup by default can cause unexpected performance
issues when writing the ref log, because the hostname of the user's
client is looked up on every ref log creation. For example this can
add a noticable delay (around 5 seconds) when adding a code review label,
due to the ref update of the notedb metadata refs in git.
Change the default of gerrit.enableReverseDnsLookup to false so that
DNS lookup is by default disabled.
Reported-by: Roman Karlstetter <Roman.Karlstetter@ifta.com>
Change-Id: I68af8e545fc54ea0c3819454d81bb4c598f65e93
Having a configuration option 'disable' with default value 'false'
results in a double-negative meaning which can be confusing.
Replace this option with enableReverseDnsLookup, with default 'true',
which effectively results in the same default behavior when no value
is configured.
Rename the DisableReverseDnsLookup and DisableReverseDnsLookupProvider
classes accordingly.
Change-Id: I36a8be9474f58a0d1457850bb5fae2fa7cc08ee8
* stable-2.16:
Acceptance: set log threshold level for tests
Re-introduce the cache-based listing by configuration
Change-Id: I6049c231690b98afd7a16fbb26f9a6004bc3a094
By default all acceptance tests start Gerrit in DEBUG level, which in
some cases may generate *a lot* of logs that may pollute the output
console and make troubleshooting very difficult.
Allow specific tests to customise the log level and reduce it to
a more acceptable level.
Sample use:
@LogThreshold(level = "INFO")
public class MyTest extends AbstractDaemonTest {
@Test
public void shouldJustWork() {
// test code
}
}
Change-Id: I2b0b8496de1a43de112ce5b5c427e3c8603d5d9e
They belong there right next to the JGit PushResultSubject, they were
just put in the wrong place initially.
Change-Id: Iaef009136888c813f1bd20b7988e4f59259e860e
* stable-2.16:
Update git submodules
Update git submodules
Update cmd-stream-events doc
PG: Set diff line lenght to 72 for commit message
Add support for 'base' in cherry pick dialog
Update git submodules
Update git submodules
CommitValidators: Inject UrlFormatter via DynamicItem
Allow omitting action in permission removal
Fix support for ldap groups that contains spaces
AccountIT: Extend assertKeyEquals to check that full key is included
Fix GPG public key export
Change-Id: Ia5396777b9922d090088fea4c010d2f1c6179534
