Merge "Update highlight.js to master branch"
diff --git a/Documentation/cmd-create-project.txt b/Documentation/cmd-create-project.txt
index 026d7b1..e48eea5 100644
--- a/Documentation/cmd-create-project.txt
+++ b/Documentation/cmd-create-project.txt
@@ -28,9 +28,8 @@
== DESCRIPTION
Creates a new bare Git repository under `gerrit.basePath`, using
the project name supplied. The newly created repository is empty
-(has no commits), but is registered in the Gerrit database so that
-the initial commit may be uploaded for review, or initial content
-can be pushed directly into a branch.
+(has no commits), and the initial content may either be uploaded for
+review, or pushed directly to a branch.
If replication is enabled, this command also connects to each of
the configured remote systems over SSH and uses command line git
diff --git a/Documentation/cmd-flush-caches.txt b/Documentation/cmd-flush-caches.txt
index 55d9083..5a84b9d 100644
--- a/Documentation/cmd-flush-caches.txt
+++ b/Documentation/cmd-flush-caches.txt
@@ -16,7 +16,7 @@
truth when it needs the information again.
Flushing a cache may be necessary if an administrator modifies
-database records directly in the database, rather than going through
+NoteDb metadata directly in a repository, rather than going through
the Gerrit web interface.
If no options are supplied, defaults to `--all`.
diff --git a/Documentation/cmd-index.txt b/Documentation/cmd-index.txt
index e999218..edb54b5 100644
--- a/Documentation/cmd-index.txt
+++ b/Documentation/cmd-index.txt
@@ -71,7 +71,7 @@
List projects visible to the caller.
link:cmd-query.html[gerrit query]::
- Query the change database.
+ Query the change search index.
'gerrit receive-pack'::
'Deprecated alias for `git receive-pack`.'
diff --git a/Documentation/cmd-ls-user-refs.txt b/Documentation/cmd-ls-user-refs.txt
index cba7d1b..0363f60 100644
--- a/Documentation/cmd-ls-user-refs.txt
+++ b/Documentation/cmd-ls-user-refs.txt
@@ -32,8 +32,8 @@
--user::
-u::
Required; User for which the visible refs should be listed. Gerrit
- will query the database to find matching users, so the
- full identity/name does not need to be specified.
+ will query the index to find matching users, so the full
+ identity/name does not need to be specified.
--only-refs-heads::
Only list the refs found under refs/heads/*
diff --git a/Documentation/cmd-query.txt b/Documentation/cmd-query.txt
index 79723c5..7eb24ea 100644
--- a/Documentation/cmd-query.txt
+++ b/Documentation/cmd-query.txt
@@ -1,7 +1,7 @@
= gerrit query
== NAME
-gerrit query - Query the change database
+gerrit query - Query the change search index
== SYNOPSIS
[verse]
@@ -24,7 +24,7 @@
== DESCRIPTION
-Queries the change database and returns results describing changes
+Queries the change search index and returns results describing changes
that match the input query. More recently updated changes appear
before older changes, which is the same order presented in the
web interface. For each matching change, the result contains data
diff --git a/Documentation/cmd-review.txt b/Documentation/cmd-review.txt
index 5417901..71385e2 100644
--- a/Documentation/cmd-review.txt
+++ b/Documentation/cmd-review.txt
@@ -27,7 +27,7 @@
== DESCRIPTION
Updates the current user's approval status of the specified patch
sets and/or submits them for merging, sending out email
-notifications and updating the database.
+notifications and updating code review metadata.
Patch sets may be specified in 'CHANGEID,PATCHSET' format, such as
'8242,2', or 'COMMIT' format.
diff --git a/Documentation/config-auto-site-initialization.txt b/Documentation/config-auto-site-initialization.txt
index 1be0af9..2253ed0 100644
--- a/Documentation/config-auto-site-initialization.txt
+++ b/Documentation/config-auto-site-initialization.txt
@@ -2,74 +2,41 @@
== Description
-Gerrit supports automatic site initialization on server startup
-when Gerrit runs in a servlet container. Both creation of a new site
-and upgrade of an existing site are supported. By default, all packaged
-plugins will be installed when Gerrit is deployed in a servlet container
-and the location of the Gerrit distribution can be determined at
-runtime. It is also possible to install only a subset of packaged
-plugins or not install any plugins.
+Gerrit supports automatic site initialization on server startup when Gerrit runs
+in a servlet container. Both creation of a new site and upgrade of an existing
+site are supported. By default, all packaged plugins will be installed when
+Gerrit is deployed in a servlet container and the location of the Gerrit
+distribution can be determined at runtime. It is also possible to install only a
+subset of packaged plugins or not install any plugins.
-This feature may be useful for such setups where Gerrit administrators
-don't have direct access to the database and the file system of the
-server where Gerrit should be deployed and, therefore, cannot perform
-the init from their local machine prior to deploying Gerrit on such a
-server. It may also make deployment and testing in a local servlet
-container faster to set up as the init step could be skipped.
+This feature may be useful for such setups where Gerrit administrators don't
+have direct access to the file system of the server where Gerrit should be
+deployed and, therefore, cannot perform the init from their local machine prior
+to deploying Gerrit on such a server. It may also make deployment and testing in
+a local servlet container faster to set up as the init step could be skipped.
== Gerrit Configuration
-The site initialization will be performed only if the `gerrit.init`
-system property exists. The value of the property is not used; only the
-existence of the property matters.
+In order to perform site initialization, define `gerrit.site_path` with the path
+to your site. If the site already exists, this is the only required property.
+If your site does not yet exist, set the `gerrit.init` system property to
+automatically initialize the site.
-If the `gerrit.site_path` system property is defined then the init is
-run for that site. The database connectivity, in that case, is defined
-in the `etc/gerrit.config`.
+During initialization, if the `gerrit.install_plugins` property is not defined,
+then all packaged plugins will be installed. If it is defined, then it is parsed
+as a comma-separated list of plugin names to install. If the value is an empty
+string then no plugins will be installed.
-`gerrit.site_path` system property must be defined to run the init for
-that site.
+=== Example
-[WARNING]
-Defining the `jdbc/ReviewDb` JNDI property for an H2 database under the
-path defined by `gerrit.site_path` will cause an incomplete auto
-initialization and Gerrit will fail to start.
-
-Opening a connection to such a database will create a subfolder under the
-site path folder (in order to create the H2 database) and Gerrit will
-no longer consider that site path to be new and, because of that,
-skip some required initialization steps (for example, Lucene index
-creation). In order to auto initialize Gerrit with an embedded H2
-database use the `gerrit.site_path` to define the location of the review
-site and don't define a JNDI resource with a URL under that path.
-
-If the `gerrit.install_plugins` property is not defined then all packaged
-plugins will be installed. If it is defined then it is parsed as a
-comma-separated list of plugin names to install. If the value is an
-empty string then no plugin will be installed.
-
-=== Example 1
-
-Prepare Tomcat so that a site is initialized at a given path using
-the H2 database (if the site doesn't exist yet) or using whatever
-database is defined in `etc/gerrit.config` of that site:
+Prepare Tomcat so that a site is initialized at a given path (if the site
+doesn't exist yet), installing all packaged plugins.
----
$ export CATALINA_OPTS='-Dgerrit.init -Dgerrit.site_path=/path/to/site'
$ catalina.sh start
----
-=== Example 2
-
-Assuming the database schema doesn't exist in the database defined
-via the `jdbc/ReviewDb` JNDI property, initialize a new site using that
-database and a given path:
-
-----
- $ export CATALINA_OPTS='-Dgerrit.init -Dgerrit.init_path=/path/to/site'
- $ catalina.sh start
-----
-
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 94ee7ab..e92b0bd 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -46,8 +46,8 @@
link:pgm-MigrateAccountPatchReviewDb.html[MigrateAccountPatchReviewDb] program.
Migration cannot be done while the server is running.
+
-Also note that the db_name has to be a new db and not reusing gerrit's own review database,
-otherwise gerrit's init will remove the table.
+Also note that the db_name has to be a new db and not reusing an old ReviewDb
+database from a former 2.x site, otherwise gerrit's init will remove the table.
----
[accountPatchReviewDb]
@@ -805,11 +805,10 @@
+
Cache entries contain important details of an active user, including
their display name, preferences, and known email addresses. Entry
-information is obtained from the `accounts` database table.
+information is obtained from NoteDb data in the `All-Users` repo.
+
-If direct updates are made to any of these database tables, this
-cache should be flushed.
+If direct updates are made to `All-Users`, this cache should be flushed.
cache `"adv_bases"`::
+
@@ -819,23 +818,18 @@
requires two HTTP requests, and this cache tries to carry state from
the first request into the second to ensure it can complete.
-cache `"change_refs"`::
-+
-Cache entries are used to compute change ref visibility efficiently. Entries
-contain the minimal required information for this task.
+cache `"changes"`::
+
The size of `memoryLimit` determines the number of projects for which
all changes will be cached. If the cache is set to 1024, this means all
-at maximum 1024 changes can be held in the cache.
+changes for up to 1024 projects can be held in the cache.
+
-Default value is 10.000.
+Default value is 0 (disabled). It is disabled by default due to the fact
+that change updates are not communicated between Gerrit servers. Hence
+this cache should be disabled in an multi-master/multi-slave setup.
+
-If the size is set to 0, the cache is disabled. The change index will not
-be used at all. Instead, the change notes cache is used directly.
-+
-A good size for this cache is twice the number of changes that the Gerrit
-instance has to allow it to hold all current changes and account for
-growth.
+The cache should be flushed whenever the database changes table is modified
+outside of Gerrit.
cache `"diff"`::
+
@@ -2623,8 +2617,7 @@
+
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. Should this limit
-be reached, database is used instead of index as applicable.
+before attempting to send them to the secondary index.
+
When the index type is `LUCENE`, also sets the maximum number of clauses
permitted per BooleanQuery. This is so that all enforced query limits
@@ -3318,9 +3311,9 @@
[[note-db]]
=== Section noteDb
-NoteDb is the next generation of Gerrit storage backend, currently powering
-`googlesource.com`. For more information, including how to migrate your data,
-see the link:note-db.html[documentation].
+NoteDb is the Git-based database storage backend for Gerrit. For more
+information, including how to migrate data from an older Gerrit version, see the
+link:note-db.html[documentation].
[[notedb.accounts.sequenceBatchSize]]notedb.accounts.sequenceBatchSize::
+
diff --git a/Documentation/config-robot-comments.txt b/Documentation/config-robot-comments.txt
index cf5de10..0077697 100644
--- a/Documentation/config-robot-comments.txt
+++ b/Documentation/config-robot-comments.txt
@@ -36,7 +36,6 @@
== Limitations
-* Robot comments are only supported with NoteDb, but not with ReviewDb.
* Robot comments are not displayed in the web UI yet.
* There is no support for draft robot comments, but robot comments are
always published and visible to everyone who can see the change.
diff --git a/Documentation/dev-contributing.txt b/Documentation/dev-contributing.txt
index 6dce1e6..3b1c501 100644
--- a/Documentation/dev-contributing.txt
+++ b/Documentation/dev-contributing.txt
@@ -250,7 +250,6 @@
Here are some design level objectives that you should keep in mind
when coding:
- * ORM entity objects should match exactly one row in the database.
* Most client pages should perform only one RPC to load so as to
keep latencies down. Exceptions would apply to RPCs which need
to load large data sets if splitting them out will help the
@@ -269,10 +268,11 @@
* Don't leave repository objects (git or schema) open. A .close()
after every open should be placed in a finally{} block.
* Don't leave UI components, which can cause new actions to occur,
- enabled during RPCs which update the DB. This is to prevent
- people from submitting actions more than once when operating
- on slow links. If the action buttons are disabled, they cannot
- be resubmitted and the user can see that Gerrit is still busy.
+ enabled during RPCs which update Git repositories, including NoteDb.
+ This is to prevent people from submitting actions more than once
+ when operating on slow links. If the action buttons are disabled,
+ they cannot be resubmitted and the user can see that Gerrit is still
+ busy.
* ...and so is Guava (previously known as Google Collections).
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index e84effd..f2bd273e 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -325,10 +325,10 @@
Plugins' InitSteps are executed during the "Gerrit Plugin init" phase, after
the extraction of the plugins embedded in the distribution .war file into
-`$GERRIT_SITE/plugins` and before the DB Schema initialization or upgrade.
+`$GERRIT_SITE/plugins` and before the site initialization or upgrade.
-A plugin's InitStep cannot refer to Gerrit's DB Schema or any other Gerrit
-runtime objects injected at startup.
+A plugin's InitStep cannot refer to any Gerrit runtime objects injected at
+startup.
[source,java]
----
@@ -1573,9 +1573,8 @@
// schedule a build
[...]
// update change
- ReviewDb db = dbProvider.get();
try (BatchUpdate bu = batchUpdateFactory.create(
- db, project.getNameKey(), user, TimeUtil.nowTs())) {
+ project.getNameKey(), user, TimeUtil.nowTs())) {
bu.addOp(change.getId(), new BatchUpdate.Op() {
@Override
public boolean updateChange(ChangeContext ctx) {
diff --git a/Documentation/install.txt b/Documentation/install.txt
index be55417..0885da1 100644
--- a/Documentation/install.txt
+++ b/Documentation/install.txt
@@ -61,8 +61,7 @@
Gerrit stores configuration files, the server's SSH keys, and the
managed Git repositories under a local directory, typically referred
-to as `'$site_path'`. If the embedded H2 database is being used,
-its data files will also be stored under this directory.
+to as `'$site_path'`.
You also have to decide where to store your server side git repositories. This
can either be a relative path under `'$site_path'` or an absolute path
@@ -87,11 +86,10 @@
then give ownership of that location to the `'gerrit'` user.
If run from an interactive terminal, the init command will prompt through a
-series of configuration questions, including gathering information
-about the database created above. If the terminal is not interactive,
-running the init command will choose some reasonable default selections,
-and will use the embedded H2 database. Once the init phase is complete,
-you can review your settings in the file `'$site_path/etc/gerrit.config'`.
+series of configuration questions. If the terminal is not interactive,
+running the init command will choose some reasonable default selections.
+Once the init phase is complete, you can review your settings in the file
+`'$site_path/etc/gerrit.config'`.
When running the init command, additional JARs might be downloaded to
support optional selected functionality. If a download fails a URL will
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index ced4609..6864c68 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -106,10 +106,6 @@
* `sshd/sessions/created`: Rate of new SSH sessions.
* `sshd/sessions/authentication_failures`: Rate of SSH authentication failures.
-=== SQL connections
-
-* `sql/connection_pool/connections`: SQL database connections.
-
=== Topics
* `topic/cross_project_submit`: number of cross-project topic submissions.
diff --git a/Documentation/pgm-LocalUsernamesToLowerCase.txt b/Documentation/pgm-LocalUsernamesToLowerCase.txt
index 4b50961..53081a1 100644
--- a/Documentation/pgm-LocalUsernamesToLowerCase.txt
+++ b/Documentation/pgm-LocalUsernamesToLowerCase.txt
@@ -47,7 +47,7 @@
== CONTEXT
This command can only be run on a server which has direct
-connectivity to the metadata database.
+connectivity to the managed Git repositories.
== EXAMPLES
To convert the local username of every account to lower case:
diff --git a/Documentation/pgm-daemon.txt b/Documentation/pgm-daemon.txt
index 0b1a3e5..ad07cfa 100644
--- a/Documentation/pgm-daemon.txt
+++ b/Documentation/pgm-daemon.txt
@@ -19,14 +19,8 @@
== DESCRIPTION
Runs the Gerrit network daemon on the local system, configured as
-per the local copy of link:config-gerrit.html[gerrit.config].
-
-The path to gerrit.config is read from the metadata database,
-which requires that all slaves (and master) reading from the same
-database must place gerrit.config at the same location on the local
-filesystem. However, any option within gerrit.config, including
-link:config-gerrit.html#gerrit.basePath[gerrit.basePath] may be set
-to different values.
+per the local copy of link:config-gerrit.html[gerrit.config] located under
+`<SITE_PATH>/etc`.
== OPTIONS
diff --git a/Documentation/pgm-init.txt b/Documentation/pgm-init.txt
index 9a16cdf..f6c3c85 100644
--- a/Documentation/pgm-init.txt
+++ b/Documentation/pgm-init.txt
@@ -28,7 +28,7 @@
into a newly created `$site_path`.
If run in an existing `$site_path`, init upgrades existing resources
-(e.g. DB schema, plugins) as necessary.
+(e.g. NoteDb schema, plugins) as necessary.
== OPTIONS
-b::
@@ -100,8 +100,7 @@
folder.
== CONTEXT
-This command can only be run on a server which has direct
-connectivity to the metadata database, and local access to the
+This command can only be run on a server which has direct local access to the
managed Git repositories.
GERRIT
diff --git a/Documentation/pgm-rulec.txt b/Documentation/pgm-rulec.txt
index 1b50812..2a987205 100644
--- a/Documentation/pgm-rulec.txt
+++ b/Documentation/pgm-rulec.txt
@@ -33,8 +33,7 @@
Compile rules for the specified project.
== CONTEXT
-This command can only be run on a server which has direct
-connectivity to the metadata database, and local access to the
+This command can only be run on a server which has local access to the
managed Git repositories.
Caching needs to be enabled. See
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 7c904f5..abd2531 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -289,6 +289,14 @@
Regular expression matching can be enabled by starting the string
with `^`. In this mode `file:` is an alias of `path:` (see above).
+[[extension]]
+extension:'EXT', ext:'EXT'::
++
+Matches any change touching a file with extension 'EXT', case-insensitive. The
+extension is defined as the portion of the filename following the final `.`.
+Files with no `.` in their name have no extension and cannot be matched with
+this operator; use `file:` instead.
+
[[star]]
star:'LABEL'::
+
diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt
index 751e886..56602e2 100644
--- a/Documentation/user-upload.txt
+++ b/Documentation/user-upload.txt
@@ -732,7 +732,7 @@
Gerrit to provide magical refs, such as `+refs/for/*+` for new
change submission and `+refs/changes/*+` for change replacement.
When a push request is received to create a ref in one of these
-namespaces Gerrit performs its own logic to update the database,
+namespaces Gerrit performs its own logic to update the review metadata,
and then lies to the client about the result of the operation.
A successful result causes the client to believe that Gerrit has
created the ref, but in reality Gerrit hasn't created the ref at all.
diff --git a/contrib/mitm-ui/README.md b/contrib/mitm-ui/README.md
index c8df490..ad23140 100644
--- a/contrib/mitm-ui/README.md
+++ b/contrib/mitm-ui/README.md
@@ -36,6 +36,17 @@
2. Open any *.googlesource.com domain in proxied window
3. plugin.html and more.js are served
+### Force or replace default site theme for *.googlesource.com
+
+1. Create a new proxied browser window and start mitmproxy via Docker:
+ ```
+ ~/mitm-gerrit/mitm-theme.sh ./path/to/theme.html
+ ```
+2. Open any *.googlesource.com domain in proxied window
+3. Default site themes are enabled.
+4. Local `theme.html` content replaces `/static/gerrit-theme.html`
+5. `/static/*` URLs are served from local theme directory, i.e. `./path/to/`
+
### Serve uncompiled PolyGerrit
1. Create a new proxied browser window and start mitmproxy via Docker:
diff --git a/contrib/mitm-ui/mitm-theme.sh b/contrib/mitm-ui/mitm-theme.sh
new file mode 100755
index 0000000..9290235
--- /dev/null
+++ b/contrib/mitm-ui/mitm-theme.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+if [[ -z "$1" ]]; then
+ echo This script forces or replaces default site theme on *.googlesource.com
+ echo Provide path to the theme.html as a parameter.
+ exit 1
+fi
+
+realpath() {
+ OURPWD=$PWD
+ cd "$(dirname "$1")"
+ LINK=$(basename "$1")
+ while [ -L "$LINK" ]; do
+ LINK=$(readlink "$LINK")
+ cd "$(dirname "$LINK")"
+ LINK="$(basename "$1")"
+ done
+ REAL_DIR=`pwd -P`
+ RESULT=$REAL_DIR/$LINK
+ cd "$OURPWD"
+ echo "$RESULT"
+}
+
+theme=$(realpath "$1")
+theme_dir=$(dirname "${theme}")
+
+mitm_dir="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
+
+"${mitm_dir}"/dev-chrome.sh &
+
+"${mitm_dir}"/mitm-docker.sh -v "${theme_dir}":"${theme_dir}" "serve-app-dev.py --strip_assets --theme \"${theme}\""
diff --git a/contrib/mitm-ui/serve-app-dev.py b/contrib/mitm-ui/serve-app-dev.py
index bd054e5..18e9de1 100644
--- a/contrib/mitm-ui/serve-app-dev.py
+++ b/contrib/mitm-ui/serve-app-dev.py
@@ -32,9 +32,10 @@
import argparse
import os.path
import json
+import mimetypes
class Server:
- def __init__(self, devpath, plugins, pluginroot, assets, strip_assets):
+ def __init__(self, devpath, plugins, pluginroot, assets, strip_assets, theme):
if devpath:
print("Serving app from " + devpath)
if pluginroot:
@@ -53,6 +54,7 @@
self.devpath = devpath
self.pluginroot = pluginroot
self.strip_assets = strip_assets
+ self.theme = theme
def readfile(self, path):
with open(path, 'rb') as contentfile:
@@ -92,6 +94,8 @@
localfile = ""
if flow.request.path == "/config/server/info":
config = json.loads(flow.response.content[5:].decode('utf8'))
+ if server.theme:
+ config['default_theme'] = '/static/gerrit-theme.html'
for filename, path in server.plugins.items():
pluginname = filename.split(".")[0]
payload = config["plugin"]["js_resource_paths" if filename.endswith(".js") else "html_resource_paths"]
@@ -115,13 +119,23 @@
localfile = server.pluginroot + pluginfile
elif os.path.isfile(server.pluginroot + pluginurl):
localfile = server.pluginroot + pluginurl
+
+ if server.theme:
+ if flow.request.path.endswith('/gerrit-theme.html'):
+ localfile = server.theme
+ else:
+ match = re.match("^/static(/[\w\.]+)$", flow.request.path)
+ if match is not None:
+ localfile = os.path.dirname(server.theme) + match.group(1)
+
if localfile and os.path.isfile(localfile):
if pluginmatch is not None:
print("Serving " + flow.request.path + " from " + localfile)
flow.response.content = server.readfile(localfile)
flow.response.status_code = 200
- if localfile.endswith('.js'):
- flow.response.headers['Content-type'] = 'text/javascript'
+ localtype = mimetypes.guess_type(localfile)
+ if localtype and localtype[0]:
+ flow.response.headers['Content-type'] = localtype[0]
def expandpath(path):
return os.path.realpath(os.path.expanduser(path))
@@ -132,8 +146,11 @@
parser.add_argument("--plugin_root", type=str, default="", help="Path containing individual plugin files to replace")
parser.add_argument("--assets", type=str, default="", help="Path containing assets file to import.")
parser.add_argument("--strip_assets", action="store_true", help="Strip plugin bundles from the response.")
+parser.add_argument("--theme", type=str, help="Path to the default site theme to be used.")
args = parser.parse_args()
server = Server(expandpath(args.app) + '/',
- args.plugins, expandpath(args.plugin_root) + '/',
+ args.plugins,
+ expandpath(args.plugin_root) + '/',
args.assets and expandpath(args.assets),
- args.strip_assets)
+ args.strip_assets,
+ expandpath(args.theme))
diff --git a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 8d05ea1..3c8f170 100644
--- a/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -14,7 +14,6 @@
package com.google.gerrit.acceptance;
-import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assert_;
import static com.google.common.truth.Truth8.assertThat;
@@ -799,14 +798,13 @@
return accountState.get();
}
- protected Context disableDb() {
+ protected AutoCloseable disableNoteDb() {
changeNotesArgs.failOnLoadForTest.set(true);
- return atrScope.disableDb();
- }
-
- protected void enableDb(Context preDisableContext) {
- changeNotesArgs.failOnLoadForTest.set(false);
- atrScope.set(preDisableContext);
+ Context oldContext = atrScope.disableNoteDb();
+ return () -> {
+ changeNotesArgs.failOnLoadForTest.set(false);
+ atrScope.set(oldContext);
+ };
}
protected void disableChangeIndexWrites() {
@@ -827,55 +825,49 @@
protected AutoCloseable disableChangeIndex() {
disableChangeIndexWrites();
- ChangeIndex searchIndex = changeIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledChangeIndex)) {
- changeIndexes.setSearchIndex(new DisabledChangeIndex(searchIndex), false);
+ ChangeIndex maybeDisabledSearchIndex = changeIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledChangeIndex)) {
+ changeIndexes.setSearchIndex(new DisabledChangeIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() throws Exception {
- enableChangeIndexWrites();
- ChangeIndex searchIndex = changeIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledChangeIndex) {
- changeIndexes.setSearchIndex(((DisabledChangeIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ enableChangeIndexWrites();
+ ChangeIndex maybeEnabledSearchIndex = changeIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledChangeIndex) {
+ changeIndexes.setSearchIndex(
+ ((DisabledChangeIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
protected AutoCloseable disableAccountIndex() {
- AccountIndex searchIndex = accountIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledAccountIndex)) {
- accountIndexes.setSearchIndex(new DisabledAccountIndex(searchIndex), false);
+ AccountIndex maybeDisabledSearchIndex = accountIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledAccountIndex)) {
+ accountIndexes.setSearchIndex(new DisabledAccountIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() {
- AccountIndex searchIndex = accountIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledAccountIndex) {
- accountIndexes.setSearchIndex(((DisabledAccountIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ AccountIndex maybeEnabledSearchIndex = accountIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledAccountIndex) {
+ accountIndexes.setSearchIndex(
+ ((DisabledAccountIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
protected AutoCloseable disableProjectIndex() {
disableProjectIndexWrites();
- ProjectIndex searchIndex = projectIndexes.getSearchIndex();
- if (!(searchIndex instanceof DisabledProjectIndex)) {
- projectIndexes.setSearchIndex(new DisabledProjectIndex(searchIndex), false);
+ ProjectIndex maybeDisabledSearchIndex = projectIndexes.getSearchIndex();
+ if (!(maybeDisabledSearchIndex instanceof DisabledProjectIndex)) {
+ projectIndexes.setSearchIndex(new DisabledProjectIndex(maybeDisabledSearchIndex), false);
}
- return new AutoCloseable() {
- @Override
- public void close() {
- enableProjectIndexWrites();
- ProjectIndex searchIndex = projectIndexes.getSearchIndex();
- if (searchIndex instanceof DisabledProjectIndex) {
- projectIndexes.setSearchIndex(((DisabledProjectIndex) searchIndex).unwrap(), false);
- }
+ return () -> {
+ enableProjectIndexWrites();
+ ProjectIndex maybeEnabledSearchIndex = projectIndexes.getSearchIndex();
+ if (maybeEnabledSearchIndex instanceof DisabledProjectIndex) {
+ projectIndexes.setSearchIndex(
+ ((DisabledProjectIndex) maybeEnabledSearchIndex).unwrap(), false);
}
};
}
@@ -1216,13 +1208,6 @@
throws Exception {
TestRepository<?> localRepo = cloneProject(proj);
GitUtil.fetch(localRepo, "refs/*:refs/*");
- ImmutableMap<String, Ref> refs =
- localRepo
- .getRepository()
- .getRefDatabase()
- .getRefs()
- .stream()
- .collect(toImmutableMap(Ref::getName, r -> r));
Map<Branch.NameKey, RevTree> refValues = new HashMap<>();
for (Branch.NameKey b : trees.keySet()) {
@@ -1230,7 +1215,7 @@
continue;
}
- Ref r = refs.get(b.get());
+ Ref r = localRepo.getRepository().exactRef(b.get());
assertThat(r).isNotNull();
RevWalk rw = localRepo.getRevWalk();
RevCommit c = rw.parseCommit(r.getObjectId());
diff --git a/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java b/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
index 8420ff2..50536d8 100644
--- a/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
+++ b/java/com/google/gerrit/acceptance/AcceptanceTestRequestScope.java
@@ -145,7 +145,10 @@
return current.get();
}
- public Context disableDb() {
+ /**
+ * Disables read and write access to NoteDb and returns the context prior to that modification.
+ */
+ public Context disableNoteDb() {
Context old = current.get();
Context ctx = new Context(old.session, old.user, old.created);
diff --git a/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperations.java b/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperations.java
index a63d28a..e597ed0 100644
--- a/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/request/RequestScopeOperations.java
@@ -28,9 +28,9 @@
* Sets the Guice request scope to the given account.
*
* <p>The resulting context has an SSH session attached. In order to use the SSH session returned
- * by {@link AcceptanceTestRequestScope.Context#getSession()}, SSH must be enabled in the test and
- * the account must have a username set. However, these are not requirements simply to call this
- * method.
+ * by {@link com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context#getSession()}, SSH
+ * must be enabled in the test and the account must have a username set. However, these are not
+ * requirements simply to call this method.
*
* @param accountId account ID. Must exist; throws an unchecked exception otherwise.
* @return the previous request scope.
@@ -41,9 +41,9 @@
* Sets the Guice request scope to the given account.
*
* <p>The resulting context has an SSH session attached. In order to use the SSH session returned
- * by {@link AcceptanceTestRequestScope.Context#getSession()}, SSH must be enabled in the test and
- * the account must have a username set. However, these are not requirements simply to call this
- * method.
+ * by {@link com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context#getSession()}, SSH
+ * must be enabled in the test and the account must have a username set. However, these are not
+ * requirements simply to call this method.
*
* @param testAccount test account from {@code AccountOperations}.
* @return the previous request scope.
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index a8552f4..aa5362b 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -66,9 +66,9 @@
import com.google.gerrit.server.config.SysExecutorModule;
import com.google.gerrit.server.events.EventBroker;
import com.google.gerrit.server.events.StreamEventsApiListener;
-import com.google.gerrit.server.git.ChangeRefCache;
import com.google.gerrit.server.git.GarbageCollectionModule;
import com.google.gerrit.server.git.GitRepositoryManagerModule;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.IndexModule;
import com.google.gerrit.server.index.IndexModule.IndexType;
@@ -288,7 +288,7 @@
modules.add(cfgInjector.getInstance(GerritGlobalModule.class));
modules.add(new GerritApiModule());
modules.add(new PluginApiModule());
- modules.add(new ChangeRefCache.Module());
+ modules.add(new SearchingChangeCacheImpl.Module());
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultPermissionBackendModule());
modules.add(new DefaultMemoryCacheModule());
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 3c4f89b..c280a2d 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -73,8 +73,8 @@
import com.google.gerrit.server.config.SysExecutorModule;
import com.google.gerrit.server.events.EventBroker;
import com.google.gerrit.server.events.StreamEventsApiListener;
-import com.google.gerrit.server.git.ChangeRefCache;
import com.google.gerrit.server.git.GarbageCollectionModule;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.group.PeriodicGroupIndexer;
import com.google.gerrit.server.index.IndexModule;
@@ -393,7 +393,7 @@
modules.add(new GerritApiModule());
modules.add(new PluginApiModule());
- modules.add(new ChangeRefCache.Module());
+ modules.add(new SearchingChangeCacheImpl.Module(slave));
modules.add(new InternalAccountDirectory.Module());
modules.add(new DefaultPermissionBackendModule());
modules.add(new DefaultMemoryCacheModule());
diff --git a/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index f263786..b0c1c25 100644
--- a/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -56,8 +56,8 @@
import com.google.gerrit.server.extensions.events.EventUtil;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.extensions.events.RevisionCreated;
-import com.google.gerrit.server.git.ChangeRefCache;
import com.google.gerrit.server.git.MergeUtil;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.NoteDbModule;
@@ -133,7 +133,7 @@
// As Reindex is a batch program, don't assume the index is available for
// the change cache.
- bind(ChangeRefCache.class).toProvider(Providers.of(null));
+ bind(SearchingChangeCacheImpl.class).toProvider(Providers.of(null));
bind(new TypeLiteral<ImmutableSet<GroupReference>>() {})
.annotatedWith(AdministrateServerGroups.class)
diff --git a/java/com/google/gerrit/server/git/ChangeRefCache.java b/java/com/google/gerrit/server/git/ChangeRefCache.java
deleted file mode 100644
index dc1b946..0000000
--- a/java/com/google/gerrit/server/git/ChangeRefCache.java
+++ /dev/null
@@ -1,226 +0,0 @@
-// Copyright (C) 2019 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.server.git;
-
-import com.google.auto.value.AutoValue;
-import com.google.common.annotations.VisibleForTesting;
-import com.google.common.cache.CacheLoader;
-import com.google.common.cache.LoadingCache;
-import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.common.Nullable;
-import com.google.gerrit.index.RefState;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.ReviewerSet;
-import com.google.gerrit.server.cache.CacheModule;
-import com.google.gerrit.server.config.GerritOptions;
-import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.logging.TraceContext;
-import com.google.gerrit.server.logging.TraceContext.TraceTimer;
-import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
-import com.google.gerrit.server.util.ManualRequestContext;
-import com.google.gerrit.server.util.OneOffRequestContext;
-import com.google.gwtorm.server.OrmException;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import com.google.inject.TypeLiteral;
-import com.google.inject.name.Named;
-import java.util.List;
-import java.util.NoSuchElementException;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.CopyOnWriteArraySet;
-import org.eclipse.jgit.lib.Config;
-import org.eclipse.jgit.lib.ObjectId;
-
-/**
- * Cache for the minimal information per change that we need to compute visibility. Used for ref
- * filtering.
- *
- * <p>This class is thread safe.
- */
-@Singleton
-public class ChangeRefCache {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
- static final String ID_CACHE = "change_refs";
-
- public static class Module extends CacheModule {
- @Override
- protected void configure() {
- cache(ID_CACHE, Key.class, new TypeLiteral<CachedChange>() {})
- .maximumWeight(10000)
- .loader(Loader.class);
-
- bind(ChangeRefCache.class);
- }
- }
-
- @AutoValue
- abstract static class Key {
- abstract Project.NameKey project();
-
- abstract Change.Id changeId();
-
- abstract ObjectId metaId();
- }
-
- @AutoValue
- abstract static class CachedChange {
- // Subset of fields in ChangeData, specifically fields needed to serve
- // VisibleRefFilter without touching the database. More can be added as
- // necessary.
- abstract Change change();
-
- @Nullable
- abstract ReviewerSet reviewers();
- }
-
- private final LoadingCache<Key, CachedChange> cache;
- private final ChangeData.Factory changeDataFactory;
- private final OneOffRequestContext requestContext;
- private final Provider<InternalChangeQuery> queryProvider;
- private final GerritOptions gerritOptions;
- private final Config gerritConfig;
- private final Set<Project.NameKey> bootstrappedProjects;
-
- @Inject
- ChangeRefCache(
- @Named(ID_CACHE) LoadingCache<Key, CachedChange> cache,
- ChangeData.Factory changeDataFactory,
- OneOffRequestContext requestContext,
- Provider<InternalChangeQuery> queryProvider,
- GerritOptions gerritOptions,
- @GerritServerConfig Config gerritConfig) {
- this.cache = cache;
- this.changeDataFactory = changeDataFactory;
- this.requestContext = requestContext;
- this.queryProvider = queryProvider;
- this.gerritOptions = gerritOptions;
- this.gerritConfig = gerritConfig;
- // Uses a CopyOnWriteArraySet internally to keep track of projects that are already
- // bootstrapped. This is efficient because we read from the set on every call to this method to
- // check if bootstrapping is required. Writes occur only if we bootstrapped, so once per
- // project.
- this.bootstrappedProjects = new CopyOnWriteArraySet<>();
- }
-
- /**
- * Read changes from the cache.
- *
- * <p>Returned changes only include the {@code Change} object (with id, branch) and the reviewers.
- * There is no guarantee that additional fields are populated, although they can be.
- *
- * @param project project to read.
- * @param changeId change ID to read
- * @param metaId object ID of the meta branch to read. This is only used to ensure consistency. It
- * does not allow for reading non-current meta versions.
- * @return change data
- * @throws IllegalArgumentException in case no change is found
- */
- public ChangeData getChangeData(Project.NameKey project, Change.Id changeId, ObjectId metaId) {
- Key key = new AutoValue_ChangeRefCache_Key(project, changeId, metaId);
- CachedChange cached = cache.getUnchecked(key);
- if (cached == null) {
- throw new IllegalArgumentException("no change found for key " + key);
- }
- ChangeData cd = changeDataFactory.create(cached.change());
- cd.setReviewers(cached.reviewers());
- return cd;
- }
-
- /**
- * This method bootstraps the cache by querying the change index if it hasn't been bootstrapped
- * before, in which case it is a cheap no-op.
- *
- * @param project the project to bootstrap
- */
- public void bootstrapIfNecessary(Project.NameKey project) {
- if (!gerritOptions.enableMasterFeatures()) {
- // Bootstrapping using the ChangeIndex is only supported on master in a master-slave replica.
- return;
- }
- if (gerritConfig.getInt("cache", ID_CACHE, "memoryLimit", -1) == 0) {
- // The cache is disabled, don't bother bootstrapping.
- return;
- }
- if (bootstrappedProjects.contains(project)) {
- // We have bootstrapped for this project before. If the cache is too small, we might have
- // evicted all entries by now. Don't bother about this though as we don't want to add the
- // complexity of checking for existing projects, since that might not be authoritative as well
- // since we could have already evicted the majority of the entries.
- return;
- }
-
- try (TraceTimer ignored =
- TraceContext.newTimer("bootstrapping ChangeRef cache for project " + project);
- ManualRequestContext ignored2 = requestContext.open()) {
- List<ChangeData> cds =
- queryProvider
- .get()
- .setRequestedFields(ChangeField.CHANGE, ChangeField.REVIEWER, ChangeField.REF_STATE)
- .byProject(project);
- for (ChangeData cd : cds) {
- Set<RefState> refStates = RefState.parseStates(cd.getRefStates()).get(project);
- Optional<RefState> refState =
- refStates
- .stream()
- .filter(r -> r.ref().equals(RefNames.changeMetaRef(cd.getId())))
- .findAny();
- if (!refState.isPresent()) {
- continue;
- }
- cache.put(
- new AutoValue_ChangeRefCache_Key(project, cd.change().getId(), refState.get().id()),
- new AutoValue_ChangeRefCache_CachedChange(cd.change(), cd.getReviewers()));
- }
- // Mark the project as bootstrapped. We could have bootstrapped it multiple times for
- // simultaneous requests. We accept this in favor of less thread synchronization and
- // complexity.
- bootstrappedProjects.add(project);
- } catch (OrmException e) {
- logger.atWarning().withCause(e).log(
- "unable to bootstrap ChangeRef cache for project " + project);
- }
- }
-
- static class Loader extends CacheLoader<Key, CachedChange> {
- private final ChangeNotes.Factory notesFactory;
-
- @Inject
- Loader(ChangeNotes.Factory notesFactory) {
- this.notesFactory = notesFactory;
- }
-
- @Override
- public CachedChange load(Key key) throws Exception {
- ChangeNotes notes = notesFactory.create(key.project(), key.changeId());
- if (notes.getMetaId().equals(key.metaId())) {
- return new AutoValue_ChangeRefCache_CachedChange(notes.getChange(), notes.getReviewers());
- }
- throw new NoSuchElementException("unable to load change");
- }
- }
-
- @VisibleForTesting
- public void resetBootstrappedProjects() {
- bootstrappedProjects.clear();
- }
-}
diff --git a/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
new file mode 100644
index 0000000..0692ccf
--- /dev/null
+++ b/java/com/google/gerrit/server/git/SearchingChangeCacheImpl.java
@@ -0,0 +1,162 @@
+// Copyright (C) 2012 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.server.git;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.ReviewerSet;
+import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.logging.TraceContext;
+import com.google.gerrit.server.logging.TraceContext.TraceTimer;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.util.ManualRequestContext;
+import com.google.gerrit.server.util.OneOffRequestContext;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+import com.google.inject.TypeLiteral;
+import com.google.inject.name.Named;
+import com.google.inject.util.Providers;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.concurrent.ExecutionException;
+
+@Singleton
+public class SearchingChangeCacheImpl implements GitReferenceUpdatedListener {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ static final String ID_CACHE = "changes";
+
+ public static class Module extends CacheModule {
+ private final boolean slave;
+
+ public Module() {
+ this(false);
+ }
+
+ public Module(boolean slave) {
+ this.slave = slave;
+ }
+
+ @Override
+ protected void configure() {
+ if (slave) {
+ bind(SearchingChangeCacheImpl.class).toProvider(Providers.of(null));
+ } else {
+ cache(ID_CACHE, Project.NameKey.class, new TypeLiteral<List<CachedChange>>() {})
+ .maximumWeight(0)
+ .loader(Loader.class);
+
+ bind(SearchingChangeCacheImpl.class);
+ DynamicSet.bind(binder(), GitReferenceUpdatedListener.class)
+ .to(SearchingChangeCacheImpl.class);
+ }
+ }
+ }
+
+ @AutoValue
+ abstract static class CachedChange {
+ // Subset of fields in ChangeData, specifically fields needed to serve
+ // VisibleRefFilter without touching the database. More can be added as
+ // necessary.
+ abstract Change change();
+
+ @Nullable
+ abstract ReviewerSet reviewers();
+ }
+
+ private final LoadingCache<Project.NameKey, List<CachedChange>> cache;
+ private final ChangeData.Factory changeDataFactory;
+
+ @Inject
+ SearchingChangeCacheImpl(
+ @Named(ID_CACHE) LoadingCache<Project.NameKey, List<CachedChange>> cache,
+ ChangeData.Factory changeDataFactory) {
+ this.cache = cache;
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ /**
+ * Read changes for the project from the secondary index.
+ *
+ * <p>Returned changes only include the {@code Change} object (with id, branch) and the reviewers.
+ * Additional stored fields are not loaded from the index.
+ *
+ * @param project project to read.
+ * @return list of known changes; empty if no changes.
+ */
+ public List<ChangeData> getChangeData(Project.NameKey project) {
+ try {
+ List<CachedChange> cached = cache.get(project);
+ List<ChangeData> cds = new ArrayList<>(cached.size());
+ for (CachedChange cc : cached) {
+ ChangeData cd = changeDataFactory.create(cc.change());
+ cd.setReviewers(cc.reviewers());
+ cds.add(cd);
+ }
+ return Collections.unmodifiableList(cds);
+ } catch (ExecutionException e) {
+ logger.atWarning().withCause(e).log("Cannot fetch changes for %s", project);
+ return Collections.emptyList();
+ }
+ }
+
+ @Override
+ public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) {
+ if (event.getRefName().startsWith(RefNames.REFS_CHANGES)) {
+ cache.invalidate(new Project.NameKey(event.getProjectName()));
+ }
+ }
+
+ static class Loader extends CacheLoader<Project.NameKey, List<CachedChange>> {
+ private final OneOffRequestContext requestContext;
+ private final Provider<InternalChangeQuery> queryProvider;
+
+ @Inject
+ Loader(OneOffRequestContext requestContext, Provider<InternalChangeQuery> queryProvider) {
+ this.requestContext = requestContext;
+ this.queryProvider = queryProvider;
+ }
+
+ @Override
+ public List<CachedChange> load(Project.NameKey key) throws Exception {
+ try (TraceTimer timer = TraceContext.newTimer("Loading changes of project %s", key);
+ ManualRequestContext ctx = requestContext.open()) {
+ List<ChangeData> cds =
+ queryProvider
+ .get()
+ .setRequestedFields(ChangeField.CHANGE, ChangeField.REVIEWER)
+ .byProject(key);
+ List<CachedChange> result = new ArrayList<>(cds.size());
+ for (ChangeData cd : cds) {
+ result.add(
+ new AutoValue_SearchingChangeCacheImpl_CachedChange(cd.change(), cd.getReviewers()));
+ }
+ return Collections.unmodifiableList(result);
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 9e8f111..52dac9d 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -37,6 +37,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Table;
import com.google.common.flogger.FluentLogger;
+import com.google.common.io.Files;
import com.google.common.primitives.Longs;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitRequirement;
@@ -185,6 +186,26 @@
public static final FieldDef<ChangeData, Iterable<String>> FILE_PART =
exact(ChangeQueryBuilder.FIELD_FILEPART).buildRepeatable(ChangeField::getFileParts);
+ /** File extensions of each file modified in the current patch set. */
+ public static final FieldDef<ChangeData, Iterable<String>> EXTENSION =
+ exact(ChangeQueryBuilder.FIELD_EXTENSION).buildRepeatable(ChangeField::getExtensions);
+
+ public static Set<String> getExtensions(ChangeData cd) throws OrmException {
+ try {
+ return cd.currentFilePaths()
+ .stream()
+ // Use case-insensitive file extensions even though other file fields are case-sensitive.
+ // If we want to find "all Java files", we want to match both .java and .JAVA, even if we
+ // normally care about case sensitivity. (Whether we should change the existing file/path
+ // predicates to be case insensitive is a separate question.)
+ .map(f -> Files.getFileExtension(f).toLowerCase(Locale.US))
+ .filter(e -> !e.isEmpty())
+ .collect(toSet());
+ } catch (IOException e) {
+ throw new OrmException(e);
+ }
+ }
+
/** Owner/creator of the change. */
public static final FieldDef<ChangeData, Integer> OWNER =
integer(ChangeQueryBuilder.FIELD_OWNER).build(changeGetter(c -> c.getOwner().get()));
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 9016fd1..cd24c92 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -101,7 +101,9 @@
// Bump Lucene version requires reindexing
@Deprecated static final Schema<ChangeData> V50 = schema(V49);
- static final Schema<ChangeData> V51 = schema(V50, ChangeField.TOTAL_COMMENT_COUNT);
+ @Deprecated static final Schema<ChangeData> V51 = schema(V50, ChangeField.TOTAL_COMMENT_COUNT);
+
+ static final Schema<ChangeData> V52 = schema(V51, ChangeField.EXTENSION);
public static final String NAME = "changes";
public static final ChangeSchemaDefinitions INSTANCE = new ChangeSchemaDefinitions();
diff --git a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
index 56c22e1..00df11a 100644
--- a/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
+++ b/java/com/google/gerrit/server/permissions/DefaultRefFilter.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.permissions;
+import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CACHE_AUTOMERGE;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CHANGES;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_CONFIG;
@@ -21,7 +22,10 @@
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.toMap;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.metrics.Counter0;
@@ -37,11 +41,12 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.config.GerritServerConfig;
-import com.google.gerrit.server.git.ChangeRefCache;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TagMatcher;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.gerrit.server.notedb.ChangeNotes.Factory.ChangeNotesResult;
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData;
@@ -50,11 +55,10 @@
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
-import java.util.Set;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
@@ -62,13 +66,15 @@
import org.eclipse.jgit.lib.SymbolicRef;
class DefaultRefFilter {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
interface Factory {
DefaultRefFilter create(ProjectControl projectControl);
}
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
- @Nullable private final ChangeRefCache changeCache;
+ @Nullable private final SearchingChangeCacheImpl changeCache;
private final GroupCache groupCache;
private final PermissionBackend permissionBackend;
private final ProjectControl projectControl;
@@ -78,14 +84,14 @@
private final Counter0 fullFilterCount;
private final Counter0 skipFilterCount;
private final boolean skipFullRefEvaluationIfAllRefsAreVisible;
- private final Map<Change.Id, Branch.NameKey> visibleChanges;
- private final Set<Change.Id> inVisibleChanges;
+
+ private Map<Change.Id, Branch.NameKey> visibleChanges;
@Inject
DefaultRefFilter(
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
- @Nullable ChangeRefCache changeCache,
+ @Nullable SearchingChangeCacheImpl changeCache,
GroupCache groupCache,
PermissionBackend permissionBackend,
@GerritServerConfig Config config,
@@ -115,9 +121,6 @@
"Rate of ref filter operations where we skip full evaluation"
+ " because the user can read all refs")
.setRate());
- // TODO(hiesel): Rework who can see change edits so that we can keep just a single set here.
- this.visibleChanges = new HashMap<>();
- this.inVisibleChanges = new HashSet<>();
}
Map<String, Ref> filter(Map<String, Ref> refs, Repository repo, RefFilterOptions opts)
@@ -158,7 +161,6 @@
Map<String, Ref> result = new HashMap<>();
List<Ref> deferredTags = new ArrayList<>();
- changeCache.bootstrapIfNecessary(projectState.getNameKey());
for (Ref ref : refs.values()) {
String name = ref.getName();
@@ -297,64 +299,111 @@
}
private boolean visible(Repository repo, Change.Id changeId) throws PermissionBackendException {
- if (visibleChanges.containsKey(changeId)) {
- return true;
- }
- if (inVisibleChanges.contains(changeId)) {
- return false;
- }
- // TODO(hiesel): The project state should be checked once in the beginning an left alone
- // thereafter.
- if (!projectState.statePermitsRead()) {
- return false;
- }
-
- Project.NameKey project = projectState.getNameKey();
- try {
- ChangeData cd =
- changeCache.getChangeData(
- project, changeId, repo.exactRef(RefNames.changeMetaRef(changeId)).getObjectId());
- ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
- try {
- permissionBackendForProject.indexedChange(cd, notes).check(ChangePermission.READ);
- visibleChanges.put(cd.getId(), cd.change().getDest());
- return true;
- } catch (AuthException e) {
- inVisibleChanges.add(changeId);
- return false;
+ if (visibleChanges == null) {
+ if (changeCache == null) {
+ visibleChanges = visibleChangesByScan(repo);
+ } else {
+ visibleChanges = visibleChangesBySearch();
}
- } catch (OrmException | IOException e) {
- throw new PermissionBackendException(
- "Cannot load change " + changeId + " for project " + project + ", assuming not visible",
- e);
}
+ return visibleChanges.containsKey(changeId);
}
private boolean visibleEdit(Repository repo, String name) throws PermissionBackendException {
Change.Id id = Change.Id.fromEditRefPart(name);
- if (id == null) {
- return false;
+ // Initialize if it wasn't yet
+ if (visibleChanges == null) {
+ visible(repo, id);
}
- if (!visible(repo, id)) {
- // Can't see the change, so can't see the edit.
+ if (id == null) {
return false;
}
if (user.isIdentifiedUser()
- && name.startsWith(RefNames.refsEditPrefix(user.asIdentifiedUser().getAccountId()))) {
- // Own edit
+ && name.startsWith(RefNames.refsEditPrefix(user.asIdentifiedUser().getAccountId()))
+ && visible(repo, id)) {
return true;
}
+ if (visibleChanges.containsKey(id)) {
+ try {
+ // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
+ permissionBackendForProject
+ .ref(visibleChanges.get(id).get())
+ .check(RefPermission.READ_PRIVATE_CHANGES);
+ return true;
+ } catch (AuthException e) {
+ return false;
+ }
+ }
+ return false;
+ }
+
+ private Map<Change.Id, Branch.NameKey> visibleChangesBySearch()
+ throws PermissionBackendException {
+ Project.NameKey project = projectState.getNameKey();
+ try {
+ Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
+ for (ChangeData cd : changeCache.getChangeData(project)) {
+ ChangeNotes notes = changeNotesFactory.createFromIndexedChange(cd.change());
+ if (!projectState.statePermitsRead()) {
+ continue;
+ }
+ try {
+ permissionBackendForProject.indexedChange(cd, notes).check(ChangePermission.READ);
+ visibleChanges.put(cd.getId(), cd.change().getDest());
+ } catch (AuthException e) {
+ // Do nothing.
+ }
+ }
+ return visibleChanges;
+ } catch (OrmException e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot load changes for project %s, assuming no changes are visible", project);
+ return Collections.emptyMap();
+ }
+ }
+
+ private Map<Change.Id, Branch.NameKey> visibleChangesByScan(Repository repo)
+ throws PermissionBackendException {
+ Project.NameKey p = projectState.getNameKey();
+ ImmutableList<ChangeNotesResult> changes;
+ try {
+ changes = changeNotesFactory.scan(repo, p).collect(toImmutableList());
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log(
+ "Cannot load changes for project %s, assuming no changes are visible", p);
+ return Collections.emptyMap();
+ }
+
+ Map<Change.Id, Branch.NameKey> result = Maps.newHashMapWithExpectedSize(changes.size());
+ for (ChangeNotesResult notesResult : changes) {
+ ChangeNotes notes = toNotes(notesResult);
+ if (notes != null) {
+ result.put(notes.getChangeId(), notes.getChange().getDest());
+ }
+ }
+ return result;
+ }
+
+ @Nullable
+ private ChangeNotes toNotes(ChangeNotesResult r) throws PermissionBackendException {
+ if (r.error().isPresent()) {
+ logger.atWarning().withCause(r.error().get()).log(
+ "Failed to load change %s in %s", r.id(), projectState.getName());
+ return null;
+ }
+
+ if (!projectState.statePermitsRead()) {
+ return null;
+ }
try {
- // Default to READ_PRIVATE_CHANGES as there is no special permission for reading edits.
- permissionBackendForProject
- .ref(visibleChanges.get(id).get())
- .check(RefPermission.READ_PRIVATE_CHANGES);
- return true;
+ permissionBackendForProject.change(r.notes()).check(ChangePermission.READ);
+ return r.notes();
} catch (AuthException e) {
- return false;
+ // Skip.
}
+ return null;
}
private boolean isMetadata(String name) {
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index f5df87b..8885f7e 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -137,6 +137,7 @@
public static final String FIELD_COMMIT = "commit";
public static final String FIELD_COMMITTER = "committer";
public static final String FIELD_EXACTCOMMITTER = "exactcommitter";
+ public static final String FIELD_EXTENSION = "extension";
public static final String FIELD_CONFLICTS = "conflicts";
public static final String FIELD_DELETED = "deleted";
public static final String FIELD_DELTA = "delta";
@@ -733,6 +734,16 @@
}
@Operator
+ public Predicate<ChangeData> ext(String ext) {
+ return extension(ext);
+ }
+
+ @Operator
+ public Predicate<ChangeData> extension(String ext) {
+ return new FileExtensionPredicate(ext);
+ }
+
+ @Operator
public Predicate<ChangeData> label(String name)
throws QueryParseException, OrmException, IOException, ConfigInvalidException {
Set<Account.Id> accounts = null;
diff --git a/java/com/google/gerrit/server/query/change/FileExtensionPredicate.java b/java/com/google/gerrit/server/query/change/FileExtensionPredicate.java
new file mode 100644
index 0000000..ee5030a
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/FileExtensionPredicate.java
@@ -0,0 +1,42 @@
+// Copyright (C) 2019 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.server.query.change;
+
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gwtorm.server.OrmException;
+import java.util.Locale;
+
+public class FileExtensionPredicate extends ChangeIndexPredicate {
+ private static String clean(String ext) {
+ if (ext.startsWith(".")) {
+ ext = ext.substring(1);
+ }
+ return ext.toLowerCase(Locale.US);
+ }
+
+ FileExtensionPredicate(String value) {
+ super(ChangeField.EXTENSION, clean(value));
+ }
+
+ @Override
+ public boolean match(ChangeData object) throws OrmException {
+ return ChangeField.getExtensions(object).contains(value);
+ }
+
+ @Override
+ public int getCost() {
+ return 0;
+ }
+}
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 2a030ac..c8cea6f 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -58,10 +58,10 @@
import com.google.gerrit.server.config.SitePath;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.config.TrackingFootersProvider;
-import com.google.gerrit.server.git.ChangeRefCache;
import com.google.gerrit.server.git.GarbageCollection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.PerThreadRequestScope;
+import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.WorkQueue;
import com.google.gerrit.server.index.IndexModule.IndexType;
import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
@@ -168,7 +168,7 @@
factory(PluginUser.Factory.class);
install(new PluginApiModule());
install(new DefaultPermissionBackendModule());
- install(new ChangeRefCache.Module());
+ install(new SearchingChangeCacheImpl.Module());
factory(GarbageCollection.Factory.class);
install(new AuditModule());
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 81acb3f..31de4cf 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -58,7 +58,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.ChangeIndexedCounter;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.GitUtil;
@@ -2735,8 +2734,7 @@
createChange();
requestScopeOperations.setApiUser(user.getId());
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
assertThat(
gApi.changes()
.query()
@@ -2747,8 +2745,6 @@
.withOption(REVIEWED)
.get())
.hasSize(2);
- } finally {
- enableDb(ctx);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 90f0a2a..b00837d 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -24,7 +24,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -923,16 +922,13 @@
addRobotComment(r2.getChangeId(), createRobotCommentInputWithMandatoryFields());
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo result = Iterables.getOnlyElement(query(r2.getChangeId()));
// currently, we create all robot comments as 'resolved' by default.
// if we allow users to resolve a robot comment, then this test should
// be modified.
assertThat(result.unresolvedCommentCount).isEqualTo(0);
assertThat(result.totalCommentCount).isEqualTo(1);
- } finally {
- enableDb(ctx);
}
}
diff --git a/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java b/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java
deleted file mode 100644
index 3939c67..0000000
--- a/javatests/com/google/gerrit/acceptance/git/ChangeRefCacheIT.java
+++ /dev/null
@@ -1,271 +0,0 @@
-// Copyright (C) 2019 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.git;
-
-import static com.google.common.truth.Truth.assertThat;
-import static java.util.stream.Collectors.toMap;
-
-import com.google.common.collect.ImmutableList;
-import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
-import com.google.gerrit.acceptance.NoHttpd;
-import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.git.ChangeRefCache;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
-import com.google.gerrit.server.query.change.ChangeData;
-import java.io.IOException;
-import java.util.List;
-import java.util.Map;
-import java.util.function.Function;
-import javax.inject.Inject;
-import org.eclipse.jgit.lib.Ref;
-import org.eclipse.jgit.lib.Repository;
-import org.junit.Before;
-import org.junit.Test;
-
-/**
- * Tests the ChangeRefCache by running ls-remote calls and conditionally disabling the index and
- * NoteDb. The cache is enabled by default.
- *
- * <p>Why are we not just testing ChangeRefCache directly? Our ref filtering code is rather complex
- * and it is easy to get something wrong there. We want our assumptions about the performance of the
- * cache to be validated against the entire component rather than just the cache.
- */
-@NoHttpd
-public class ChangeRefCacheIT extends AbstractDaemonTest {
-
- @Inject private ChangeRefCache changeRefCache;
- @Inject private PermissionBackend permissionBackend;
- @Inject private RequestScopeOperations requestScopeOperations;
-
- @Before
- public void setUp() throws Exception {
- // We want full ref evaluation so that we hit the cache every time.
- baseConfig.setBoolean("auth", null, "skipFullRefEvaluationIfAllRefsAreVisible", false);
- }
-
- /**
- * Ensure we use only the change index for getting initial data populated and don't touch NoteDb.
- */
- @Test
- public void useIndexForBootstrapping() throws Exception {
- ChangeData change = createChange().getChange();
- // TODO(hiesel) Rework as AutoClosable. Here and below.
- changeRefCache.resetBootstrappedProjects();
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change.getId()),
- change.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
- }
- }
-
- /**
- * Ensure we use only the change index for getting initial data populated and don't require any
- * storage backend after that as long the data didn't change.
- */
- @Test
- public void serveResultsFromCacheAfterInitialBootstrap() throws Exception {
- ChangeData change = createChange().getChange();
- changeRefCache.resetBootstrappedProjects();
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change.getId()),
- change.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
- }
-
- // No change since our first call, so this time we don't bootstrap or touch NoteDb
- AcceptanceTestRequestScope.Context ctx2 = disableDb();
- try {
- try (AutoCloseable ignored = disableChangeIndex()) {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change.getId()),
- change.currentPatchSet().getId().toRefName());
- }
- } finally {
- enableDb(ctx2);
- }
- }
-
- /**
- * Ensure we use only the change index for getting initial data populated and NoteDb for reloading
- * data that changed since.
- */
- @Test
- public void useIndexForBootstrappingAndDbForDeltaReload() throws Exception {
- ChangeData change1 = createChange().getChange();
- AcceptanceTestRequestScope.Context ctx = disableDb();
- // Bootstrap: No NoteDb access as we expect it to use the index.
- changeRefCache.resetBootstrappedProjects();
- try {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
- }
- // Delta reload: No index access as we expect it to use NoteDb.
- ChangeData change2 = createChange().getChange();
- try (AutoCloseable ignored = disableChangeIndex()) {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName(),
- RefNames.changeMetaRef(change2.getId()),
- change2.currentPatchSet().getId().toRefName());
- }
- }
-
- /**
- * Ensure we use only the change index for getting initial data populated and NoteDb for reloading
- * data that changed since.
- */
- @Test
- public void useDbForDeltaReloadOnNewPatchSet() throws Exception {
- ChangeData change1 =
- pushFactory
- .create(admin.getIdent(), testRepo, "original subject", "a", "a1")
- .to("refs/for/master")
- .getChange();
-
- AcceptanceTestRequestScope.Context ctx = disableDb();
- // Bootstrap: No NoteDb access as we expect it to use the index.
- changeRefCache.resetBootstrappedProjects();
- try {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
- }
-
- // Delta reload: No index access as we expect it to use NoteDb.
- ChangeData change2 =
- pushFactory
- .create(
- admin.getIdent(), testRepo, "subject2", "a", "a2", change1.change().getKey().get())
- .to("refs/for/master")
- .getChange();
- List<PatchSet> patchSets = ImmutableList.copyOf(change2.patchSets());
- assertThat(patchSets).hasSize(2);
- try (AutoCloseable ctx2 = disableChangeIndex()) {
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- patchSets.get(0).getId().toRefName(),
- patchSets.get(1).getId().toRefName());
- }
- }
-
- /**
- * Ensure we use only the change index for getting initial data populated and NoteDb for reloading
- * data that changed since.
- */
- @Test
- public void useDbForIterativeFetchingOnMetadataChange() throws Exception {
- ChangeData change1 =
- pushFactory
- .create(admin.getIdent(), testRepo, "original subject", "a", "a1")
- .to("refs/for/master")
- .getChange();
- // Bootstrap: No NoteDb access as we expect it to use the index.
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
- changeRefCache.resetBootstrappedProjects();
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName());
- } finally {
- enableDb(ctx);
- }
-
- try (AutoCloseable ignored = disableChangeIndex()) {
- // user can see public change
- requestScopeOperations.setApiUser(user.getId());
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName());
- }
-
- // Delta reload: No index access as we expect it to use NoteDb.
- requestScopeOperations.setApiUser(admin.getId());
- gApi.changes().id(change1.getId().id).setPrivate(true);
-
- try (AutoCloseable ignored = disableChangeIndex()) {
- // user can't see private change from admin
- requestScopeOperations.setApiUser(user.getId());
- assertUploadPackRefs("HEAD", "refs/heads/master");
- }
-
- // admin adds the user as reviewer
- requestScopeOperations.setApiUser(admin.getId());
- gApi.changes().id(change1.getId().id).addReviewer(user.email);
-
- try (AutoCloseable ignored = disableChangeIndex()) {
- // Use can see private change
- requestScopeOperations.setApiUser(user.getId());
- assertUploadPackRefs(
- "HEAD",
- "refs/heads/master",
- RefNames.changeMetaRef(change1.getId()),
- change1.currentPatchSet().getId().toRefName());
- }
- }
-
- private void assertUploadPackRefs(String... expectedRefs) throws Exception {
- try (Repository repo = repoManager.openRepository(project)) {
- assertRefs(repo, permissionBackend.user(user(user)).project(project), expectedRefs);
- }
- }
-
- private void assertRefs(
- Repository repo, PermissionBackend.ForProject forProject, String... expectedRefs)
- throws Exception {
- Map<String, Ref> all = getAllRefs(repo);
- assertThat(forProject.filter(all, repo, RefFilterOptions.defaults()).keySet())
- .containsExactlyElementsIn(expectedRefs);
- }
-
- private static Map<String, Ref> getAllRefs(Repository repo) throws IOException {
- return repo.getRefDatabase()
- .getRefs()
- .stream()
- .collect(toMap(Ref::getName, Function.identity()));
- }
-}
diff --git a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
index 79a6957..91d5c32 100644
--- a/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java
@@ -358,6 +358,7 @@
repo,
permissionBackend.user(user(user)).project(project),
// Can't use stored values from the index so DB must be enabled.
+ false,
"HEAD",
psRef1,
metaRef1,
@@ -377,10 +378,10 @@
@Test
public void uploadPackSequencesWithAccessDatabase() throws Exception {
try (Repository repo = repoManager.openRepository(allProjects)) {
- assertRefs(repo, newFilter(allProjects, user));
+ assertRefs(repo, newFilter(allProjects, user), true);
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
- assertRefs(repo, newFilter(allProjects, user), "refs/sequences/changes");
+ assertRefs(repo, newFilter(allProjects, user), true, "refs/sequences/changes");
}
}
@@ -728,16 +729,29 @@
*/
private void assertUploadPackRefs(String... expectedRefs) throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
- assertRefs(repo, permissionBackend.user(user(user)).project(project), expectedRefs);
+ assertRefs(repo, permissionBackend.user(user(user)).project(project), true, expectedRefs);
}
}
private void assertRefs(
- Repository repo, PermissionBackend.ForProject forProject, String... expectedRefs)
+ Repository repo,
+ PermissionBackend.ForProject forProject,
+ boolean disableDb,
+ String... expectedRefs)
throws Exception {
- Map<String, Ref> all = getAllRefs(repo);
- assertThat(forProject.filter(all, repo, RefFilterOptions.defaults()).keySet())
- .containsExactlyElementsIn(expectedRefs);
+ AutoCloseable ctx = null;
+ if (disableDb) {
+ ctx = disableNoteDb();
+ }
+ try {
+ Map<String, Ref> all = getAllRefs(repo);
+ assertThat(forProject.filter(all, repo, RefFilterOptions.defaults()).keySet())
+ .containsExactlyElementsIn(expectedRefs);
+ } finally {
+ if (disableDb) {
+ ctx.close();
+ }
+ }
}
private ReceiveCommitsAdvertiseRefsHook.Result getReceivePackRefs() throws Exception {
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
index 13c76e3..b5d3838 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/ChangeReviewersByEmailIT.java
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope.Context;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -323,14 +322,11 @@
input.state = state;
gApi.changes().id(r.getChangeId()).addReviewer(input);
- Context oldCtx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo info =
Iterables.getOnlyElement(
gApi.changes().query(r.getChangeId()).withOption(DETAILED_LABELS).get());
assertThat(info.reviewers).isEqualTo(ImmutableMap.of(state, ImmutableList.of(acc)));
- } finally {
- enableDb(oldCtx);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 0a4d972..d5ceb9a 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -27,7 +27,6 @@
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
@@ -734,8 +733,7 @@
assertThat(comments.get(FILE_NAME)).hasSize(1);
addComment(result, "comment 2", false, true, comments.get(FILE_NAME).get(0).id);
- AcceptanceTestRequestScope.Context ctx = disableDb();
- try {
+ try (AutoCloseable ignored = disableNoteDb()) {
ChangeInfo changeInfo1 = Iterables.getOnlyElement(query(changeId1));
ChangeInfo changeInfo2 = Iterables.getOnlyElement(query(changeId2));
ChangeInfo changeInfo3 = Iterables.getOnlyElement(query(changeId3));
@@ -745,8 +743,6 @@
assertThat(changeInfo2.totalCommentCount).isEqualTo(2);
assertThat(changeInfo3.unresolvedCommentCount).isEqualTo(1);
assertThat(changeInfo3.totalCommentCount).isEqualTo(2);
- } finally {
- enableDb(ctx);
}
}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 58ea1d2..6390caa 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -126,6 +126,7 @@
import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.junit.TestRepository.CommitBuilder;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
@@ -1323,14 +1324,7 @@
@Test
public void byFileExact() throws Exception {
TestRepository<Repo> repo = createProject("repo");
- RevCommit commit =
- repo.parseBody(
- repo.commit()
- .message("one")
- .add("dir/file1", "contents1")
- .add("dir/file2", "contents2")
- .create());
- Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change change = insert(repo, newChangeWithFiles(repo, "dir/file1", "dir/file2"));
assertQuery("file:file");
assertQuery("file:dir", change);
@@ -1343,14 +1337,7 @@
@Test
public void byFileRegex() throws Exception {
TestRepository<Repo> repo = createProject("repo");
- RevCommit commit =
- repo.parseBody(
- repo.commit()
- .message("one")
- .add("dir/file1", "contents1")
- .add("dir/file2", "contents2")
- .create());
- Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change change = insert(repo, newChangeWithFiles(repo, "dir/file1", "dir/file2"));
assertQuery("file:.*file.*");
assertQuery("file:^file.*"); // Whole path only.
@@ -1360,14 +1347,7 @@
@Test
public void byPathExact() throws Exception {
TestRepository<Repo> repo = createProject("repo");
- RevCommit commit =
- repo.parseBody(
- repo.commit()
- .message("one")
- .add("dir/file1", "contents1")
- .add("dir/file2", "contents2")
- .create());
- Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change change = insert(repo, newChangeWithFiles(repo, "dir/file1", "dir/file2"));
assertQuery("path:file");
assertQuery("path:dir");
@@ -1380,20 +1360,31 @@
@Test
public void byPathRegex() throws Exception {
TestRepository<Repo> repo = createProject("repo");
- RevCommit commit =
- repo.parseBody(
- repo.commit()
- .message("one")
- .add("dir/file1", "contents1")
- .add("dir/file2", "contents2")
- .create());
- Change change = insert(repo, newChangeForCommit(repo, commit));
+ Change change = insert(repo, newChangeWithFiles(repo, "dir/file1", "dir/file2"));
assertQuery("path:.*file.*");
assertQuery("path:^dir.file.*", change);
}
@Test
+ public void byExtension() throws Exception {
+ assume().that(getSchema().hasField(ChangeField.EXTENSION)).isTrue();
+
+ TestRepository<Repo> repo = createProject("repo");
+ Change change1 = insert(repo, newChangeWithFiles(repo, "foo.h", "foo.cc"));
+ Change change2 = insert(repo, newChangeWithFiles(repo, "bar.H", "bar.CC"));
+ Change change3 = insert(repo, newChangeWithFiles(repo, "dir/baz.h", "dir/baz.cc"));
+ Change change4 = insert(repo, newChangeWithFiles(repo, "Quux.java"));
+
+ assertQuery("extension:java", change4);
+ assertQuery("ext:java", change4);
+ assertQuery("ext:.java", change4);
+ assertQuery("ext:jAvA", change4);
+ assertQuery("ext:.jAvA", change4);
+ assertQuery("ext:cc", change3, change2, change1);
+ }
+
+ @Test
public void byComment() throws Exception {
TestRepository<Repo> repo = createProject("repo");
ChangeInserter ins = newChange(repo);
@@ -2879,6 +2870,15 @@
return newChange(repo, commit, null, null, null, false);
}
+ protected ChangeInserter newChangeWithFiles(TestRepository<Repo> repo, String... paths)
+ throws Exception {
+ CommitBuilder b = repo.commit().message("Change with files");
+ for (String path : paths) {
+ b.add(path, "contents of " + path);
+ }
+ return newChangeForCommit(repo, repo.parseBody(b.create()));
+ }
+
protected ChangeInserter newChangeForBranch(TestRepository<Repo> repo, String branch)
throws Exception {
return newChange(repo, null, branch, null, null, false);
diff --git a/plugins/download-commands b/plugins/download-commands
index edd7156..ce8f072 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit edd715618415d9a5e03b4555d9a2d3cca8fff6e8
+Subproject commit ce8f07234c96e563cfc60aae33b64e2148e0e192
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
index a81526c..8aff87e 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
@@ -36,6 +36,8 @@
'conflicts:',
'deleted:',
'delta:',
+ 'ext:',
+ 'extension:',
'file:',
'from:',
'has:',
