Merge "Honor --slave and --headless options better"
diff --git a/.mailmap b/.mailmap
index f2fe6ca..cbf1f3b 100644
--- a/.mailmap
+++ b/.mailmap
@@ -38,6 +38,7 @@
Johan Björk <jbjoerk@gmail.com> Johan Bjork <phb@spotify.com>
JT Olds <hello@jtolds.com> <jtolds@gmail.com>
Kasper Nilsson <kaspern@google.com> <kaspern@google.com>
+Lawrence Dubé <ldube@audiokinetic.com> <ldube@audiokinetic.com>
Lei Sun <lei.sun01@sap.com> LeiSun <lei.sun01@sap.com>
Lincoln Oliveira Campos Do Nascimento <lincoln.oliveiracamposdonascimento@sonyericsson.com> lincoln <lincoln.oliveiracamposdonascimento@sonyericsson.com>
Luca Milanesio <luca.milanesio@gmail.com> <luca@gitent-scm.com>
diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt
index 589071a..6362597 100644
--- a/Documentation/config-gerrit.txt
+++ b/Documentation/config-gerrit.txt
@@ -4654,103 +4654,6 @@
+
By default 0.
-[[theme]]
-=== Section theme
-
-[[theme.backgroundColor]]theme.backgroundColor::
-+
-_(GWT UI only)_ Background color for the page, and major data tables like the all
-open changes table or the account dashboard. The value must be a
-valid HTML hex color code, or standard color name.
-+
-By default white, `FFFFFF`.
-
-[[theme.topMenuColor]]theme.topMenuColor::
-+
-_(GWT UI only)_ This is the color of the main menu bar at the top of the page.
-The value must be a valid HTML hex color code, or standard color
-name.
-+
-By default white, `FFFFFF`.
-
-[[theme.textColor]]theme.textColor::
-+
-_(GWT UI only)_ Text color for the page, and major data tables like the all open
-changes table or the account dashboard. The value must be a valid HTML hex color
-code, or standard color name.
-+
-By default dark grey, `353535`.
-
-[[theme.trimColor]]theme.trimColor::
-+
-_(GWT UI only)_ Primary color used as a background color behind text. This is
-the color of the main menu bar at the top, of table headers, and of major UI
-areas that we want to offset from other portions of the page. The value must be
-a valid HTML hex color code, or standard color name.
-+
-By default a light grey, `EEEEEE`.
-
-[[theme.selectionColor]]theme.selectionColor::
-+
-_(GWT UI only)_ Background color used within a trimColor area to denote the
-currently selected tab, or the background color used in a table to denote the
-currently selected row. The value must be a valid HTML hex color code, or
-standard color name.
-+
-By default a pale blue, `D8EDF9`.
-
-[[theme.changeTableOutdatedColor]]theme.changeTableOutdatedColor::
-+
-_(GWT UI only)_ Background color used for patch outdated messages. The value
-must be a valid HTML hex color code, or standard color name.
-+
-By default a shade of red, `F08080`.
-
-[[theme.tableOddRowColor]]theme.tableOddRowColor::
-+
-_(GWT UI only)_ Background color for tables such as lists of open reviews for
-odd rows. This is so you can have a different color for odd and even rows of
-the table. The value must be a valid HTML hex color code, or standard color
-name.
-+
-By default transparent.
-
-[[theme.tableEvenRowColor]]theme.tableEvenRowColor::
-+
-_(GWT UI only)_ Background color for tables such as lists of open reviews for
-even rows. This is so you can have a different color for odd and even rows of
-the table. The value must be a valid HTML hex color code, or standard color
-name.
-+
-By default transparent.
-
-A different theme may be used for signed-in vs. signed-out user status
-by using the "signed-in" and "signed-out" theme sections. Variables
-not specified in a section are inherited from the default theme.
-
-----
-[theme]
- backgroundColor = FFFFFF
-[theme "signed-in"]
- backgroundColor = C0C0C0
-[theme "signed-out"]
- backgroundColor = 00FFFF
-----
-
-As example, here is the theme configuration to have the old green look:
-
-----
-[theme]
- backgroundColor = FCFEEF
- textColor = 000000
- trimColor = D4E9A9
- selectionColor = FFFFCC
- topMenuColor = D4E9A9
- changeTableOutdatedColor = F08080
-[theme "signed-in"]
- backgroundColor = FFFFFF
-----
-
[[trackingid]]
=== Section trackingid
@@ -5056,6 +4959,38 @@
Assuming that the server is started on `Mon 07:00` then this yields the
first run on Tuesday at 06:00 and a repetition interval of 1 day.
+[[All-Projects-project.config]]
+== File `etc/All-Projects/project.config`
+
+The optional file `'$site_path'/etc/All-Projects/project.config` provides
+defaults for configuration read from
+link:config-project-config.html[`project.config`] in the
+`All-Projects` repo. Unlike `gerrit.config`, this file contains project-type
+configuration rather than server-type configuration.
+
+Most administrators will not need this file, and should instead make commits to
+`All-Projects` to modify global config. However, a separate file can be useful
+when managing multiple Gerrit servers, since pushing changes to defaults using
+Puppet or a similar tool can be easier than scripting git updates to
+`All-Projects`.
+
+The contents of the file are loaded each time the `All-Projects` project is
+reloaded. Updating the file requires either evicting the project cache or
+restarting the server.
+
+Caveats:
+
+* The path from which the file is read corresponds to the name of the repo,
+ which is link:#gerrit.allProjects[configurable].
+* Although the file lives in a directory that shares a name with a repository,
+ this directory is not a Git repository.
+* Only the file `project.config` is read from this directory to provide
+ defaults; any other files in this directory, such as `rules.pl`, are ignored.
+ (This behavior may change in the future.)
+* Group names listed in the access config in this file are resolved to UUIDs
+ using the `groups` file in the repository, not in the config directory. As a
+ result, setting ACLs in this file is not recommended.
+
[[secure.config]]
== File `etc/secure.config`
@@ -5095,10 +5030,6 @@
The format is one Base-64 encoded public key per line.
-== Configuring the Polygerrit UI
-
-Please see link:dev-polygerrit.html[UI] on configuring the Polygerrit UI.
-
=== Configurable Parameters
site_path::
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index d5ffda2..d7bd8b3 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -801,6 +801,64 @@
}
----
+=== Calling Command Options ===
+
+Within an OptionHandler, during the processing of an option, plugins can
+provide and call extra parameters on the current command during parsing
+simulating as if they had been passed from the command line originally.
+
+To call additional parameters from within an option handler, instantiate
+the com.google.gerrit.util.cli.CmdLineParser.Parameters class with the
+existing parameters, and then call callParameters() with the additional
+parameters to be parsed. OptionHandlers may optionally pass this class to
+other methods which may then both parse/consume more parameters and call
+additional parameters.
+
+When calling command options not provided by your plugin, there is always
+a risk that the options may not exist, perhaps because the options being
+called are to be provided by another plugin, and said plugin is not
+currently installed. To protect againt this situation, it is possible to
+define an option as being dependent on other options using the
+@RequiresOptions() annotation. If the required options are not all not
+currently present, then the dependent option will not be available or
+visible in the help.
+
+The example below shows a plugin that adds a "--special" option (perhaps
+for use with the Query command) that calls (and requires) the
+"--format json" option.
+
+[source, java]
+----
+public class JsonOutputOptionHandler<T> extends OptionHandler<T> {
+ protected com.google.gerrit.util.cli.CmdLineParser.MyParser myParser;
+
+ public JsonOutputOptionHandler(CmdLineParser parser, OptionDef option, Setter<? super T> setter) {
+ super(parser, option, setter);
+ myParser = (com.google.gerrit.util.cli.CmdLineParser.MyParser) owner;
+ }
+
+ @Override
+ public int parseArguments(org.kohsuke.args4j.spi.Parameters params) throws CmdLineException {
+ new Parameters(params, myParser).callParameters("--format", "json");
+ setter.addValue(true);
+ return 0; // we didn't consume any additional args
+ }
+
+ @Override
+ public String getDefaultMetaVariable() {
+ ...
+ }
+}
+
+@RequiresOptions("--format")
+@Option(
+ name = "--special",
+ usage = "ouptut results using json",
+ handler = JsonOutputOptionHandler.class
+)
+boolean json;
+----
+
[[query_attributes]]
=== Query Attributes ===
diff --git a/Documentation/dev-polygerrit.txt b/Documentation/dev-polygerrit.txt
deleted file mode 100644
index 5621d32..0000000
--- a/Documentation/dev-polygerrit.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-= PolyGerrit - GUI
-
-== Configuring
-
-By default both GWT and PolyGerrit UI are available to users.
-
-To make PolyGerrit the default UI but keep GWT as a secondary UI:
-----
-[gerrit]
- ui = POLYGERRIT
-----
-
-To disable GWT but not PolyGerrit:
-----
-[gerrit]
- enableGwtUi = false
- enablePolyGerrit = true
-----
-
-To enable GWT but not PolyGerrit:
-----
-[gerrit]
- enableGwtUi = true
- enablePolyGerrit = false
-----
-
-To switch to the PolyGerrit UI you have to add `?polygerrit=1` in the URL.
-
-for example https://gerrit.example.org/?polygerrit=1
-
-To disable PolyGerrit UI, change 1 to 0, which will take you back to GWT UI.
-
-
-More information can be found in the link:https://gerrit.googlesource.com/gerrit/+/master/polygerrit-ui/[README]
diff --git a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
index fec7137..6da19cd 100644
--- a/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
+++ b/java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
@@ -373,7 +373,7 @@
List<T> results = Collections.emptyList();
String uri = getURI(index, SEARCH);
Response response =
- performRequest(HttpPost.METHOD_NAME, search, uri, Collections.emptyMap());
+ performRequest(HttpPost.METHOD_NAME, uri, search, Collections.emptyMap());
StatusLine statusLine = response.getStatusLine();
if (statusLine.getStatusCode() == HttpStatus.SC_OK) {
String content = getContent(response);
diff --git a/java/com/google/gerrit/extensions/client/UiType.java b/java/com/google/gerrit/extensions/client/UiType.java
deleted file mode 100644
index 0d9df39..0000000
--- a/java/com/google/gerrit/extensions/client/UiType.java
+++ /dev/null
@@ -1,32 +0,0 @@
-// Copyright (C) 2016 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.extensions.client;
-
-public enum UiType {
- NONE,
- GWT,
- POLYGERRIT;
-
- public static UiType parse(String str) {
- if (str != null) {
- for (UiType type : UiType.values()) {
- if (type.name().equalsIgnoreCase(str)) {
- return type;
- }
- }
- }
- return null;
- }
-}
diff --git a/java/com/google/gerrit/extensions/common/GerritInfo.java b/java/com/google/gerrit/extensions/common/GerritInfo.java
index f904b06..e825f2e 100644
--- a/java/com/google/gerrit/extensions/common/GerritInfo.java
+++ b/java/com/google/gerrit/extensions/common/GerritInfo.java
@@ -14,9 +14,6 @@
package com.google.gerrit.extensions.common;
-import com.google.gerrit.extensions.client.UiType;
-import java.util.Set;
-
public class GerritInfo {
public String allProjects;
public String allUsers;
@@ -25,5 +22,4 @@
public Boolean editGpgKeys;
public String reportBugUrl;
public String reportBugText;
- public Set<UiType> webUis;
}
diff --git a/java/com/google/gerrit/pgm/Init.java b/java/com/google/gerrit/pgm/Init.java
index 1739de9..4ea31da 100644
--- a/java/com/google/gerrit/pgm/Init.java
+++ b/java/com/google/gerrit/pgm/Init.java
@@ -245,7 +245,7 @@
}
private void verifyInstallPluginList(ConsoleUI ui, List<PluginData> plugins) {
- if (nullOrEmpty(installPlugins) || nullOrEmpty(plugins)) {
+ if (nullOrEmpty(installPlugins)) {
return;
}
Set<String> missing = Sets.newHashSet(installPlugins);
diff --git a/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java b/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
index 9fd3f16..20e7ba2 100644
--- a/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
+++ b/java/com/google/gerrit/pgm/init/api/AllProjectsConfig.java
@@ -15,8 +15,10 @@
package com.google.gerrit.pgm.init.api;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.project.GroupList;
import com.google.gerrit.server.project.ProjectConfig;
@@ -27,16 +29,21 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.RepositoryCache;
+import org.eclipse.jgit.lib.StoredConfig;
public class AllProjectsConfig extends VersionedMetaDataOnInit {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+ @Nullable private final StoredConfig baseConfig;
private Config cfg;
private GroupList groupList;
@Inject
AllProjectsConfig(AllProjectsNameOnInitProvider allProjects, SitePaths site, InitFlags flags) {
super(flags, site, allProjects.get(), RefNames.REFS_CONFIG);
+ this.baseConfig =
+ ProjectConfig.Factory.getBaseConfig(
+ site, new AllProjectsName(allProjects.get()), new Project.NameKey(allProjects.get()));
}
public Config getConfig() {
@@ -55,8 +62,11 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
+ if (baseConfig != null) {
+ baseConfig.load();
+ }
groupList = readGroupList();
- cfg = readConfig(ProjectConfig.PROJECT_CONFIG);
+ cfg = readConfig(ProjectConfig.PROJECT_CONFIG, baseConfig);
}
private GroupList readGroupList() throws IOException {
diff --git a/java/com/google/gerrit/server/AccessPath.java b/java/com/google/gerrit/server/AccessPath.java
index cb720c8..4d07d62 100644
--- a/java/com/google/gerrit/server/AccessPath.java
+++ b/java/com/google/gerrit/server/AccessPath.java
@@ -22,9 +22,6 @@
/** Access through the REST API. */
REST_API,
- /** Access through the old JSON-RPC interface. */
- JSON_RPC,
-
/** Access by a web cookie. This path is not protected like REST_API. */
WEB_BROWSER,
diff --git a/java/com/google/gerrit/server/DynamicOptions.java b/java/com/google/gerrit/server/DynamicOptions.java
index 3759f09..dc5a262 100644
--- a/java/com/google/gerrit/server/DynamicOptions.java
+++ b/java/com/google/gerrit/server/DynamicOptions.java
@@ -50,8 +50,18 @@
* }
* </pre>
*
- * The option will be prefixed by the plugin name. In the example above, if the plugin name was
+ * <p>The option will be prefixed by the plugin name. In the example above, if the plugin name was
* my-plugin, then the --verbose option as used by the caller would be --my-plugin--verbose.
+ *
+ * <p>Additional options can be annotated with @RequiresOption which will cause them to be ignored
+ * unless the required option is present. For example:
+ *
+ * <pre>
+ * {@literal @}RequiresOptions("--help")
+ * {@literal @}Option(name = "--help-as-json",
+ * usage = "display help text in json format")
+ * public boolean displayHelpAsJson;
+ * </pre>
*/
public interface DynamicBean {}
@@ -261,6 +271,7 @@
for (Entry<String, DynamicBean> e : beansByPlugin.entrySet()) {
clp.parseWithPrefix("--" + e.getKey(), e.getValue());
}
+ clp.drainOptionQueue();
}
public void setDynamicBeans() {
diff --git a/java/com/google/gerrit/server/audit/RpcAuditEvent.java b/java/com/google/gerrit/server/audit/RpcAuditEvent.java
deleted file mode 100644
index 6c53bb2..0000000
--- a/java/com/google/gerrit/server/audit/RpcAuditEvent.java
+++ /dev/null
@@ -1,47 +0,0 @@
-// Copyright (C) 2013 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.audit;
-
-import com.google.common.collect.ListMultimap;
-import com.google.gerrit.server.CurrentUser;
-
-public class RpcAuditEvent extends HttpAuditEvent {
-
- /**
- * Creates a new audit event with results
- *
- * @param sessionId session id the event belongs to
- * @param who principal that has generated the event
- * @param what object of the event
- * @param when time-stamp of when the event started
- * @param params parameters of the event
- * @param httpMethod HTTP method
- * @param input input
- * @param status HTTP status
- * @param result result of the event
- */
- public RpcAuditEvent(
- String sessionId,
- CurrentUser who,
- String what,
- long when,
- ListMultimap<String, ?> params,
- String httpMethod,
- Object input,
- int status,
- Object result) {
- super(sessionId, who, what, when, params, httpMethod, input, status, result);
- }
-}
diff --git a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
index 196267e..b33aa3c 100644
--- a/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
+++ b/java/com/google/gerrit/server/git/meta/VersionedMetaData.java
@@ -462,7 +462,12 @@
}
protected Config readConfig(String fileName) throws IOException, ConfigInvalidException {
- Config rc = new Config();
+ return readConfig(fileName, null);
+ }
+
+ protected Config readConfig(String fileName, Config baseConfig)
+ throws IOException, ConfigInvalidException {
+ Config rc = new Config(baseConfig);
String text = readUTF8(fileName);
if (!text.isEmpty()) {
try {
diff --git a/java/com/google/gerrit/server/notedb/IntBlob.java b/java/com/google/gerrit/server/notedb/IntBlob.java
index 9bac2a4..f8c713c 100644
--- a/java/com/google/gerrit/server/notedb/IntBlob.java
+++ b/java/com/google/gerrit/server/notedb/IntBlob.java
@@ -18,6 +18,7 @@
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.primitives.Ints;
import com.google.gerrit.common.Nullable;
@@ -28,6 +29,7 @@
import java.io.IOException;
import java.util.Optional;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectLoader;
@@ -116,8 +118,9 @@
return result == RefUpdate.Result.NEW || result == RefUpdate.Result.FORCED;
}
- private static IntBlob create(ObjectId id, int value) {
- return new AutoValue_IntBlob(id, value);
+ @VisibleForTesting
+ static IntBlob create(AnyObjectId id, int value) {
+ return new AutoValue_IntBlob(id.copy(), value);
}
public abstract ObjectId id();
diff --git a/java/com/google/gerrit/server/permissions/RefControl.java b/java/com/google/gerrit/server/permissions/RefControl.java
index 74b04a3..83ea7f8 100644
--- a/java/com/google/gerrit/server/permissions/RefControl.java
+++ b/java/com/google/gerrit/server/permissions/RefControl.java
@@ -197,7 +197,6 @@
case GIT:
return false;
- case JSON_RPC:
case REST_API:
case SSH_COMMAND:
case UNKNOWN:
@@ -229,7 +228,6 @@
case GIT:
return canPushWithForce() || canPerform(Permission.DELETE);
- case JSON_RPC:
case REST_API:
case SSH_COMMAND:
case UNKNOWN:
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 74c0f3b..b16b076 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -52,13 +52,16 @@
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.ProjectWatches.NotifyType;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.ConfigUtil;
import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.BranchOrderSection;
import com.google.gerrit.server.git.NotifyConfig;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
+import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.ArrayList;
@@ -83,8 +86,11 @@
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
import org.eclipse.jgit.revwalk.RevWalk;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.transport.RefSpec;
+import org.eclipse.jgit.util.FS;
public class ProjectConfig extends VersionedMetaData implements ValidationError.Sink {
public static final String COMMENTLINK = "commentlink";
@@ -173,8 +179,28 @@
// ProjectCache, so this would retain lots more memory.
@Singleton
public static class Factory {
+ @Nullable
+ public static StoredConfig getBaseConfig(
+ SitePaths sitePaths, AllProjectsName allProjects, Project.NameKey projectName) {
+ return projectName.equals(allProjects)
+ // Delay loading till onLoad method.
+ ? new FileBasedConfig(
+ sitePaths.etc_dir.resolve(allProjects.get()).resolve(PROJECT_CONFIG).toFile(),
+ FS.DETECTED)
+ : null;
+ }
+
+ private final SitePaths sitePaths;
+ private final AllProjectsName allProjects;
+
+ @Inject
+ Factory(SitePaths sitePaths, AllProjectsName allProjects) {
+ this.sitePaths = sitePaths;
+ this.allProjects = allProjects;
+ }
+
public ProjectConfig create(Project.NameKey projectName) {
- return new ProjectConfig(projectName);
+ return new ProjectConfig(projectName, getBaseConfig(sitePaths, allProjects, projectName));
}
public ProjectConfig read(MetaDataUpdate update) throws IOException, ConfigInvalidException {
@@ -191,6 +217,8 @@
}
}
+ private final StoredConfig baseConfig;
+
private Project project;
private AccountsSection accountsSection;
private GroupList groupList;
@@ -253,8 +281,9 @@
commentLinkSections.add(commentLink);
}
- private ProjectConfig(Project.NameKey projectName) {
+ private ProjectConfig(Project.NameKey projectName, @Nullable StoredConfig baseConfig) {
this.projectName = projectName;
+ this.baseConfig = baseConfig;
}
public void load(Repository repo) throws IOException, ConfigInvalidException {
@@ -516,11 +545,14 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
+ if (baseConfig != null) {
+ baseConfig.load();
+ }
readGroupList();
groupsByName = mapGroupReferences();
rulesId = getObjectId("rules.pl");
- Config rc = readConfig(PROJECT_CONFIG);
+ Config rc = readConfig(PROJECT_CONFIG, baseConfig);
project = new Project(projectName);
Project p = project;
diff --git a/java/com/google/gerrit/server/project/ProjectCreator.java b/java/com/google/gerrit/server/project/ProjectCreator.java
new file mode 100644
index 0000000..b50b046
--- /dev/null
+++ b/java/com/google/gerrit/server/project/ProjectCreator.java
@@ -0,0 +1,255 @@
+// Copyright (C) 2018 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.project;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.GroupDescription;
+import com.google.gerrit.common.data.GroupReference;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.events.NewProjectCreatedListener;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.GerritPersonIdent;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.GroupBackend;
+import com.google.gerrit.server.config.RepositoryConfig;
+import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.RepositoryCaseMismatchException;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import java.io.IOException;
+import java.util.List;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.errors.RepositoryNotFoundException;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.lib.RefUpdate.Result;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.transport.ReceiveCommand;
+
+public class ProjectCreator {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final GitRepositoryManager repoManager;
+ private final PluginSetContext<NewProjectCreatedListener> createdListeners;
+ private final ProjectCache projectCache;
+ private final GroupBackend groupBackend;
+ private final MetaDataUpdate.User metaDataUpdateFactory;
+ private final GitReferenceUpdated referenceUpdated;
+ private final RepositoryConfig repositoryCfg;
+ private final Provider<PersonIdent> serverIdent;
+ private final Provider<IdentifiedUser> identifiedUser;
+ private final ProjectConfig.Factory projectConfigFactory;
+
+ @Inject
+ ProjectCreator(
+ GitRepositoryManager repoManager,
+ PluginSetContext<NewProjectCreatedListener> createdListeners,
+ ProjectCache projectCache,
+ GroupBackend groupBackend,
+ MetaDataUpdate.User metaDataUpdateFactory,
+ GitReferenceUpdated referenceUpdated,
+ RepositoryConfig repositoryCfg,
+ @GerritPersonIdent Provider<PersonIdent> serverIdent,
+ Provider<IdentifiedUser> identifiedUser,
+ ProjectConfig.Factory projectConfigFactory) {
+ this.repoManager = repoManager;
+ this.createdListeners = createdListeners;
+ this.projectCache = projectCache;
+ this.groupBackend = groupBackend;
+ this.metaDataUpdateFactory = metaDataUpdateFactory;
+ this.referenceUpdated = referenceUpdated;
+ this.repositoryCfg = repositoryCfg;
+ this.serverIdent = serverIdent;
+ this.identifiedUser = identifiedUser;
+ this.projectConfigFactory = projectConfigFactory;
+ }
+
+ public ProjectState createProject(CreateProjectArgs args)
+ throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
+ final Project.NameKey nameKey = args.getProject();
+ try {
+ final String head = args.permissionsOnly ? RefNames.REFS_CONFIG : args.branch.get(0);
+ try (Repository repo = repoManager.openRepository(nameKey)) {
+ if (repo.getObjectDatabase().exists()) {
+ throw new ResourceConflictException("project \"" + nameKey + "\" exists");
+ }
+ } catch (RepositoryNotFoundException e) {
+ // It does not exist, safe to ignore.
+ }
+ try (Repository repo = repoManager.createRepository(nameKey)) {
+ RefUpdate u = repo.updateRef(Constants.HEAD);
+ u.disableRefLog();
+ u.link(head);
+
+ createProjectConfig(args);
+
+ if (!args.permissionsOnly && args.createEmptyCommit) {
+ createEmptyCommits(repo, nameKey, args.branch);
+ }
+
+ fire(nameKey, head);
+
+ return projectCache.get(nameKey);
+ }
+ } catch (RepositoryCaseMismatchException e) {
+ throw new ResourceConflictException(
+ "Cannot create "
+ + nameKey.get()
+ + " because the name is already occupied by another project."
+ + " The other project has the same name, only spelled in a"
+ + " different case.");
+ } catch (RepositoryNotFoundException badName) {
+ throw new BadRequestException("invalid project name: " + nameKey);
+ } catch (ConfigInvalidException e) {
+ String msg = "Cannot create " + nameKey;
+ logger.atSevere().withCause(e).log(msg);
+ throw e;
+ }
+ }
+
+ private void createProjectConfig(CreateProjectArgs args)
+ throws IOException, ConfigInvalidException {
+ try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
+ ProjectConfig config = projectConfigFactory.read(md);
+
+ Project newProject = config.getProject();
+ newProject.setDescription(args.projectDescription);
+ newProject.setSubmitType(
+ MoreObjects.firstNonNull(
+ args.submitType, repositoryCfg.getDefaultSubmitType(args.getProject())));
+ newProject.setBooleanConfig(
+ BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, args.contributorAgreements);
+ newProject.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, args.signedOffBy);
+ newProject.setBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE, args.contentMerge);
+ newProject.setBooleanConfig(
+ BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
+ args.newChangeForAllNotInTarget);
+ newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
+ newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
+ newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
+ newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);
+ newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_SIGNED_PUSH, args.requireSignedPush);
+ if (args.newParent != null) {
+ newProject.setParentName(args.newParent);
+ }
+
+ if (!args.ownerIds.isEmpty()) {
+ AccessSection all = config.getAccessSection(AccessSection.ALL, true);
+ for (AccountGroup.UUID ownerId : args.ownerIds) {
+ GroupDescription.Basic g = groupBackend.get(ownerId);
+ if (g != null) {
+ GroupReference group = config.resolve(GroupReference.forGroup(g));
+ all.getPermission(Permission.OWNER, true).add(new PermissionRule(group));
+ }
+ }
+ }
+
+ md.setMessage("Created project\n");
+ config.commit(md);
+ md.getRepository().setGitwebDescription(args.projectDescription);
+ }
+ projectCache.onCreateProject(args.getProject());
+ }
+
+ private void createEmptyCommits(Repository repo, Project.NameKey project, List<String> refs)
+ throws IOException {
+ try (ObjectInserter oi = repo.newObjectInserter()) {
+ CommitBuilder cb = new CommitBuilder();
+ cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
+ cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent());
+ cb.setCommitter(serverIdent.get());
+ cb.setMessage("Initial empty repository\n");
+
+ ObjectId id = oi.insert(cb);
+ oi.flush();
+
+ for (String ref : refs) {
+ RefUpdate ru = repo.updateRef(ref);
+ ru.setNewObjectId(id);
+ Result result = ru.update();
+ switch (result) {
+ case NEW:
+ referenceUpdated.fire(
+ project, ru, ReceiveCommand.Type.CREATE, identifiedUser.get().state());
+ break;
+ case FAST_FORWARD:
+ case FORCED:
+ case IO_FAILURE:
+ case LOCK_FAILURE:
+ case NOT_ATTEMPTED:
+ case NO_CHANGE:
+ case REJECTED:
+ case REJECTED_CURRENT_BRANCH:
+ case RENAMED:
+ case REJECTED_MISSING_OBJECT:
+ case REJECTED_OTHER_REASON:
+ default:
+ {
+ throw new IOException(
+ String.format("Failed to create ref \"%s\": %s", ref, result.name()));
+ }
+ }
+ }
+ } catch (IOException e) {
+ logger.atSevere().withCause(e).log("Cannot create empty commit for %s", project.get());
+ throw e;
+ }
+ }
+
+ private void fire(Project.NameKey name, String head) {
+ if (createdListeners.isEmpty()) {
+ return;
+ }
+
+ ProjectCreator.Event event = new ProjectCreator.Event(name, head);
+ createdListeners.runEach(l -> l.onNewProjectCreated(event));
+ }
+
+ static class Event extends AbstractNoNotifyEvent implements NewProjectCreatedListener.Event {
+ private final Project.NameKey name;
+ private final String head;
+
+ Event(Project.NameKey name, String head) {
+ this.name = name;
+ this.head = head;
+ }
+
+ @Override
+ public String getProjectName() {
+ return name.get();
+ }
+
+ @Override
+ public String getHeadName() {
+ return head;
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
index 9a49ffe..ac78aef 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryProcessor.java
@@ -17,6 +17,8 @@
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.server.query.change.ChangeQueryBuilder.FIELD_LIMIT;
+import com.google.common.collect.HashMultimap;
+import com.google.common.collect.Multimap;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.index.IndexConfig;
@@ -68,6 +70,8 @@
private final Provider<CurrentUser> userProvider;
private final ChangeNotes.Factory notesFactory;
private final DynamicMap<ChangeAttributeFactory> attributeFactories;
+ private final Multimap<String, ChangeAttributeFactory> attributeFactoriesByPlugin =
+ HashMultimap.create();
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
private final Provider<AnonymousUser> anonymousUserProvider;
@@ -109,6 +113,7 @@
this.permissionBackend = permissionBackend;
this.projectCache = projectCache;
this.anonymousUserProvider = anonymousUserProvider;
+ setupAttributeFactories();
}
@Override
@@ -132,22 +137,29 @@
return dynamicBeans.get(plugin);
}
- @Override
- public List<PluginDefinedInfo> create(ChangeData cd) {
- List<PluginDefinedInfo> plugins = new ArrayList<>(attributeFactories.plugins().size());
+ public void setupAttributeFactories() {
for (String plugin : attributeFactories.plugins()) {
for (Provider<ChangeAttributeFactory> provider :
attributeFactories.byPlugin(plugin).values()) {
- PluginDefinedInfo pda = null;
- try {
- pda = provider.get().create(cd, this, plugin);
- } catch (RuntimeException e) {
- /* Eat runtime exceptions so that queries don't fail. */
- }
- if (pda != null) {
- pda.name = plugin;
- plugins.add(pda);
- }
+ attributeFactoriesByPlugin.put(plugin, provider.get());
+ }
+ }
+ }
+
+ @Override
+ public List<PluginDefinedInfo> create(ChangeData cd) {
+ List<PluginDefinedInfo> plugins = new ArrayList<>(attributeFactories.plugins().size());
+ for (Map.Entry<String, ChangeAttributeFactory> e : attributeFactoriesByPlugin.entries()) {
+ String plugin = e.getKey();
+ PluginDefinedInfo pda = null;
+ try {
+ pda = e.getValue().create(cd, this, plugin);
+ } catch (RuntimeException ex) {
+ /* Eat runtime exceptions so that queries don't fail. */
+ }
+ if (pda != null) {
+ pda.name = plugin;
+ plugins.add(pda);
}
}
if (plugins.isEmpty()) {
diff --git a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
index 963a1b4..98ef220 100644
--- a/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
+++ b/java/com/google/gerrit/server/restapi/config/GetServerInfo.java
@@ -20,7 +20,6 @@
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.ContributorAgreement;
-import com.google.gerrit.extensions.client.UiType;
import com.google.gerrit.extensions.common.AccountsInfo;
import com.google.gerrit.extensions.common.AuthInfo;
import com.google.gerrit.extensions.common.ChangeConfigInfo;
@@ -66,7 +65,6 @@
import java.nio.file.Files;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.EnumSet;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
@@ -304,7 +302,6 @@
info.docSearch = docSearcher.isAvailable();
info.editGpgKeys =
toBoolean(enableSignedPush && config.getBoolean("gerrit", null, "editGpgKeys", true));
- info.webUis = EnumSet.noneOf(UiType.class);
return info;
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index 60a24d8..3785784 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -19,21 +19,14 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.Lists;
-import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.ProjectUtil;
-import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability;
-import com.google.gerrit.common.data.GroupDescription;
-import com.google.gerrit.common.data.GroupReference;
-import com.google.gerrit.common.data.Permission;
-import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.projects.ConfigInput;
import com.google.gerrit.extensions.api.projects.ProjectInput;
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.extensions.common.ProjectInfo;
-import com.google.gerrit.extensions.events.NewProjectCreatedListener;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -41,29 +34,17 @@
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestCollectionCreateView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
-import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.client.BooleanProjectConfig;
-import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
-import com.google.gerrit.server.GerritPersonIdent;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.ProjectOwnerGroupsProvider;
-import com.google.gerrit.server.config.RepositoryConfig;
-import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
-import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.git.RepositoryCaseMismatchException;
-import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.group.GroupResolver;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.plugincontext.PluginItemContext;
import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.CreateProjectArgs;
-import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectConfig;
+import com.google.gerrit.server.project.ProjectCreator;
import com.google.gerrit.server.project.ProjectJson;
import com.google.gerrit.server.project.ProjectNameLockManager;
import com.google.gerrit.server.project.ProjectResource;
@@ -79,84 +60,47 @@
import java.util.List;
import java.util.concurrent.locks.Lock;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.errors.RepositoryNotFoundException;
-import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectInserter;
-import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.RefUpdate;
-import org.eclipse.jgit.lib.RefUpdate.Result;
import org.eclipse.jgit.lib.Repository;
-import org.eclipse.jgit.transport.ReceiveCommand;
@RequiresCapability(GlobalCapability.CREATE_PROJECT)
@Singleton
public class CreateProject
implements RestCollectionCreateView<TopLevelResource, ProjectResource, ProjectInput> {
- private static final FluentLogger logger = FluentLogger.forEnclosingClass();
-
private final Provider<ProjectsCollection> projectsCollection;
private final Provider<GroupResolver> groupResolver;
private final PluginSetContext<ProjectCreationValidationListener>
projectCreationValidationListeners;
private final ProjectJson json;
- private final GitRepositoryManager repoManager;
- private final PluginSetContext<NewProjectCreatedListener> createdListeners;
- private final ProjectCache projectCache;
- private final GroupBackend groupBackend;
private final ProjectOwnerGroupsProvider.Factory projectOwnerGroups;
- private final MetaDataUpdate.User metaDataUpdateFactory;
- private final GitReferenceUpdated referenceUpdated;
- private final RepositoryConfig repositoryCfg;
- private final Provider<PersonIdent> serverIdent;
- private final Provider<IdentifiedUser> identifiedUser;
private final Provider<PutConfig> putConfig;
private final AllProjectsName allProjects;
private final AllUsersName allUsers;
private final PluginItemContext<ProjectNameLockManager> lockManager;
- private final ProjectConfig.Factory projectConfigFactory;
+ private final ProjectCreator projectCreator;
@Inject
CreateProject(
+ ProjectCreator projectCreator,
Provider<ProjectsCollection> projectsCollection,
Provider<GroupResolver> groupResolver,
ProjectJson json,
PluginSetContext<ProjectCreationValidationListener> projectCreationValidationListeners,
- GitRepositoryManager repoManager,
- PluginSetContext<NewProjectCreatedListener> createdListeners,
- ProjectCache projectCache,
- GroupBackend groupBackend,
ProjectOwnerGroupsProvider.Factory projectOwnerGroups,
- MetaDataUpdate.User metaDataUpdateFactory,
- GitReferenceUpdated referenceUpdated,
- RepositoryConfig repositoryCfg,
- @GerritPersonIdent Provider<PersonIdent> serverIdent,
- Provider<IdentifiedUser> identifiedUser,
Provider<PutConfig> putConfig,
AllProjectsName allProjects,
AllUsersName allUsers,
- PluginItemContext<ProjectNameLockManager> lockManager,
- ProjectConfig.Factory projectConfigFactory) {
+ PluginItemContext<ProjectNameLockManager> lockManager) {
this.projectsCollection = projectsCollection;
+ this.projectCreator = projectCreator;
this.groupResolver = groupResolver;
this.projectCreationValidationListeners = projectCreationValidationListeners;
this.json = json;
- this.repoManager = repoManager;
- this.createdListeners = createdListeners;
- this.projectCache = projectCache;
- this.groupBackend = groupBackend;
this.projectOwnerGroups = projectOwnerGroups;
- this.metaDataUpdateFactory = metaDataUpdateFactory;
- this.referenceUpdated = referenceUpdated;
- this.repositoryCfg = repositoryCfg;
- this.serverIdent = serverIdent;
- this.identifiedUser = identifiedUser;
this.putConfig = putConfig;
this.allProjects = allProjects;
this.allUsers = allUsers;
this.lockManager = lockManager;
- this.projectConfigFactory = projectConfigFactory;
}
@Override
@@ -227,7 +171,7 @@
throw new ResourceConflictException(e.getMessage(), e);
}
- ProjectState projectState = createProject(args);
+ ProjectState projectState = projectCreator.createProject(args);
requireNonNull(
projectState,
() -> String.format("failed to create project %s", args.getProject().get()));
@@ -243,93 +187,6 @@
}
}
- private ProjectState createProject(CreateProjectArgs args)
- throws BadRequestException, ResourceConflictException, IOException, ConfigInvalidException {
- final Project.NameKey nameKey = args.getProject();
- try {
- final String head = args.permissionsOnly ? RefNames.REFS_CONFIG : args.branch.get(0);
- try (Repository repo = repoManager.openRepository(nameKey)) {
- if (repo.getObjectDatabase().exists()) {
- throw new ResourceConflictException("project \"" + nameKey + "\" exists");
- }
- } catch (RepositoryNotFoundException e) {
- // It does not exist, safe to ignore.
- }
- try (Repository repo = repoManager.createRepository(nameKey)) {
- RefUpdate u = repo.updateRef(Constants.HEAD);
- u.disableRefLog();
- u.link(head);
-
- createProjectConfig(args);
-
- if (!args.permissionsOnly && args.createEmptyCommit) {
- createEmptyCommits(repo, nameKey, args.branch);
- }
-
- fire(nameKey, head);
-
- return projectCache.get(nameKey);
- }
- } catch (RepositoryCaseMismatchException e) {
- throw new ResourceConflictException(
- "Cannot create "
- + nameKey.get()
- + " because the name is already occupied by another project."
- + " The other project has the same name, only spelled in a"
- + " different case.");
- } catch (RepositoryNotFoundException badName) {
- throw new BadRequestException("invalid project name: " + nameKey);
- } catch (ConfigInvalidException e) {
- String msg = "Cannot create " + nameKey;
- logger.atSevere().withCause(e).log(msg);
- throw e;
- }
- }
-
- private void createProjectConfig(CreateProjectArgs args)
- throws IOException, ConfigInvalidException {
- try (MetaDataUpdate md = metaDataUpdateFactory.create(args.getProject())) {
- ProjectConfig config = projectConfigFactory.read(md);
-
- Project newProject = config.getProject();
- newProject.setDescription(args.projectDescription);
- newProject.setSubmitType(
- MoreObjects.firstNonNull(
- args.submitType, repositoryCfg.getDefaultSubmitType(args.getProject())));
- newProject.setBooleanConfig(
- BooleanProjectConfig.USE_CONTRIBUTOR_AGREEMENTS, args.contributorAgreements);
- newProject.setBooleanConfig(BooleanProjectConfig.USE_SIGNED_OFF_BY, args.signedOffBy);
- newProject.setBooleanConfig(BooleanProjectConfig.USE_CONTENT_MERGE, args.contentMerge);
- newProject.setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- args.newChangeForAllNotInTarget);
- newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_CHANGE_ID, args.changeIdRequired);
- newProject.setBooleanConfig(BooleanProjectConfig.REJECT_EMPTY_COMMIT, args.rejectEmptyCommit);
- newProject.setMaxObjectSizeLimit(args.maxObjectSizeLimit);
- newProject.setBooleanConfig(BooleanProjectConfig.ENABLE_SIGNED_PUSH, args.enableSignedPush);
- newProject.setBooleanConfig(BooleanProjectConfig.REQUIRE_SIGNED_PUSH, args.requireSignedPush);
- if (args.newParent != null) {
- newProject.setParentName(args.newParent);
- }
-
- if (!args.ownerIds.isEmpty()) {
- AccessSection all = config.getAccessSection(AccessSection.ALL, true);
- for (AccountGroup.UUID ownerId : args.ownerIds) {
- GroupDescription.Basic g = groupBackend.get(ownerId);
- if (g != null) {
- GroupReference group = config.resolve(GroupReference.forGroup(g));
- all.getPermission(Permission.OWNER, true).add(new PermissionRule(group));
- }
- }
- }
-
- md.setMessage("Created project\n");
- config.commit(md);
- md.getRepository().setGitwebDescription(args.projectDescription);
- }
- projectCache.onCreateProject(args.getProject());
- }
-
private List<String> normalizeBranchNames(List<String> branches) throws BadRequestException {
if (branches == null || branches.isEmpty()) {
return Collections.singletonList(Constants.R_HEADS + Constants.MASTER);
@@ -350,78 +207,4 @@
}
return normalizedBranches;
}
-
- private void createEmptyCommits(Repository repo, Project.NameKey project, List<String> refs)
- throws IOException {
- try (ObjectInserter oi = repo.newObjectInserter()) {
- CommitBuilder cb = new CommitBuilder();
- cb.setTreeId(oi.insert(Constants.OBJ_TREE, new byte[] {}));
- cb.setAuthor(metaDataUpdateFactory.getUserPersonIdent());
- cb.setCommitter(serverIdent.get());
- cb.setMessage("Initial empty repository\n");
-
- ObjectId id = oi.insert(cb);
- oi.flush();
-
- for (String ref : refs) {
- RefUpdate ru = repo.updateRef(ref);
- ru.setNewObjectId(id);
- Result result = ru.update();
- switch (result) {
- case NEW:
- referenceUpdated.fire(
- project, ru, ReceiveCommand.Type.CREATE, identifiedUser.get().state());
- break;
- case FAST_FORWARD:
- case FORCED:
- case IO_FAILURE:
- case LOCK_FAILURE:
- case NOT_ATTEMPTED:
- case NO_CHANGE:
- case REJECTED:
- case REJECTED_CURRENT_BRANCH:
- case RENAMED:
- case REJECTED_MISSING_OBJECT:
- case REJECTED_OTHER_REASON:
- default:
- {
- throw new IOException(
- String.format("Failed to create ref \"%s\": %s", ref, result.name()));
- }
- }
- }
- } catch (IOException e) {
- logger.atSevere().withCause(e).log("Cannot create empty commit for %s", project.get());
- throw e;
- }
- }
-
- private void fire(Project.NameKey name, String head) {
- if (createdListeners.isEmpty()) {
- return;
- }
-
- Event event = new Event(name, head);
- createdListeners.runEach(l -> l.onNewProjectCreated(event));
- }
-
- static class Event extends AbstractNoNotifyEvent implements NewProjectCreatedListener.Event {
- private final Project.NameKey name;
- private final String head;
-
- Event(Project.NameKey name, String head) {
- this.name = name;
- this.head = head;
- }
-
- @Override
- public String getProjectName() {
- return name.get();
- }
-
- @Override
- public String getHeadName() {
- return head;
- }
- }
}
diff --git a/java/com/google/gerrit/server/schema/ProjectConfigSchemaUpdate.java b/java/com/google/gerrit/server/schema/ProjectConfigSchemaUpdate.java
index c25b846..483e363 100644
--- a/java/com/google/gerrit/server/schema/ProjectConfigSchemaUpdate.java
+++ b/java/com/google/gerrit/server/schema/ProjectConfigSchemaUpdate.java
@@ -17,12 +17,17 @@
import static com.google.gerrit.server.project.ProjectConfig.ACCESS;
import static java.util.stream.Collectors.toList;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.git.meta.VersionedMetaData;
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gwtorm.server.OrmException;
+import com.google.inject.Inject;
import java.io.IOException;
import java.util.Arrays;
import java.util.List;
@@ -31,22 +36,38 @@
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.lib.StoredConfig;
public class ProjectConfigSchemaUpdate extends VersionedMetaData {
+ public static class Factory {
+ private final SitePaths sitePaths;
+ private final AllProjectsName allProjectsName;
+
+ @Inject
+ Factory(SitePaths sitePaths, AllProjectsName allProjectsName) {
+ this.sitePaths = sitePaths;
+ this.allProjectsName = allProjectsName;
+ }
+
+ ProjectConfigSchemaUpdate read(MetaDataUpdate update)
+ throws IOException, ConfigInvalidException {
+ ProjectConfigSchemaUpdate r =
+ new ProjectConfigSchemaUpdate(
+ update,
+ ProjectConfig.Factory.getBaseConfig(sitePaths, allProjectsName, allProjectsName));
+ r.load(update);
+ return r;
+ }
+ }
private final MetaDataUpdate update;
+ @Nullable private final StoredConfig baseConfig;
private Config config;
private boolean updated;
- public static ProjectConfigSchemaUpdate read(MetaDataUpdate update)
- throws IOException, ConfigInvalidException {
- ProjectConfigSchemaUpdate r = new ProjectConfigSchemaUpdate(update);
- r.load(update);
- return r;
- }
-
- private ProjectConfigSchemaUpdate(MetaDataUpdate update) {
+ private ProjectConfigSchemaUpdate(MetaDataUpdate update, @Nullable StoredConfig baseConfig) {
this.update = update;
+ this.baseConfig = baseConfig;
}
@Override
@@ -56,7 +77,15 @@
@Override
protected void onLoad() throws IOException, ConfigInvalidException {
- config = readConfig(ProjectConfig.PROJECT_CONFIG);
+ if (baseConfig != null) {
+ baseConfig.load();
+ }
+ config = readConfig(ProjectConfig.PROJECT_CONFIG, baseConfig);
+ }
+
+ @VisibleForTesting
+ Config getConfig() {
+ return config;
}
public void removeForceFromPermission(String name) {
diff --git a/java/com/google/gerrit/server/schema/Schema_130.java b/java/com/google/gerrit/server/schema/Schema_130.java
index 0c9d79d..e550121 100644
--- a/java/com/google/gerrit/server/schema/Schema_130.java
+++ b/java/com/google/gerrit/server/schema/Schema_130.java
@@ -41,15 +41,18 @@
private final GitRepositoryManager repoManager;
private final PersonIdent serverUser;
+ private final ProjectConfigSchemaUpdate.Factory projectConfigSchemaUpdateFactory;
@Inject
Schema_130(
Provider<Schema_129> prior,
GitRepositoryManager repoManager,
- @GerritPersonIdent PersonIdent serverUser) {
+ @GerritPersonIdent PersonIdent serverUser,
+ ProjectConfigSchemaUpdate.Factory projectConfigSchemaUpdateFactory) {
super(prior);
this.repoManager = repoManager;
this.serverUser = serverUser;
+ this.projectConfigSchemaUpdateFactory = projectConfigSchemaUpdateFactory;
}
@Override
@@ -60,7 +63,7 @@
for (Project.NameKey projectName : repoList) {
try (Repository git = repoManager.openRepository(projectName);
MetaDataUpdate md = new MetaDataUpdate(GitReferenceUpdated.DISABLED, projectName, git)) {
- ProjectConfigSchemaUpdate cfg = ProjectConfigSchemaUpdate.read(md);
+ ProjectConfigSchemaUpdate cfg = projectConfigSchemaUpdateFactory.read(md);
cfg.removeForceFromPermission("pushTag");
if (cfg.isUpdated()) {
repoUpgraded.add(projectName);
diff --git a/java/com/google/gerrit/server/securestore/testing/BUILD b/java/com/google/gerrit/server/securestore/testing/BUILD
new file mode 100644
index 0000000..793f8ec
--- /dev/null
+++ b/java/com/google/gerrit/server/securestore/testing/BUILD
@@ -0,0 +1,11 @@
+package(default_testonly = 1)
+
+java_library(
+ name = "testing",
+ srcs = glob(["*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/server",
+ "//lib/jgit/org.eclipse.jgit:jgit",
+ ],
+)
diff --git a/java/com/google/gerrit/server/securestore/testing/InMemorySecureStore.java b/java/com/google/gerrit/server/securestore/testing/InMemorySecureStore.java
new file mode 100644
index 0000000..23894c1
--- /dev/null
+++ b/java/com/google/gerrit/server/securestore/testing/InMemorySecureStore.java
@@ -0,0 +1,59 @@
+// Copyright (C) 2018 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.securestore.testing;
+
+import com.google.gerrit.server.securestore.SecureStore;
+import java.util.List;
+import org.eclipse.jgit.lib.Config;
+
+public class InMemorySecureStore extends SecureStore {
+ private final Config cfg = new Config();
+
+ @Override
+ public String[] getList(String section, String subsection, String name) {
+ return cfg.getStringList(section, subsection, name);
+ }
+
+ @Override
+ public String[] getListForPlugin(
+ String pluginName, String section, String subsection, String name) {
+ throw new UnsupportedOperationException("not used by tests");
+ }
+
+ @Override
+ public void setList(String section, String subsection, String name, List<String> values) {
+ cfg.setStringList(section, subsection, name, values);
+ }
+
+ @Override
+ public void unset(String section, String subsection, String name) {
+ cfg.unset(section, subsection, name);
+ }
+
+ @Override
+ public Iterable<EntryKey> list() {
+ throw new UnsupportedOperationException("not used by tests");
+ }
+
+ @Override
+ public boolean isOutdated() {
+ throw new UnsupportedOperationException("not used by tests");
+ }
+
+ @Override
+ public void reload() {
+ throw new UnsupportedOperationException("not used by tests");
+ }
+}
diff --git a/java/com/google/gerrit/util/cli/CmdLineParser.java b/java/com/google/gerrit/util/cli/CmdLineParser.java
index 5b7ea3f..555abc3 100644
--- a/java/com/google/gerrit/util/cli/CmdLineParser.java
+++ b/java/com/google/gerrit/util/cli/CmdLineParser.java
@@ -50,8 +50,11 @@
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.ResourceBundle;
@@ -85,6 +88,89 @@
CmdLineParser create(Object bean);
}
+ /**
+ * This may be used by an option handler during parsing to "call" additional parameters simulating
+ * as if they had been passed from the command line originally.
+ *
+ * <p>To call additional parameters from within an option handler, instantiate this class with the
+ * parameters and then call callParameters() with the additional parameters to be parsed.
+ * OptionHandlers may optionally pass this class to other methods which may then both
+ * parse/consume more parameters and call additional parameters.
+ */
+ public static class Parameters implements org.kohsuke.args4j.spi.Parameters {
+ protected final String[] args;
+ protected MyParser parser;
+ protected int consumed = 0;
+
+ public Parameters(org.kohsuke.args4j.spi.Parameters args, MyParser parser)
+ throws CmdLineException {
+ this.args = new String[args.size()];
+ for (int i = 0; i < args.size(); i++) {
+ this.args[i] = args.getParameter(i);
+ }
+ this.parser = parser;
+ }
+
+ public Parameters(String[] args, MyParser parser) {
+ this.args = args;
+ this.parser = parser;
+ }
+
+ @Override
+ public String getParameter(int idx) throws CmdLineException {
+ return args[idx];
+ }
+
+ /**
+ * get and consume (consider parsed) a parameter
+ *
+ * @return the consumed parameter
+ */
+ public String consumeParameter() throws CmdLineException {
+ return getParameter(consumed++);
+ }
+
+ @Override
+ public int size() {
+ return args.length;
+ }
+
+ /**
+ * Add 'count' to the value of parsed parameters. May be called more than once.
+ *
+ * @param count How many parameters were just parsed.
+ */
+ public void consume(int count) {
+ consumed += count;
+ }
+
+ /**
+ * Reports handlers how many parameters were parsed
+ *
+ * @return the count of parsed parameters
+ */
+ public int getConsumed() {
+ return consumed;
+ }
+
+ /**
+ * Use during parsing to call additional parameters simulating as if they had been passed from
+ * the command line originally.
+ *
+ * @param args A variable amount of parameters to call immediately
+ * <p>The parameters will be parsed immediately, before the remaining parameter will be
+ * parsed.
+ * <p>Note: Since this is done outside of the arg4j parsing loop, it will not match exactly
+ * what would happen if they were actually passed from the command line, but it will be
+ * pretty close. If this were moved to args4j, the interface could be the same and it could
+ * match exactly the behavior as if passed from the command line originally.
+ */
+ public void callParameters(String... args) throws CmdLineException {
+ Parameters impl = new Parameters(Arrays.copyOfRange(args, 1, args.length), parser);
+ parser.findOptionByName(args[0]).parseArguments(impl);
+ }
+ }
+
private final OptionHandlers handlers;
private final MyParser parser;
@@ -270,6 +356,10 @@
parser.parseWithPrefix(prefix, bean);
}
+ public void drainOptionQueue() {
+ parser.addOptionsWithMetRequirements();
+ }
+
private String makeOption(String name) {
if (!name.startsWith("-")) {
if (name.length() == 1) {
@@ -404,18 +494,64 @@
}
}
- private class MyParser extends org.kohsuke.args4j.CmdLineParser {
+ public class MyParser extends org.kohsuke.args4j.CmdLineParser {
@SuppressWarnings("rawtypes")
private List<OptionHandler> optionsList;
+ private Map<String, QueuedOption> queuedOptionsByName = new LinkedHashMap<>();
private HelpOption help;
+ private class QueuedOption {
+ public final Option option;
+
+ @SuppressWarnings("rawtypes")
+ public final Setter setter;
+
+ public final String[] requiredOptions;
+
+ private QueuedOption(
+ Option option,
+ @SuppressWarnings("rawtypes") Setter setter,
+ RequiresOptions requiresOptions) {
+ this.option = option;
+ this.setter = setter;
+ this.requiredOptions = requiresOptions != null ? requiresOptions.value() : new String[0];
+ }
+ }
+
MyParser(Object bean) {
super(bean, ParserProperties.defaults().withAtSyntax(false));
parseAdditionalOptions(bean, new HashSet<>());
+ addOptionsWithMetRequirements();
ensureOptionsInitialized();
}
+ public int addOptionsWithMetRequirements() {
+ int count = 0;
+ for (Iterator<Map.Entry<String, QueuedOption>> it = queuedOptionsByName.entrySet().iterator();
+ it.hasNext(); ) {
+ QueuedOption queuedOption = it.next().getValue();
+ if (hasAllRequiredOptions(queuedOption)) {
+ addOption(queuedOption.setter, queuedOption.option);
+ it.remove();
+ count++;
+ }
+ }
+ if (count > 0) {
+ count += addOptionsWithMetRequirements();
+ }
+ return count;
+ }
+
+ private boolean hasAllRequiredOptions(QueuedOption queuedOption) {
+ for (String name : queuedOption.requiredOptions) {
+ if (findOptionByName(name) == null) {
+ return false;
+ }
+ }
+ return true;
+ }
+
// NOTE: Argument annotations on bean are ignored.
public void parseWithPrefix(String prefix, Object bean) {
parseWithPrefix(prefix, bean, new HashSet<>());
@@ -430,13 +566,19 @@
for (Method m : c.getDeclaredMethods()) {
Option o = m.getAnnotation(Option.class);
if (o != null) {
- addOption(new MethodSetter(this, bean, m), new PrefixedOption(prefix, o));
+ queueOption(
+ new PrefixedOption(prefix, o),
+ new MethodSetter(this, bean, m),
+ m.getAnnotation(RequiresOptions.class));
}
}
for (Field f : c.getDeclaredFields()) {
Option o = f.getAnnotation(Option.class);
if (o != null) {
- addOption(Setters.create(f, bean), new PrefixedOption(prefix, o));
+ queueOption(
+ new PrefixedOption(prefix, o),
+ Setters.create(f, bean),
+ f.getAnnotation(RequiresOptions.class));
}
if (f.isAnnotationPresent(Options.class)) {
try {
@@ -480,6 +622,41 @@
return add(super.createOptionHandler(option, setter));
}
+ /**
+ * Finds a registered {@code OptionHandler} by its name or its alias.
+ *
+ * @param name name
+ * @return the {@code OptionHandler} or {@code null}
+ * <p>Note: this is cut & pasted from the parent class in arg4j, it was private and it
+ * needed to be exposed.
+ */
+ @SuppressWarnings("rawtypes")
+ public OptionHandler findOptionByName(String name) {
+ for (OptionHandler h : optionsList) {
+ NamedOptionDef option = (NamedOptionDef) h.option;
+ if (name.equals(option.name())) {
+ return h;
+ }
+ for (String alias : option.aliases()) {
+ if (name.equals(alias)) {
+ return h;
+ }
+ }
+ }
+ return null;
+ }
+
+ private void queueOption(
+ Option option,
+ @SuppressWarnings("rawtypes") Setter setter,
+ RequiresOptions requiresOptions) {
+ if (queuedOptionsByName.put(option.name(), new QueuedOption(option, setter, requiresOptions))
+ != null) {
+ throw new IllegalAnnotationError(
+ "Option name " + option.name() + " is used more than once");
+ }
+ }
+
@SuppressWarnings("rawtypes")
private OptionHandler add(OptionHandler handler) {
ensureOptionsInitialized();
diff --git a/java/com/google/gerrit/util/cli/RequiresOptions.java b/java/com/google/gerrit/util/cli/RequiresOptions.java
new file mode 100644
index 0000000..de6ba44
--- /dev/null
+++ b/java/com/google/gerrit/util/cli/RequiresOptions.java
@@ -0,0 +1,45 @@
+// Copyright (C) 2017 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.util.cli;
+
+import static java.lang.annotation.ElementType.FIELD;
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.PARAMETER;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Marks a field/setter annotated with {@literal @}Option as having a dependency on multiple other
+ * command line option.
+ *
+ * <p>If any of the required command line options are not present, the {@literal @}Option will be
+ * ignored.
+ *
+ * <p>For example:
+ *
+ * <pre>
+ * {@literal @}RequiresOptions({"--help", "--usage"})
+ * {@literal @}Option(name = "--help-as-json",
+ * usage = "display help text in json format")
+ * public boolean displayHelpAsJson;
+ * </pre>
+ */
+@Retention(RUNTIME)
+@Target({FIELD, METHOD, PARAMETER})
+public @interface RequiresOptions {
+ String[] value();
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index ee22141..6f25d28 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -147,7 +147,7 @@
}
// Creates a group, but with uniquified name.
- protected String createGroup(String name) throws Exception {
+ protected String createUniqueGroup() throws Exception {
// TODO(hanwen): rewrite this test in terms of UUID. This requires redoing the assertion helpers
// too.
AccountGroup.UUID g = groupOperations.newGroup().ownerGroupUuid(adminGroupUuid()).create();
@@ -155,6 +155,8 @@
}
protected String createGroup(String name, String owner) throws Exception {
+ // TODO(hanwen): rewrite to use groupOperations. This requires passing the owner
+ // group's UUID rathen than its name.
name = name(name);
GroupInput in = new GroupInput();
in.name = name;
@@ -191,7 +193,7 @@
@Test
public void addRemoveMember() throws Exception {
- String g = createGroup("users");
+ String g = createUniqueGroup();
gApi.groups().id(g).addMembers("user");
assertMembers(g, user);
@@ -206,7 +208,7 @@
// Fill the cache for the observed account.
groupIncludeCache.getGroupsWithMember(accountId);
- String groupName = createGroup("users");
+ String groupName = createUniqueGroup();
AccountGroup.UUID groupUuid = new AccountGroup.UUID(gApi.groups().id(groupName).get().id);
gApi.groups().id(groupName).addMembers(username);
@@ -238,7 +240,7 @@
@Test
public void addMultipleMembers() throws Exception {
- String g = createGroup("users");
+ String g = createUniqueGroup();
String u1 = name("u1");
accountOperations.newAccount().username(u1).create();
@@ -255,7 +257,7 @@
@Test
public void membersWithAtSignInUsernameCanBeAdded() throws Exception {
- String g = createGroup("users");
+ String g = createUniqueGroup();
String usernameWithAt = name("u1@something");
accountOperations.newAccount().username(usernameWithAt).create();
@@ -269,7 +271,7 @@
@Test
public void membersWithAtSignInUsernameAreNotConfusedWithSimilarUsernames() throws Exception {
- String g = createGroup("users");
+ String g = createUniqueGroup();
String usernameWithAt = name("u1@something");
accountOperations.newAccount().username(usernameWithAt).create();
String usernameWithoutAt = name("u1something");
@@ -291,8 +293,8 @@
@Test
public void includeRemoveGroup() throws Exception {
- String p = createGroup("parent");
- String g = createGroup("newGroup");
+ String p = createUniqueGroup();
+ String g = createUniqueGroup();
gApi.groups().id(p).addGroups(g);
assertIncludes(p, g);
@@ -302,7 +304,7 @@
@Test
public void includeExternalGroup() throws Exception {
- String g = createGroup("group");
+ String g = createUniqueGroup();
String subgroupUuid = SystemGroupBackend.REGISTERED_USERS.get();
gApi.groups().id(g).addGroups(subgroupUuid);
@@ -319,8 +321,8 @@
@Test
public void includeExistingGroup_OK() throws Exception {
- String p = createGroup("parent");
- String g = createGroup("newGroup");
+ String p = createUniqueGroup();
+ String g = createUniqueGroup();
gApi.groups().id(p).addGroups(g);
assertIncludes(p, g);
gApi.groups().id(p).addGroups(g);
@@ -329,9 +331,9 @@
@Test
public void addMultipleIncludes() throws Exception {
- String p = createGroup("parent");
- String g1 = createGroup("newGroup1");
- String g2 = createGroup("newGroup2");
+ String p = createUniqueGroup();
+ String g1 = createUniqueGroup();
+ String g2 = createUniqueGroup();
List<String> groups = new ArrayList<>();
groups.add(g1);
groups.add(g2);
@@ -637,22 +639,22 @@
@Test
public void listEmptyGroupIncludes() throws Exception {
- String gx = createGroup("gx");
+ String gx = createUniqueGroup();
assertThat(gApi.groups().id(gx).includedGroups()).isEmpty();
}
@Test
public void includeNonExistingGroup() throws Exception {
- String gx = createGroup("gx");
+ String gx = createUniqueGroup();
exception.expect(UnprocessableEntityException.class);
gApi.groups().id(gx).addGroups("non-existing");
}
@Test
public void listNonEmptyGroupIncludes() throws Exception {
- String gx = createGroup("gx");
- String gy = createGroup("gy");
- String gz = createGroup("gz");
+ String gx = createUniqueGroup();
+ String gy = createUniqueGroup();
+ String gz = createUniqueGroup();
gApi.groups().id(gx).addGroups(gy);
gApi.groups().id(gx).addGroups(gz);
assertIncludes(gApi.groups().id(gx).includedGroups(), gy, gz);
@@ -660,8 +662,8 @@
@Test
public void listOneIncludeMember() throws Exception {
- String gx = createGroup("gx");
- String gy = createGroup("gy");
+ String gx = createUniqueGroup();
+ String gy = createUniqueGroup();
gApi.groups().id(gx).addGroups(gy);
assertIncludes(gApi.groups().id(gx).includedGroups(), gy);
}
@@ -674,13 +676,13 @@
@Test
public void listEmptyGroupMembers() throws Exception {
- String group = createGroup("empty");
+ String group = createUniqueGroup();
assertThat(gApi.groups().id(group).members()).isEmpty();
}
@Test
public void listNonEmptyGroupMembers() throws Exception {
- String group = createGroup("group");
+ String group = createUniqueGroup();
String user1 = name("user1");
accountOperations.newAccount().username(user1).create();
String user2 = name("user2");
@@ -692,7 +694,7 @@
@Test
public void listOneGroupMember() throws Exception {
- String group = createGroup("group");
+ String group = createUniqueGroup();
String user = name("user1");
accountOperations.newAccount().username(user).create();
gApi.groups().id(group).addMembers(user);
@@ -702,17 +704,17 @@
@Test
public void listGroupMembersRecursively() throws Exception {
- String gx = createGroup("gx");
+ String gx = createUniqueGroup();
String ux = name("ux");
accountOperations.newAccount().username(ux).create();
gApi.groups().id(gx).addMembers(ux);
- String gy = createGroup("gy");
+ String gy = createUniqueGroup();
String uy = name("uy");
accountOperations.newAccount().username(uy).create();
gApi.groups().id(gy).addMembers(uy);
- String gz = createGroup("gz");
+ String gz = createUniqueGroup();
String uz = name("uz");
accountOperations.newAccount().username(uz).create();
gApi.groups().id(gz).addMembers(uz);
@@ -725,7 +727,7 @@
@Test
public void usersSeeTheirDirectMembershipWhenListingMembersRecursively() throws Exception {
- String group = createGroup("group");
+ String group = createUniqueGroup();
gApi.groups().id(group).addMembers(user.username);
setApiUser(user);
@@ -734,8 +736,8 @@
@Test
public void usersDoNotSeeTheirIndirectMembershipWhenListingMembersRecursively() throws Exception {
- String group1 = createGroup("group1");
- String group2 = createGroup("group2");
+ String group1 = createUniqueGroup();
+ String group2 = createUniqueGroup();
gApi.groups().id(group1).addGroups(group2);
gApi.groups().id(group2).addMembers(user.username);
@@ -791,7 +793,7 @@
@Test
public void getGroupsByOwner() throws Exception {
- String parent = createGroup("test-parent");
+ String parent = createUniqueGroup();
List<String> children =
Arrays.asList(createGroup("test-child1", parent), createGroup("test-child2", parent));
diff --git a/javatests/com/google/gerrit/pgm/BUILD b/javatests/com/google/gerrit/pgm/BUILD
index e4afae2..8e3b71d 100644
--- a/javatests/com/google/gerrit/pgm/BUILD
+++ b/javatests/com/google/gerrit/pgm/BUILD
@@ -11,6 +11,8 @@
"//java/com/google/gerrit/pgm/init",
"//java/com/google/gerrit/pgm/init/api",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/securestore/testing",
+ "//java/com/google/gerrit/testing:gerrit-test-util",
"//lib:guava",
"//lib:junit",
"//lib/easymock",
diff --git a/javatests/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java b/javatests/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
index 7721fca..4d3d6df 100644
--- a/javatests/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
+++ b/javatests/com/google/gerrit/pgm/init/UpgradeFrom2_0_xTest.java
@@ -31,15 +31,13 @@
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.pgm.init.api.Section;
import com.google.gerrit.server.config.SitePaths;
-import com.google.gerrit.server.securestore.SecureStore;
+import com.google.gerrit.server.securestore.testing.InMemorySecureStore;
import java.io.IOException;
import java.io.InputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Collections;
-import java.util.List;
import org.eclipse.jgit.errors.ConfigInvalidException;
-import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.storage.file.FileBasedConfig;
import org.eclipse.jgit.util.FS;
import org.junit.Test;
@@ -110,44 +108,4 @@
u.run();
}
-
- private static class InMemorySecureStore extends SecureStore {
- private final Config cfg = new Config();
-
- @Override
- public String[] getList(String section, String subsection, String name) {
- return cfg.getStringList(section, subsection, name);
- }
-
- @Override
- public String[] getListForPlugin(
- String pluginName, String section, String subsection, String name) {
- throw new UnsupportedOperationException("not used by tests");
- }
-
- @Override
- public void setList(String section, String subsection, String name, List<String> values) {
- cfg.setStringList(section, subsection, name, values);
- }
-
- @Override
- public void unset(String section, String subsection, String name) {
- cfg.unset(section, subsection, name);
- }
-
- @Override
- public Iterable<EntryKey> list() {
- throw new UnsupportedOperationException("not used by tests");
- }
-
- @Override
- public boolean isOutdated() {
- throw new UnsupportedOperationException("not used by tests");
- }
-
- @Override
- public void reload() {
- throw new UnsupportedOperationException("not used by tests");
- }
- }
}
diff --git a/javatests/com/google/gerrit/pgm/init/api/AllProjectsConfigTest.java b/javatests/com/google/gerrit/pgm/init/api/AllProjectsConfigTest.java
new file mode 100644
index 0000000..68b4a8e
--- /dev/null
+++ b/javatests/com/google/gerrit/pgm/init/api/AllProjectsConfigTest.java
@@ -0,0 +1,117 @@
+// Copyright (C) 2018 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.pgm.init.api;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.easymock.EasyMock.createStrictMock;
+import static org.easymock.EasyMock.replay;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.securestore.testing.InMemorySecureStore;
+import com.google.gerrit.testing.TempFileUtil;
+import java.io.File;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class AllProjectsConfigTest {
+ private static final String ALL_PROJECTS = "All-The-Projects";
+
+ private SitePaths sitePaths;
+ private AllProjectsConfig allProjectsConfig;
+ private File allProjectsRepoFile;
+
+ @Before
+ public void setUp() throws Exception {
+ sitePaths = new SitePaths(TempFileUtil.createTempDirectory().toPath());
+ Files.createDirectories(sitePaths.etc_dir);
+
+ Path gitPath = sitePaths.resolve("git");
+
+ StoredConfig gerritConfig =
+ new FileBasedConfig(
+ sitePaths.resolve("etc").resolve("gerrit.config").toFile(), FS.DETECTED);
+ gerritConfig.load();
+ gerritConfig.setString("gerrit", null, "basePath", gitPath.toAbsolutePath().toString());
+ gerritConfig.setString("gerrit", null, "allProjects", ALL_PROJECTS);
+ gerritConfig.save();
+
+ Files.createDirectories(sitePaths.resolve("git"));
+ allProjectsRepoFile = gitPath.resolve("All-The-Projects.git").toFile();
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ repo.create(true);
+ }
+
+ InMemorySecureStore secureStore = new InMemorySecureStore();
+ InitFlags flags = new InitFlags(sitePaths, secureStore, ImmutableList.of(), false);
+ ConsoleUI ui = createStrictMock(ConsoleUI.class);
+ replay(ui);
+ Section.Factory sections =
+ (name, subsection) -> new Section(flags, sitePaths, secureStore, ui, name, subsection);
+ allProjectsConfig =
+ new AllProjectsConfig(new AllProjectsNameOnInitProvider(sections), sitePaths, flags);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ TempFileUtil.cleanup();
+ }
+
+ @Test
+ public void noBaseConfig() throws Exception {
+ assertThat(getConfig().getString("foo", null, "bar")).isNull();
+
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ TestRepository<?> tr = new TestRepository<>(repo);
+ tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
+ }
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("baz");
+ }
+
+ @Test
+ public void baseConfig() throws Exception {
+ assertThat(getConfig().getString("foo", null, "bar")).isNull();
+
+ Path baseConfigPath = sitePaths.etc_dir.resolve(ALL_PROJECTS).resolve("project.config");
+ Files.createDirectories(baseConfigPath.getParent());
+ Files.write(baseConfigPath, ImmutableList.of("[foo]", "bar = base"));
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base");
+
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ TestRepository<?> tr = new TestRepository<>(repo);
+ tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
+ }
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("baz");
+ }
+
+ private Config getConfig() throws IOException, ConfigInvalidException {
+ return allProjectsConfig.load().getConfig();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
index 982bd14..842a5b0 100644
--- a/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
+++ b/javatests/com/google/gerrit/server/change/LabelNormalizerTest.java
@@ -24,14 +24,14 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.lifecycle.LifecycleManager;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.LabelId;
-import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
-import com.google.gerrit.reviewdb.client.PatchSetInfo;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
@@ -75,6 +75,7 @@
@Inject protected ThreadLocalRequestContext requestContext;
@Inject private ChangeNotes.Factory changeNotesFactory;
@Inject private ProjectConfig.Factory projectConfigFactory;
+ @Inject private GerritApi gApi;
private LifecycleManager lifecycle;
private ReviewDb db;
@@ -127,18 +128,14 @@
}
private void setUpChange() throws Exception {
- change =
- new Change(
- new Change.Key("Iabcd1234abcd1234abcd1234abcd1234abcd1234"),
- new Change.Id(1),
- userId,
- new Branch.NameKey(allProjects, "refs/heads/master"),
- TimeUtil.nowTs());
- PatchSetInfo ps = new PatchSetInfo(new PatchSet.Id(change.getId(), 1));
- ps.setSubject("Test change");
- change.setCurrentPatchSet(ps);
- db.changes().insert(ImmutableList.of(change));
- notes = changeNotesFactory.createChecked(db, change);
+ ChangeInput input = new ChangeInput();
+ input.project = allProjects.get();
+ input.branch = "master";
+ input.newBranch = true;
+ input.subject = "Test change";
+ ChangeInfo info = gApi.changes().create(input).get();
+ notes = changeNotesFactory.createChecked(db, allProjects, new Change.Id(info._number));
+ change = notes.getChange();
}
@After
diff --git a/javatests/com/google/gerrit/server/notedb/IntBlobTest.java b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
new file mode 100644
index 0000000..1abaa22
--- /dev/null
+++ b/javatests/com/google/gerrit/server/notedb/IntBlobTest.java
@@ -0,0 +1,202 @@
+// Copyright (C) 2018 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.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+import static com.google.gerrit.truth.OptionalSubject.assertThat;
+
+import com.google.gerrit.git.LockFailureException;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gwtorm.server.OrmException;
+import java.io.IOException;
+import org.eclipse.jgit.errors.IncorrectObjectTypeException;
+import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.RefUpdate;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.Before;
+import org.junit.Test;
+
+public class IntBlobTest {
+ // Note: Can't easily test GitRefUpdated behavior, since binding GitRefUpdated requires a thick
+ // stack of dependencies, and it's not just a simple interface or abstract class.
+
+ private Project.NameKey projectName;
+ private InMemoryRepository repo;
+ private TestRepository<InMemoryRepository> tr;
+ private RevWalk rw;
+
+ @Before
+ public void setUp() throws Exception {
+ projectName = new Project.NameKey("repo");
+ repo = new InMemoryRepository(new DfsRepositoryDescription(projectName.get()));
+ tr = new TestRepository<>(repo);
+ rw = tr.getRevWalk();
+ }
+
+ @Test
+ public void parseNoRef() throws Exception {
+ assertThat(IntBlob.parse(repo, "refs/nothing")).isEmpty();
+ }
+
+ @Test
+ public void parseNonBlob() throws Exception {
+ String refName = "refs/foo/master";
+ tr.branch(refName).commit().create();
+ try {
+ IntBlob.parse(repo, refName);
+ assert_().fail("Expected IncorrectObjectTypeException");
+ } catch (IncorrectObjectTypeException e) {
+ // Expected.
+ }
+ }
+
+ @Test
+ public void parseValid() throws Exception {
+ String refName = "refs/foo";
+ ObjectId id = tr.update(refName, tr.blob("123"));
+ assertThat(IntBlob.parse(repo, refName)).value().isEqualTo(IntBlob.create(id, 123));
+ }
+
+ @Test
+ public void parseWithWhitespace() throws Exception {
+ String refName = "refs/foo";
+ ObjectId id = tr.update(refName, tr.blob(" 123 "));
+ assertThat(IntBlob.parse(repo, refName)).value().isEqualTo(IntBlob.create(id, 123));
+ }
+
+ @Test
+ public void parseInvalid() throws Exception {
+ String refName = "refs/foo";
+ ObjectId id = tr.update(refName, tr.blob("1 2 3"));
+ try {
+ IntBlob.parse(repo, refName);
+ assert_().fail("Expected OrmException");
+ } catch (OrmException e) {
+ assertThat(e).hasMessageThat().isEqualTo("invalid value in refs/foo blob at " + id.name());
+ }
+ }
+
+ @Test
+ public void tryStoreNoOldId() throws Exception {
+ String refName = "refs/foo";
+ RefUpdate ru =
+ IntBlob.tryStore(repo, rw, projectName, refName, null, 123, GitReferenceUpdated.DISABLED);
+ assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.NEW);
+ assertThat(ru.getName()).isEqualTo(refName);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(ru.getNewObjectId(), 123));
+ }
+
+ @Test
+ public void tryStoreOldIdZero() throws Exception {
+ String refName = "refs/foo";
+ RefUpdate ru =
+ IntBlob.tryStore(
+ repo, rw, projectName, refName, ObjectId.zeroId(), 123, GitReferenceUpdated.DISABLED);
+ assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.NEW);
+ assertThat(ru.getName()).isEqualTo(refName);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(ru.getNewObjectId(), 123));
+ }
+
+ @Test
+ public void tryStoreCorrectOldId() throws Exception {
+ String refName = "refs/foo";
+ ObjectId id = tr.update(refName, tr.blob("123"));
+ RefUpdate ru =
+ IntBlob.tryStore(repo, rw, projectName, refName, id, 456, GitReferenceUpdated.DISABLED);
+ assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.FORCED);
+ assertThat(ru.getName()).isEqualTo(refName);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(ru.getNewObjectId(), 456));
+ }
+
+ @Test
+ public void tryStoreWrongOldId() throws Exception {
+ String refName = "refs/foo";
+ RefUpdate ru =
+ IntBlob.tryStore(
+ repo,
+ rw,
+ projectName,
+ refName,
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+ 123,
+ GitReferenceUpdated.DISABLED);
+ assertThat(ru.getResult()).isEqualTo(RefUpdate.Result.LOCK_FAILURE);
+ assertThat(ru.getName()).isEqualTo(refName);
+ assertThat(IntBlob.parse(repo, refName)).isEmpty();
+ }
+
+ @Test
+ public void storeNoOldId() throws Exception {
+ String refName = "refs/foo";
+ IntBlob.store(repo, rw, projectName, refName, null, 123, GitReferenceUpdated.DISABLED);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(getRef(refName), 123));
+ }
+
+ @Test
+ public void storeOldIdZero() throws Exception {
+ String refName = "refs/foo";
+ IntBlob.store(
+ repo, rw, projectName, refName, ObjectId.zeroId(), 123, GitReferenceUpdated.DISABLED);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(getRef(refName), 123));
+ }
+
+ @Test
+ public void storeCorrectOldId() throws Exception {
+ String refName = "refs/foo";
+ ObjectId id = tr.update(refName, tr.blob("123"));
+ IntBlob.store(repo, rw, projectName, refName, id, 456, GitReferenceUpdated.DISABLED);
+ assertThat(IntBlob.parse(repo, refName))
+ .value()
+ .isEqualTo(IntBlob.create(getRef(refName), 456));
+ }
+
+ @Test
+ public void storeWrongOldId() throws Exception {
+ String refName = "refs/foo";
+ try {
+ IntBlob.store(
+ repo,
+ rw,
+ projectName,
+ refName,
+ ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"),
+ 123,
+ GitReferenceUpdated.DISABLED);
+ assert_().fail("expected LockFailureException");
+ } catch (LockFailureException e) {
+ assertThat(e.getFailedRefs()).containsExactly("refs/foo");
+ }
+ assertThat(IntBlob.parse(repo, refName)).isEmpty();
+ }
+
+ private ObjectId getRef(String refName) throws IOException {
+ return repo.exactRef(refName).getObjectId();
+ }
+}
diff --git a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
index 764d49a..a1e0566 100644
--- a/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
+++ b/javatests/com/google/gerrit/server/project/ProjectConfigTest.java
@@ -16,7 +16,9 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.gerrit.reviewdb.client.BooleanProjectConfig.REQUIRE_CHANGE_ID;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.ContributorAgreement;
@@ -24,16 +26,22 @@
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.SitePaths;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.git.meta.MetaDataUpdate;
import com.google.gerrit.server.project.testing.Util;
import com.google.gerrit.testing.GerritBaseTests;
+import com.google.gerrit.testing.TempFileUtil;
import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.Map;
@@ -50,6 +58,7 @@
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.util.RawParseUtils;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -74,21 +83,31 @@
+ !LabelType.DEF_COPY_ALL_SCORES_IF_NO_CHANGE
+ "\n";
+ private static final AllProjectsName ALL_PROJECTS = new AllProjectsName("All-The-Projects");
+
private final GroupReference developers =
new GroupReference(new AccountGroup.UUID("X"), "Developers");
private final GroupReference staff = new GroupReference(new AccountGroup.UUID("Y"), "Staff");
+ private SitePaths sitePaths;
private ProjectConfig.Factory factory;
private Repository db;
private TestRepository<?> tr;
@Before
public void setUp() throws Exception {
- factory = new ProjectConfig.Factory();
+ sitePaths = new SitePaths(TempFileUtil.createTempDirectory().toPath());
+ Files.createDirectories(sitePaths.etc_dir);
+ factory = new ProjectConfig.Factory(sitePaths, ALL_PROJECTS);
db = new InMemoryRepository(new DfsRepositoryDescription("repo"));
tr = new TestRepository<>(db);
}
+ @After
+ public void tearDown() throws Exception {
+ TempFileUtil.cleanup();
+ }
+
@Test
public void readConfig() throws Exception {
RevCommit rev =
@@ -593,6 +612,44 @@
+ "commentlink.bugzilla must have either link or html"));
}
+ @Test
+ public void readAllProjectsBaseConfigFromSitePaths() throws Exception {
+ ProjectConfig cfg = factory.create(ALL_PROJECTS);
+ cfg.load(db);
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
+ .isEqualTo(InheritableBoolean.INHERIT);
+
+ writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false");
+
+ cfg.load(db);
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
+ .isEqualTo(InheritableBoolean.FALSE);
+ }
+
+ @Test
+ public void readOtherProjectIgnoresAllProjectsBaseConfig() throws Exception {
+ ProjectConfig cfg = factory.create(new Project.NameKey("test"));
+ cfg.load(db);
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
+ .isEqualTo(InheritableBoolean.INHERIT);
+
+ writeDefaultAllProjectsConfig("[receive]", "requireChangeId = false");
+
+ cfg.load(db);
+ // If we went through ProjectState, then this would return FALSE, since project.config for
+ // All-Projects would inherit from all_projects.config, and this project would inherit from
+ // All-Projects. But in ProjectConfig itself, there is no inheritance from All-Projects, so this
+ // continues to return the default.
+ assertThat(cfg.getProject().getBooleanConfig(REQUIRE_CHANGE_ID))
+ .isEqualTo(InheritableBoolean.INHERIT);
+ }
+
+ private Path writeDefaultAllProjectsConfig(String... lines) throws IOException {
+ Path dir = sitePaths.etc_dir.resolve(ALL_PROJECTS.get());
+ Files.createDirectories(dir);
+ return Files.write(dir.resolve("project.config"), ImmutableList.copyOf(lines));
+ }
+
private ProjectConfig read(RevCommit rev) throws IOException, ConfigInvalidException {
ProjectConfig cfg = factory.create(new Project.NameKey("test"));
cfg.load(db, rev);
diff --git a/javatests/com/google/gerrit/server/schema/ProjectConfigSchemaUpdateTest.java b/javatests/com/google/gerrit/server/schema/ProjectConfigSchemaUpdateTest.java
new file mode 100644
index 0000000..de825e6
--- /dev/null
+++ b/javatests/com/google/gerrit/server/schema/ProjectConfigSchemaUpdateTest.java
@@ -0,0 +1,113 @@
+// Copyright (C) 2018 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.schema;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
+import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.testing.TempFileUtil;
+import java.io.File;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import org.eclipse.jgit.internal.storage.file.FileRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.lib.StoredConfig;
+import org.eclipse.jgit.storage.file.FileBasedConfig;
+import org.eclipse.jgit.util.FS;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ProjectConfigSchemaUpdateTest {
+ private static final String ALL_PROJECTS = "All-The-Projects";
+
+ private SitePaths sitePaths;
+ private ProjectConfigSchemaUpdate.Factory factory;
+ private File allProjectsRepoFile;
+
+ @Before
+ public void setUp() throws Exception {
+ sitePaths = new SitePaths(TempFileUtil.createTempDirectory().toPath());
+ Files.createDirectories(sitePaths.etc_dir);
+
+ Path gitPath = sitePaths.resolve("git");
+
+ StoredConfig gerritConfig = new FileBasedConfig(sitePaths.gerrit_config.toFile(), FS.DETECTED);
+ gerritConfig.load();
+ gerritConfig.setString("gerrit", null, "basePath", gitPath.toAbsolutePath().toString());
+ gerritConfig.setString("gerrit", null, "allProjects", ALL_PROJECTS);
+ gerritConfig.save();
+
+ Files.createDirectories(sitePaths.resolve("git"));
+ allProjectsRepoFile = gitPath.resolve("All-The-Projects.git").toFile();
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ repo.create(true);
+ }
+
+ factory = new ProjectConfigSchemaUpdate.Factory(sitePaths, new AllProjectsName(ALL_PROJECTS));
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ TempFileUtil.cleanup();
+ }
+
+ @Test
+ public void noBaseConfig() throws Exception {
+ assertThat(getConfig().getString("foo", null, "bar")).isNull();
+
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ TestRepository<?> tr = new TestRepository<>(repo);
+ tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
+ }
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("baz");
+ }
+
+ @Test
+ public void baseConfig() throws Exception {
+ assertThat(getConfig().getString("foo", null, "bar")).isNull();
+
+ Path baseConfigPath = sitePaths.etc_dir.resolve(ALL_PROJECTS).resolve("project.config");
+ Files.createDirectories(baseConfigPath.getParent());
+ Files.write(baseConfigPath, ImmutableList.of("[foo]", "bar = base"));
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("base");
+
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ TestRepository<?> tr = new TestRepository<>(repo);
+ tr.branch("refs/meta/config").commit().add("project.config", "[foo]\nbar = baz").create();
+ }
+
+ assertThat(getConfig().getString("foo", null, "bar")).isEqualTo("baz");
+ }
+
+ private Config getConfig() throws Exception {
+ try (Repository repo = new FileRepository(allProjectsRepoFile)) {
+ return factory
+ .read(
+ new MetaDataUpdate(
+ GitReferenceUpdated.DISABLED, new Project.NameKey(ALL_PROJECTS), repo, null))
+ .getConfig();
+ }
+ }
+}
diff --git a/plugins/BUILD b/plugins/BUILD
index 1ce0fba7..0feac10 100644
--- a/plugins/BUILD
+++ b/plugins/BUILD
@@ -42,6 +42,7 @@
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/schema",
"//java/com/google/gerrit/server/util/time",
+ "//java/com/google/gerrit/util/cli",
"//java/com/google/gerrit/util/http",
"//lib/commons:compress",
"//lib/commons:dbcp",
diff --git a/plugins/download-commands b/plugins/download-commands
index cf58d79..edd7156 160000
--- a/plugins/download-commands
+++ b/plugins/download-commands
@@ -1 +1 @@
-Subproject commit cf58d79bc034e8904aa459d8974df5796a734e1d
+Subproject commit edd715618415d9a5e03b4555d9a2d3cca8fff6e8
diff --git a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
index af982cf..4266b22 100644
--- a/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
+++ b/polygerrit-ui/app/behaviors/keyboard-shortcut-behavior/keyboard-shortcut-behavior.html
@@ -91,7 +91,7 @@
element. An example of this is in comment threads. A diff view supports actions
on comment threads, but there may be zero or many comment threads attached at
any given point. So the shortcut is declared as doc-only by the diff view and
-by gr-app, and actually implemented by gr-diff-comment-thread.
+by gr-app, and actually implemented by gr-comment-thread.
NOTE: doc-only shortcuts will not be customizable in the same way that other
shortcuts are.
diff --git a/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog.html b/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog.html
index bf17b911..e6d123c 100644
--- a/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog.html
+++ b/polygerrit-ui/app/elements/change-list/gr-create-commands-dialog/gr-create-commands-dialog.html
@@ -74,8 +74,8 @@
<li>
<p>
Close this dialog and you should be able to see your recently
- created change in ``Outgoing changes'' section on
- ``Your changes'' page.
+ created change in the 'Outgoing changes' section on the
+ 'Your changes' page.
</p>
</li>
</ol>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index 42c9e88..67fdff1 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -219,7 +219,7 @@
[this.Shortcut.TOGGLE_FILE_REVIEWED]: '_handleToggleFileReviewed',
[this.Shortcut.TOGGLE_LEFT_PANE]: '_handleToggleLeftPane',
- // Final two are actually handled by gr-diff-comment-thread.
+ // Final two are actually handled by gr-comment-thread.
[this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
[this.Shortcut.COLLAPSE_ALL_COMMENT_THREADS]: null,
};
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
index 2201a9a..4d8e5ae 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
@@ -18,7 +18,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../bower_components/paper-toggle-button/paper-toggle-button.html">
<link rel="import" href="../../../styles/shared-styles.html">
-<link rel="import" href="../../diff/gr-diff-comment-thread/gr-diff-comment-thread.html">
+<link rel="import" href="../../shared/gr-comment-thread/gr-comment-thread.html">
<dom-module id="gr-thread-list">
<template>
@@ -28,7 +28,7 @@
min-height: 20rem;
padding: 1rem;
}
- gr-diff-comment-thread {
+ gr-comment-thread {
display: block;
margin-bottom: .5rem;
max-width: 80ch;
@@ -54,9 +54,9 @@
display: flex;
margin-right: 1rem;
}
- .draftsOnly:not(.unresolvedOnly) gr-diff-comment-thread[has-draft],
- .unresolvedOnly:not(.draftsOnly) gr-diff-comment-thread[unresolved],
- .draftsOnly.unresolvedOnly gr-diff-comment-thread[has-draft][unresolved] {
+ .draftsOnly:not(.unresolvedOnly) gr-comment-thread[has-draft],
+ .unresolvedOnly:not(.draftsOnly) gr-comment-thread[unresolved],
+ .draftsOnly.unresolvedOnly gr-comment-thread[has-draft][unresolved] {
display: block
}
</style>
@@ -82,7 +82,7 @@
as="thread"
initial-count="5"
target-framerate="60">
- <gr-diff-comment-thread
+ <gr-comment-thread
show-file-path
change-num="[[changeNum]]"
comments="[[thread.comments]]"
@@ -94,7 +94,7 @@
path="[[thread.path]]"
root-id="{{thread.rootId}}"
on-thread-changed="_handleCommentsChanged"
- on-thread-discard="_handleThreadDiscard"></gr-diff-comment-thread>
+ on-thread-discard="_handleThreadDiscard"></gr-comment-thread>
</template>
</div>
</template>
diff --git a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
index 804446a..792644e 100644
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
@@ -171,7 +171,7 @@
];
flushAsynchronousOperations();
threadElements = Polymer.dom(element.root)
- .querySelectorAll('gr-diff-comment-thread');
+ .querySelectorAll('gr-comment-thread');
});
teardown(() => {
@@ -188,7 +188,7 @@
test('there are five threads by default', () => {
assert.equal(Polymer.dom(element.root)
- .querySelectorAll('gr-diff-comment-thread').length, 5);
+ .querySelectorAll('gr-comment-thread').length, 5);
});
test('_computeSortedThreads', () => {
@@ -231,14 +231,14 @@
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
- .querySelectorAll('gr-diff-comment-thread').length, 3);
+ .querySelectorAll('gr-comment-thread').length, 3);
});
test('toggle drafts only shows threads with draft comments', () => {
MockInteractions.tap(element.$.draftToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
- .querySelectorAll('gr-diff-comment-thread').length, 2);
+ .querySelectorAll('gr-comment-thread').length, 2);
});
test('toggle drafts and unresolved only shows threads with drafts and ' +
@@ -247,7 +247,7 @@
MockInteractions.tap(element.$.unresolvedToggle);
flushAsynchronousOperations();
assert.equal(Polymer.dom(element.root)
- .querySelectorAll('gr-diff-comment-thread').length, 2);
+ .querySelectorAll('gr-comment-thread').length, 2);
});
test('modification events are consumed and displatched', () => {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
index d2731a2..6f5a8d3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-binary.js
@@ -20,10 +20,8 @@
// Prevent redefinition.
if (window.GrDiffBuilderBinary) { return; }
- function GrDiffBuilderBinary(diff, commentThreadEls, prefs,
- outputEl) {
- GrDiffBuilder.call(this, diff, commentThreadEls, prefs,
- outputEl);
+ function GrDiffBuilderBinary(diff, prefs, outputEl) {
+ GrDiffBuilder.call(this, diff, prefs, outputEl);
}
GrDiffBuilderBinary.prototype = Object.create(GrDiffBuilder.prototype);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index f05f4f0..bf543e5 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -22,10 +22,8 @@
const IMAGE_MIME_PATTERN = /^image\/(bmp|gif|jpeg|jpg|png|tiff|webp)$/;
- function GrDiffBuilderImage(diff, commentThreadEls, prefs,
- outputEl, baseImage, revisionImage) {
- GrDiffBuilderSideBySide.call(this, diff, commentThreadEls,
- prefs, outputEl, []);
+ function GrDiffBuilderImage(diff, prefs, outputEl, baseImage, revisionImage) {
+ GrDiffBuilderSideBySide.call(this, diff, prefs, outputEl, []);
this._baseImage = baseImage;
this._revisionImage = revisionImage;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
index 81cbabb..2cf9782 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-side-by-side.js
@@ -20,10 +20,8 @@
// Prevent redefinition.
if (window.GrDiffBuilderSideBySide) { return; }
- function GrDiffBuilderSideBySide(diff, commentThreadEls,
- prefs, outputEl, layers) {
- GrDiffBuilder.call(this, diff, commentThreadEls, prefs,
- outputEl, layers);
+ function GrDiffBuilderSideBySide(diff, prefs, outputEl, layers) {
+ GrDiffBuilder.call(this, diff, prefs, outputEl, layers);
}
GrDiffBuilderSideBySide.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderSideBySide.prototype.constructor = GrDiffBuilderSideBySide;
@@ -99,10 +97,6 @@
row.appendChild(action);
} else {
const textEl = this._createTextEl(line, side);
- const threadGroupEl = this._commentThreadGroupForLine(line, side);
- if (threadGroupEl) {
- textEl.appendChild(threadGroupEl);
- }
row.appendChild(textEl);
}
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
index 2dcdee4..6020e19 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-unified.js
@@ -20,10 +20,8 @@
// Prevent redefinition.
if (window.GrDiffBuilderUnified) { return; }
- function GrDiffBuilderUnified(diff, commentThreadEls, prefs,
- outputEl, layers) {
- GrDiffBuilder.call(this, diff, commentThreadEls, prefs,
- outputEl, layers);
+ function GrDiffBuilderUnified(diff, prefs, outputEl, layers) {
+ GrDiffBuilder.call(this, diff, prefs, outputEl, layers);
}
GrDiffBuilderUnified.prototype = Object.create(GrDiffBuilder.prototype);
GrDiffBuilderUnified.prototype.constructor = GrDiffBuilderUnified;
@@ -88,10 +86,6 @@
row.appendChild(action);
} else {
const textEl = this._createTextEl(line);
- const threadGroupEl = this._commentThreadGroupForLine(line);
- if (threadGroupEl) {
- textEl.appendChild(threadGroupEl);
- }
row.appendChild(textEl);
}
return row;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 6aea35b..098a4af 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -18,7 +18,6 @@
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
<link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
-<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
<link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
<link rel="import" href="../gr-syntax-layer/gr-syntax-layer.html">
@@ -121,10 +120,6 @@
return this.queryEffectiveChildren('#diffTable');
},
- get _commentThreadElements() {
- return this.queryAllEffectiveChildren('.comment-thread');
- },
-
observers: [
'_groupsChanged(_groups.splices)',
],
@@ -292,20 +287,16 @@
let builder = null;
if (this.isImageDiff) {
- builder = new GrDiffBuilderImage(diff,
- this._commentThreadElements, prefs, this.diffElement,
+ builder = new GrDiffBuilderImage(diff, prefs, this.diffElement,
this.baseImage, this.revisionImage);
} else if (diff.binary) {
// If the diff is binary, but not an image.
- return new GrDiffBuilderBinary(diff,
- this._commentThreadElements, prefs, this.diffElement);
+ return new GrDiffBuilderBinary(diff, prefs, this.diffElement);
} else if (this.viewMode === DiffViewMode.SIDE_BY_SIDE) {
- builder = new GrDiffBuilderSideBySide(diff,
- this._commentThreadElements, prefs, this.diffElement,
+ builder = new GrDiffBuilderSideBySide(diff, prefs, this.diffElement,
this._layers);
} else if (this.viewMode === DiffViewMode.UNIFIED) {
- builder = new GrDiffBuilderUnified(diff,
- this._commentThreadElements, prefs, this.diffElement,
+ builder = new GrDiffBuilderUnified(diff, prefs, this.diffElement,
this._layers);
}
if (!builder) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index d428f68..e892605 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -20,60 +20,6 @@
// Prevent redefinition.
if (window.GrDiffBuilder) { return; }
- /** @enum {string} */
- Gerrit.DiffSide = {
- LEFT: 'left',
- RIGHT: 'right',
- BOTH: 'both',
- };
-
- /**
- * @param {!Array<!HTMLElement>} threadEls
- * @param {!{beforeNumber: (number|string|undefined),
- * afterNumber: (number|string|undefined)}}
- * lineInfo
- * @param {!Gerrit.DiffSide=} side The side (LEFT, RIGHT, BOTH) for
- * which to return the threads (default: BOTH).
- * @return {!Array<!HTMLElement>} The thread elements matching the given
- * location.
- */
- Gerrit.filterThreadElsForLocation = function(
- threadEls, lineInfo, side = Gerrit.DiffSide.BOTH) {
- function matchesLeftLine(threadEl) {
- return threadEl.getAttribute('comment-side') ==
- Gerrit.DiffSide.LEFT &&
- threadEl.getAttribute('line-num') == lineInfo.beforeNumber;
- }
- function matchesRightLine(threadEl) {
- return threadEl.getAttribute('comment-side') ==
- Gerrit.DiffSide.RIGHT &&
- threadEl.getAttribute('line-num') == lineInfo.afterNumber;
- }
- function matchesFileComment(threadEl) {
- return (side === Gerrit.DiffSide.BOTH ||
- threadEl.getAttribute('comment-side') == side) &&
- // line/range comments have 1-based line set, if line is falsy it's
- // a file comment
- !threadEl.getAttribute('line-num');
- }
-
- // Select the appropriate matchers for the desired side and line
- // If side is BOTH, we want both the left and right matcher.
- const matchers = [];
- if (side !== Gerrit.DiffSide.RIGHT) {
- matchers.push(matchesLeftLine);
- }
- if (side !== Gerrit.DiffSide.LEFT) {
- matchers.push(matchesRightLine);
- }
- if (lineInfo.afterNumber === 'FILE' ||
- lineInfo.beforeNumber === 'FILE') {
- matchers.push(matchesFileComment);
- }
- return threadEls.filter(threadEl =>
- matchers.some(matcher => matcher(threadEl)));
- };
-
/**
* In JS, unicode code points above 0xFFFF occupy two elements of a string.
* For example '𐀏'.length is 2. An occurence of such a code point is called a
@@ -96,8 +42,7 @@
*/
const REGEX_TAB_OR_SURROGATE_PAIR = /\t|[\uD800-\uDBFF][\uDC00-\uDFFF]/;
- function GrDiffBuilder(diff, commentThreadEls, prefs,
- outputEl, layers) {
+ function GrDiffBuilder(diff, prefs, outputEl, layers) {
this._diff = diff;
this._prefs = prefs;
this._outputEl = outputEl;
@@ -119,8 +64,6 @@
layer.addListener(this._handleLayerUpdate.bind(this));
}
}
-
- this._threadEls = commentThreadEls;
}
GrDiffBuilder.GroupType = {
@@ -373,31 +316,6 @@
return button;
};
- /**
- * @param {!GrDiffLine} line
- * @param {!GrDiffBuilder.Side=} side The side (LEFT, RIGHT, BOTH) for which
- * to return the thread group (default: BOTH).
- * @return {!Object}
- */
- GrDiffBuilder.prototype._commentThreadGroupForLine = function(
- line, commentSide = GrDiffBuilder.Side.BOTH) {
- const threadElsForGroup =
- Gerrit.filterThreadElsForLocation(this._threadEls, line, commentSide);
- if (!threadElsForGroup || threadElsForGroup.length === 0) {
- return null;
- }
-
- const threadGroupEl = document.createElement('div');
- threadGroupEl.className = 'thread-group';
- for (const threadEl of threadElsForGroup) {
- Polymer.dom(threadGroupEl).appendChild(threadEl);
- }
- if (commentSide) {
- threadGroupEl.setAttribute('data-side', commentSide);
- }
- return threadGroupEl;
- };
-
GrDiffBuilder.prototype._createLineEl = function(
line, number, type, opt_class) {
const td = this._createElement('td');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
index c7a1140..1b0ba04 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder_test.html
@@ -75,70 +75,11 @@
show_tabs: true,
tab_size: 4,
};
- builder = new GrDiffBuilder({content: []}, [], prefs);
+ builder = new GrDiffBuilder({content: []}, prefs);
});
teardown(() => { sandbox.restore(); });
- test('filterThreadElsForLocation with no threads', () => {
- const line = {beforeNumber: 3, afterNumber: 5};
-
- const threads = [];
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line), []);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
- Gerrit.DiffSide.LEFT), []);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threads, line,
- Gerrit.DiffSide.RIGHT), []);
- });
-
- test('filterThreadElsForLocation for line comments', () => {
- const line = {beforeNumber: 3, afterNumber: 5};
-
- const l3 = document.createElement('div');
- l3.setAttribute('line-num', 3);
- l3.setAttribute('comment-side', 'left');
-
- const l5 = document.createElement('div');
- l5.setAttribute('line-num', 5);
- l5.setAttribute('comment-side', 'left');
-
- const r3 = document.createElement('div');
- r3.setAttribute('line-num', 3);
- r3.setAttribute('comment-side', 'right');
-
- const r5 = document.createElement('div');
- r5.setAttribute('line-num', 5);
- r5.setAttribute('comment-side', 'right');
-
- const threadEls = [l3, l5, r3, r5];
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
- [l3, r5]);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
- Gerrit.DiffSide.LEFT), [l3]);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
- Gerrit.DiffSide.RIGHT), [r5]);
- });
-
- test('filterThreadElsForLocation for file comments', () => {
- const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
-
- const l = document.createElement('div');
- l.setAttribute('comment-side', 'left');
-
- const r = document.createElement('div');
- r.setAttribute('comment-side', 'right');
-
- const threadEls = [l, r];
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line),
- [l, r]);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
- Gerrit.DiffSide.BOTH), [l, r]);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
- Gerrit.DiffSide.LEFT), [l]);
- assert.deepEqual(Gerrit.filterThreadElsForLocation(threadEls, line,
- Gerrit.DiffSide.RIGHT), [r]);
- });
-
test('_createElement classStr applies all classes', () => {
const node = builder._createElement('div', 'test classes');
assert.isTrue(node.classList.contains('gr-diff'));
@@ -312,73 +253,6 @@
}
});
- test('comment thread group creation', () => {
- const l3 = document.createElement('div');
- l3.className = 'comment-thread';
- l3.setAttribute('comment-side', 'left');
- l3.setAttribute('line-num', 3);
-
- const l5 = document.createElement('div');
- l5.className = 'comment-thread';
- l5.setAttribute('comment-side', 'left');
- l5.setAttribute('line-num', 5);
-
- const r5 = document.createElement('div');
- r5.className = 'comment-thread';
- r5.setAttribute('comment-side', 'right');
- r5.setAttribute('line-num', 5);
-
- builder = new GrDiffBuilder({content: []}, [l3, l5, r5], prefs);
-
- function checkThreadGroupProps(threadGroupEl,
- expectedThreadEls) {
- const threadEls = Polymer.dom(threadGroupEl).queryDistributedElements(
- '.comment-thread');
- assert.equal(threadEls.length, expectedThreadEls.length);
- for (let i=0; i<expectedThreadEls.length; i++) {
- assert.equal(threadEls[i], expectedThreadEls[i]);
- }
- }
-
- let line = new GrDiffLine(GrDiffLine.Type.BOTH);
- line.beforeNumber = 5;
- line.afterNumber = 5;
- let threadGroupEl = builder._commentThreadGroupForLine(line);
- checkThreadGroupProps(threadGroupEl, [l5, r5]);
-
- threadGroupEl =
- builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- checkThreadGroupProps(threadGroupEl, [r5]);
-
- threadGroupEl =
- builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- checkThreadGroupProps(threadGroupEl, [l5]);
-
- threadGroupEl = builder._commentThreadGroupForLine(line);
- checkThreadGroupProps(threadGroupEl, [l5, r5]);
-
- threadEl =
- builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.LEFT);
- checkThreadGroupProps(threadEl, [l5]);
-
- threadGroupEl =
- builder._commentThreadGroupForLine(line, GrDiffBuilder.Side.RIGHT);
- checkThreadGroupProps(threadGroupEl, [r5]);
-
- line = new GrDiffLine(GrDiffLine.Type.REMOVE);
- line.beforeNumber = 5;
- line.afterNumber = 5;
- threadGroupEl = builder._commentThreadGroupForLine(line);
- checkThreadGroupProps(threadGroupEl, [l5, r5]);
-
- line = new GrDiffLine(GrDiffLine.Type.ADD);
- line.beforeNumber = 3;
- line.afterNumber = 5;
- threadGroupEl = builder._commentThreadGroupForLine(line);
- checkThreadGroupProps(threadGroupEl, [l3, r5]);
- });
-
-
test('_handlePreferenceError called with invalid preference', () => {
sandbox.stub(element, '_handlePreferenceError');
const prefs = {tab_size: 0};
@@ -913,7 +787,7 @@
outputEl = element.queryEffectiveChildren('#diffTable');
keyLocations = {left: {}, right: {}};
sandbox.stub(element, '_getDiffBuilder', () => {
- const builder = new GrDiffBuilder({content}, [], prefs, outputEl);
+ const builder = new GrDiffBuilder({content}, prefs, outputEl);
sandbox.stub(builder, 'addColumns');
builder.buildSectionElement = function(group) {
const section = document.createElement('stub');
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index aee7a62..860d900 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -293,7 +293,7 @@
},
_rowHasThread(row) {
- return row.querySelector('gr-diff-comment-thread');
+ return row.querySelector('.comment-thread');
},
/**
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
index 85ba202..ff00383 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight.js
@@ -37,8 +37,8 @@
},
listeners: {
- 'comment-mouse-out': '_handleCommentMouseOut',
- 'comment-mouse-over': '_handleCommentMouseOver',
+ 'comment-thread-mouseleave': '_handleCommentThreadMouseleave',
+ 'comment-thread-mouseenter': '_handleCommentThreadMouseenter',
'create-range-comment': '_createRangeComment',
},
@@ -74,8 +74,16 @@
this.debounce('selectionChange', this._handleSelection, 200);
},
- _handleCommentMouseOver(e) {
- const threadEl = Polymer.dom(e).localTarget;
+ _getThreadEl(e) {
+ const path = Polymer.dom(e).path || [];
+ for (const pathEl of path) {
+ if (pathEl.classList.contains('comment-thread')) return pathEl;
+ }
+ return null;
+ },
+
+ _handleCommentThreadMouseenter(e) {
+ const threadEl = this._getThreadEl(e);
const index = this._indexForThreadEl(threadEl);
if (index !== undefined) {
@@ -83,8 +91,8 @@
}
},
- _handleCommentMouseOut(e) {
- const threadEl = Polymer.dom(e).localTarget;
+ _handleCommentThreadMouseleave(e) {
+ const threadEl = this._getThreadEl(e);
const index = this._indexForThreadEl(threadEl);
if (index !== undefined) {
@@ -248,7 +256,7 @@
node = contentText;
column = 0;
} else {
- const thread = contentTd.querySelector('gr-diff-comment-thread');
+ const thread = contentTd.querySelector('.comment-thread');
if (thread && thread.contains(node)) {
column = this._getLength(contentText);
node = contentText;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
index 23de407..8e3c7b0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-highlight/gr-diff-highlight_test.html
@@ -72,9 +72,9 @@
<tr class="diff-row side-by-side" left-type="remove" right-type="add">
<td class="left lineNum" data-value="140"></td>
<!-- Next tag is formatted to eliminate zero-length text nodes. -->
- <td class="content remove"><div class="contentText">na💢ti <hl class="foo">te, inquit</hl>, sumus <hl class="bar">aliquando</hl> otiosum, <hl>certe</hl> a <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl>udiam, <hl>quid</hl> sit, <span class="tab-indicator" style="tab-size:8;"> </span>quod <hl>Epicurum</hl></div><gr-diff-comment-thread>
+ <td class="content remove"><div class="contentText">na💢ti <hl class="foo">te, inquit</hl>, sumus <hl class="bar">aliquando</hl> otiosum, <hl>certe</hl> a <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl>udiam, <hl>quid</hl> sit, <span class="tab-indicator" style="tab-size:8;"> </span>quod <hl>Epicurum</hl></div><div class="comment-thread">
[Yet another random diff thread content here]
- </gr-diff-comment-thread></td>
+ </div></td>
<td class="right lineNum" data-value="120"></td>
<!-- Next tag is formatted to eliminate zero-length text nodes. -->
<td class="content add"><div class="contentText">nacti , <hl>,</hl> sumus <hl><span class="tab-indicator" style="tab-size:8;"> </span></hl> otiosum, <span class="tab-indicator" style="tab-size:8;"> </span> audiam, sit, quod</div></td>
@@ -197,22 +197,60 @@
element._cachedDiffBuilder = builder;
});
- test('comment-mouse-over from line comments is ignored', () => {
+ test('comment-thread-mouseenter from line comments is ignored', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right'}];
+
sandbox.stub(element, 'set');
- element.fire('comment-mouse-over', {comment: {}});
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isFalse(element.set.called);
});
- test('comment-mouse-over from ranged comment causes set', () => {
+ test('comment-thread-mouseenter from ranged comment causes set', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ threadEl.setAttribute('range', JSON.stringify({
+ start_line: 3,
+ start_character: 4,
+ end_line: 5,
+ end_character: 6,
+ }));
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right', range: {
+ start_line: 3,
+ start_character: 4,
+ end_line: 5,
+ end_character: 6,
+ }}];
+
sandbox.stub(element, 'set');
- sandbox.stub(element, '_indexForThreadEl').returns(0);
- element.fire('comment-mouse-over', {comment: {range: {}}});
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
assert.isTrue(element.set.called);
+ const args = element.set.lastCall.args;
+ assert.deepEqual(args[0], ['commentRanges', 0, 'hovering']);
+ assert.deepEqual(args[1], true);
});
- test('comment-mouse-out from line comments is ignored', () => {
- element.fire('comment-mouse-over', {comment: {}});
- assert.isFalse(builder.getContentsByLineRange.called);
+ test('comment-thread-mouseleave from line comments is ignored', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ threadEl.setAttribute('comment-side', 'right');
+ threadEl.setAttribute('line-num', 3);
+ element.appendChild(threadEl);
+ element.commentRanges = [{side: 'right'}];
+
+ sandbox.stub(element, 'set');
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseleave', {bubbles: true}));
+ assert.isFalse(element.set.called);
});
test('on create-range-comment action box is removed', () => {
@@ -465,7 +503,7 @@
test('starts in comment thread element', () => {
const startContent = stubContent(140, 'left');
const comment = startContent.parentElement.querySelector(
- 'gr-diff-comment-thread');
+ '.comment-thread');
const endContent = stubContent(141, 'left');
emulateSelection(comment.firstChild, 2, endContent.firstChild, 4);
assert.isTrue(element.isRangeSelected());
@@ -481,7 +519,7 @@
test('ends in comment thread element', () => {
const content = stubContent(140, 'left');
const comment = content.parentElement.querySelector(
- 'gr-diff-comment-thread');
+ '.comment-thread');
emulateSelection(content.firstChild, 4, comment.firstChild, 1);
assert.isTrue(element.isRangeSelected());
assert.deepEqual(getActionRange(), {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
index 3eb7b37..4c310b9 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html
@@ -19,7 +19,7 @@
<link rel="import" href="../../../behaviors/gr-patch-set-behavior/gr-patch-set-behavior.html">
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
-<link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
+<link rel="import" href="../../shared/gr-comment-thread/gr-comment-thread.html">
<link rel="import" href="../gr-diff/gr-diff.html">
<dom-module id="gr-diff-host">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
index 5839141..8b9d8066 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js
@@ -63,6 +63,12 @@
a.end_character === b.end_character;
}
+ /** @enum {string} */
+ Gerrit.DiffSide = {
+ LEFT: 'left',
+ RIGHT: 'right',
+ };
+
/**
* Wrapper around gr-diff.
*
@@ -196,11 +202,6 @@
type: Number,
computed: '_computeParentIndex(patchRange.*)',
},
-
- _threadEls: {
- type: Array,
- value: () => [],
- },
},
behaviors: [
@@ -344,7 +345,7 @@
* @return {!Array<!HTMLElement>}
*/
getThreadEls() {
- return this._threadEls;
+ return Polymer.dom(this.$.diff).querySelectorAll('.comment-thread');
},
/** @param {HTMLElement} el */
@@ -471,7 +472,6 @@
return isImageDiff(diff);
},
-
_commentsChanged(newComments) {
const allComments = [];
for (const side of [GrDiffBuilder.Side.LEFT, GrDiffBuilder.Side.RIGHT]) {
@@ -590,21 +590,20 @@
},
_attachThreadElement(threadEl) {
- this._threadEls.push(threadEl);
Polymer.dom(this.$.diff).appendChild(threadEl);
},
_clearThreads() {
- for (const threadEl of this._threadEls) {
+ for (const threadEl of this.getThreadEls()) {
const parent = Polymer.dom(threadEl).parentNode;
Polymer.dom(parent).removeChild(threadEl);
}
- this._threadEls = [];
},
_createThreadElement(thread) {
- const threadEl = document.createElement('gr-diff-comment-thread');
+ const threadEl = document.createElement('gr-comment-thread');
threadEl.className = 'comment-thread';
+ threadEl.slot = `${thread.commentSide}-${thread.lineNum}`;
threadEl.comments = thread.comments;
threadEl.commentSide = thread.commentSide;
threadEl.isOnParent = !!thread.isOnParent;
@@ -625,10 +624,6 @@
const parent = Polymer.dom(threadEl).parentNode;
Polymer.dom(parent).removeChild(threadEl);
- const i = this._threadEls.findIndex(
- threadEl => threadEl.rootId == e.detail.rootId);
- this._threadEls.splice(i, 1);
-
threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
threadEl.removeEventListener('thread-discard', threadDiscardListener);
};
@@ -660,12 +655,57 @@
return rangesEqual(threadRange, range);
}
- const filteredThreadEls = Gerrit.filterThreadElsForLocation(
- this._threadEls, line, commentSide).filter(matchesRange);
+ const filteredThreadEls = this._filterThreadElsForLocation(
+ this.getThreadEls(), line, commentSide).filter(matchesRange);
return filteredThreadEls.length ? filteredThreadEls[0] : null;
},
/**
+ * @param {!Array<!HTMLElement>} threadEls
+ * @param {!{beforeNumber: (number|string|undefined|null),
+ * afterNumber: (number|string|undefined|null)}}
+ * lineInfo
+ * @param {!Gerrit.DiffSide=} side The side (LEFT, RIGHT) for
+ * which to return the threads.
+ * @return {!Array<!HTMLElement>} The thread elements matching the given
+ * location.
+ */
+ _filterThreadElsForLocation(threadEls, lineInfo, side) {
+ function matchesLeftLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.LEFT &&
+ threadEl.getAttribute('line-num') == lineInfo.beforeNumber;
+ }
+ function matchesRightLine(threadEl) {
+ return threadEl.getAttribute('comment-side') ==
+ Gerrit.DiffSide.RIGHT &&
+ threadEl.getAttribute('line-num') == lineInfo.afterNumber;
+ }
+ function matchesFileComment(threadEl) {
+ return threadEl.getAttribute('comment-side') == side &&
+ // line/range comments have 1-based line set, if line is falsy it's
+ // a file comment
+ !threadEl.getAttribute('line-num');
+ }
+
+ // Select the appropriate matchers for the desired side and line
+ // If side is BOTH, we want both the left and right matcher.
+ const matchers = [];
+ if (side !== Gerrit.DiffSide.RIGHT) {
+ matchers.push(matchesLeftLine);
+ }
+ if (side !== Gerrit.DiffSide.LEFT) {
+ matchers.push(matchesRightLine);
+ }
+ if (lineInfo.afterNumber === 'FILE' ||
+ lineInfo.beforeNumber === 'FILE') {
+ matchers.push(matchesFileComment);
+ }
+ return threadEls.filter(threadEl =>
+ matchers.some(matcher => matcher(threadEl)));
+ },
+
+ /**
* Take a diff that was loaded with a ignore-whitespace other than
* IGNORE_NONE, and convert delta chunks labeled as common into shared
* chunks.
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
index 5344256..ab9daec 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html
@@ -256,7 +256,7 @@
threadEls[0].dispatchEvent(
new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
const attachedThreads = element.queryAllEffectiveChildren(
- 'gr-diff-comment-thread');
+ 'gr-comment-thread');
assert.equal(attachedThreads.length, 1);
assert.equal(attachedThreads[0].rootId, 42);
});
@@ -769,10 +769,11 @@
});
});
- test('getThreadEls() returns _threadEls', () => {
- const returnValue = [document.createElement('b')];
- element._threadEls = returnValue;
- assert.equal(element.getThreadEls(), returnValue);
+ test('getThreadEls() returns .comment-threads', () => {
+ const threadEl = document.createElement('div');
+ threadEl.className = 'comment-thread';
+ Polymer.dom(element.$.diff).appendChild(threadEl);
+ assert.deepEqual(element.getThreadEls(), [threadEl]);
});
test('delegates addDraftAtLine(el)', () => {
@@ -1129,7 +1130,7 @@
commentSide, undefined, false));
let threads = Polymer.dom(element.$.diff)
- .queryDistributedElements('gr-diff-comment-thread');
+ .queryDistributedElements('gr-comment-thread');
assert.equal(threads.length, 1);
assert.equal(threads[0].commentSide, commentSide);
@@ -1150,7 +1151,7 @@
'3', 1, commentSide, range, true));
threads = Polymer.dom(element.$.diff)
- .queryDistributedElements('gr-diff-comment-thread');
+ .queryDistributedElements('gr-comment-thread');
assert.equal(threads.length, 2);
assert.equal(threads[1].commentSide, commentSide);
@@ -1159,6 +1160,65 @@
assert.equal(threads[1].patchNum, 3);
});
+ test('_filterThreadElsForLocation with no threads', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
+
+ const threads = [];
+ assert.deepEqual(element._filterThreadElsForLocation(threads, line), []);
+ assert.deepEqual(element._filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.LEFT), []);
+ assert.deepEqual(element._filterThreadElsForLocation(threads, line,
+ Gerrit.DiffSide.RIGHT), []);
+ });
+
+ test('_filterThreadElsForLocation for line comments', () => {
+ const line = {beforeNumber: 3, afterNumber: 5};
+
+ const l3 = document.createElement('div');
+ l3.setAttribute('line-num', 3);
+ l3.setAttribute('comment-side', 'left');
+
+ const l5 = document.createElement('div');
+ l5.setAttribute('line-num', 5);
+ l5.setAttribute('comment-side', 'left');
+
+ const r3 = document.createElement('div');
+ r3.setAttribute('line-num', 3);
+ r3.setAttribute('comment-side', 'right');
+
+ const r5 = document.createElement('div');
+ r5.setAttribute('line-num', 5);
+ r5.setAttribute('comment-side', 'right');
+
+ const threadEls = [l3, l5, r3, r5];
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line),
+ [l3, r5]);
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l3]);
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r5]);
+ });
+
+ test('_filterThreadElsForLocation for file comments', () => {
+ const line = {beforeNumber: 'FILE', afterNumber: 'FILE'};
+
+ const l = document.createElement('div');
+ l.setAttribute('comment-side', 'left');
+
+ const r = document.createElement('div');
+ r.setAttribute('comment-side', 'right');
+
+ const threadEls = [l, r];
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line),
+ [l, r]);
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.BOTH), [l, r]);
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.LEFT), [l]);
+ assert.deepEqual(element._filterThreadElsForLocation(threadEls, line,
+ Gerrit.DiffSide.RIGHT), [r]);
+ });
+
suite('_translateChunksToIgnore', () => {
let content;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
index 27e467d..6a9d88f 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection.js
@@ -83,7 +83,7 @@
targetClasses.push(SelectionClass.BLAME);
} else {
const commentSelected =
- this._elementDescendedFromClass(e.target, 'gr-diff-comment');
+ this._elementDescendedFromClass(e.target, 'gr-comment');
const side = this.diffBuilder.getSideByLineEl(lineEl);
targetClasses.push(side === 'left' ?
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
index f34429b8..469a894 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-selection/gr-diff-selection_test.html
@@ -36,7 +36,7 @@
<td class="content">
<div class="contentText" data-side="left">ba ba</div>
<div data-side="left">
- <div class="gr-diff-comment-thread">
+ <div class="comment-thread">
<div class="gr-formatted-text message">
<span id="output" class="gr-linked-text">This is a comment</span>
</div>
@@ -58,7 +58,7 @@
<td class="content">
<div class="contentText" data-side="right">more more more</div>
<div data-side="right">
- <div class="gr-diff-comment-thread">
+ <div class="comment-thread">
<div class="gr-formatted-text message">
<span id="output" class="gr-linked-text">This is a comment on the right</span>
</div>
@@ -72,7 +72,7 @@
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
<div data-side="left">
- <div class="gr-diff-comment-thread">
+ <div class="comment-thread">
<div class="gr-formatted-text message">
<span id="output" class="gr-linked-text">This is <a>a</a> different comment 💩 unicode is fun</span>
</div>
@@ -87,7 +87,7 @@
<td class="content">
<div class="contentText" data-side="left">ga ga</div>
<div data-side="left">
- <div class="gr-diff-comment-thread">
+ <div class="comment-thread">
<textarea data-side="right">test for textarea copying</textarea>
</div>
</div>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
index 5a56069..dadf8a7 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.js
@@ -221,7 +221,7 @@
[this.Shortcut.EXPAND_ALL_DIFF_CONTEXT]: '_handleExpandAllDiffContext',
[this.Shortcut.NEXT_UNREVIEWED_FILE]: '_handleNextUnreviewedFile',
- // Final two are actually handled by gr-diff-comment-thread.
+ // Final two are actually handled by gr-comment-thread.
[this.Shortcut.EXPAND_ALL_COMMENT_THREADS]: null,
[this.Shortcut.COLLAPSE_ALL_COMMENT_THREADS]: null,
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index e587953..4ccdf96 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -294,7 +294,6 @@
is-image-diff="[[isImageDiff]]"
base-image="[[baseImage]]"
revision-image="[[revisionImage]]">
- <slot></slot>
<table
id="diffTable"
class$="[[_diffTableClass]]"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index a8cf320..996d484 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -60,6 +60,20 @@
node.classList.contains('comment-thread');
}
+ /**
+ * Turn a slot element into the corresponding content element.
+ * Slots are only fully supported in Polymer 2 - in Polymer 1, they are
+ * replaced with content elements during template parsing. This conversion is
+ * not applied for imperatively created slot elements, so this method
+ * implements the same behavior as the template parsing for imperative slots.
+ */
+ Gerrit.slotToContent = function(slot) {
+ const content = document.createElement('content');
+ content.name = slot.name;
+ content.setAttribute('select', `[slot='${slot.name}']`);
+ return content;
+ };
+
Polymer({
is: 'gr-diff',
@@ -250,6 +264,7 @@
// this situation until it occurs.
this._updateRanges(addedThreadEls);
this._updateKeyLocations(addedThreadEls);
+ this._redispatchHoverEvents(addedThreadEls);
});
},
@@ -274,6 +289,20 @@
}
},
+ // Dispatch events that are handled by the gr-diff-highlight.
+ _redispatchHoverEvents(addedThreadEls) {
+ for (const threadEl of addedThreadEls) {
+ threadEl.addEventListener('mouseenter', () => {
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseenter', {bubbles: true}));
+ });
+ threadEl.addEventListener('mouseleave', () => {
+ threadEl.dispatchEvent(
+ new CustomEvent('comment-thread-mouseleave', {bubbles: true}));
+ });
+ }
+ },
+
/** Cancel any remaining diff builder rendering work. */
cancel() {
this.$.diffBuilder.cancel();
@@ -440,14 +469,16 @@
* Gets or creates a comment thread group for a specific line and side on a
* diff.
* @param {!Object} contentEl
+ * @param {!Gerrit.DiffSide} commentSide
* @return {!Node}
*/
- _getOrCreateThreadGroup(contentEl) {
+ _getOrCreateThreadGroup(contentEl, commentSide) {
// Check if thread group exists.
let threadGroupEl = this._getThreadGroupForLine(contentEl);
if (!threadGroupEl) {
threadGroupEl = document.createElement('div');
threadGroupEl.className = 'thread-group';
+ threadGroupEl.setAttribute('data-side', commentSide);
contentEl.appendChild(threadGroupEl);
}
return threadGroupEl;
@@ -609,8 +640,17 @@
lineNumString, commentSide);
const contentText = this.$.diffBuilder.getContentByLineEl(lineEl);
const contentEl = contentText.parentElement;
- const threadGroupEl = this._getOrCreateThreadGroup(contentEl);
- Polymer.dom(threadGroupEl).appendChild(threadEl);
+ const threadGroupEl = this._getOrCreateThreadGroup(
+ contentEl, commentSide);
+ // Create a slot for the thread and attach it to the thread group.
+ // The Polyfill has some bugs and this only works if the slot is
+ // attached to the group after the group is attached to the DOM.
+ // The thread group may already have a slot with the right name, but
+ // that is okay because the first matching slot is used and the rest
+ // are ignored.
+ const slot = document.createElement('slot');
+ slot.name = threadEl.slot;
+ Polymer.dom(threadGroupEl).appendChild(Gerrit.slotToContent(slot));
}
});
},
diff --git a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
index fa488f0..8cee1f4 100644
--- a/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
+++ b/polygerrit-ui/app/elements/diff/gr-ranged-comment-layer/gr-ranged-comment-layer.js
@@ -167,6 +167,9 @@
const ranges = this.get(['_rangesMap', side, lineNum]) || [];
return ranges
.map(range => {
+ // Make a copy, so that the normalization below does not mess with
+ // our map.
+ range = Object.assign({}, range);
range.end = range.end === -1 ? line.text.length : range.end;
// Normalize invalid ranges where the start is after the end but the
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
similarity index 95%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
rename to polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
index c3a1de4..8c80b37 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.html
@@ -22,9 +22,9 @@
<link rel="import" href="../../core/gr-reporting/gr-reporting.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../shared/gr-storage/gr-storage.html">
-<link rel="import" href="../gr-diff-comment/gr-diff-comment.html">
+<link rel="import" href="../gr-comment/gr-comment.html">
-<dom-module id="gr-diff-comment-thread">
+<dom-module id="gr-comment-thread">
<template>
<style include="shared-styles">
gr-button {
@@ -72,7 +72,7 @@
<div id="container" class$="[[_computeHostClass(unresolved)]]">
<template id="commentList" is="dom-repeat" items="[[_orderedComments]]"
as="comment">
- <gr-diff-comment
+ <gr-comment
comment="{{comment}}"
robot-button-disabled="[[_hideActions(_showActions, _lastComment)]]"
change-num="[[changeNum]]"
@@ -85,7 +85,7 @@
project-config="[[_projectConfig]]"
on-create-fix-comment="_handleCommentFix"
on-comment-discard="_handleCommentDiscard"
- on-comment-save="_handleCommentSavedOrDiscarded"></gr-diff-comment>
+ on-comment-save="_handleCommentSavedOrDiscarded"></gr-comment>
</template>
<div id="commentInfoContainer"
hidden$="[[_hideActions(_showActions, _lastComment)]]">
@@ -122,5 +122,5 @@
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
<gr-storage id="storage"></gr-storage>
</template>
- <script src="gr-diff-comment-thread.js"></script>
+ <script src="gr-comment-thread.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
similarity index 96%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
rename to polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
index a2439d7..11fce6d 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread.js
@@ -21,7 +21,7 @@
const NEWLINE_PATTERN = /\n/g;
Polymer({
- is: 'gr-diff-comment-thread',
+ is: 'gr-comment-thread',
/**
* Fired when the thread should be discarded.
@@ -36,7 +36,7 @@
*/
/**
- * gr-diff-comment-thread exposes the following attributes that allow a
+ * gr-comment-thread exposes the following attributes that allow a
* diff widget like gr-diff to show the thread in the right location:
*
* line-num:
@@ -219,7 +219,7 @@
if (this.shouldSuppressKeyboardShortcut(e)) { return; }
// Don’t preventDefault in this case because it will render the event
- // useless for other handlers (other gr-diff-comment-thread elements).
+ // useless for other handlers (other gr-comment-thread elements).
if (e.detail.keyboardEvent.shiftKey) {
this._expandCollapseComments(true);
} else {
@@ -230,7 +230,7 @@
_expandCollapseComments(actionIsCollapse) {
const comments =
- Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
+ Polymer.dom(this.root).querySelectorAll('gr-comment');
for (const comment of comments) {
comment.collapsed = actionIsCollapse;
}
@@ -283,7 +283,7 @@
parent.range);
// If there is currently a comment in an editing state, add an attribute
- // so that the gr-diff-comment knows not to populate the draft text.
+ // so that the gr-comment knows not to populate the draft text.
for (let i = 0; i < this.comments.length; i++) {
if (this.comments[i].__editing) {
reply.__otherEditing = true;
@@ -350,7 +350,7 @@
},
_commentElWithDraftID(id) {
- const els = Polymer.dom(this.root).querySelectorAll('gr-diff-comment');
+ const els = Polymer.dom(this.root).querySelectorAll('gr-comment');
for (const el of els) {
if (el.comment.id === id || el.comment.__draftID === id) {
return el;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
similarity index 95%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
rename to polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
index 1881497..2e1b3bd 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.html
@@ -17,31 +17,31 @@
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>gr-diff-comment-thread</title>
+<title>gr-comment-thread</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
<link rel="import" href="../../../test/common-test-setup.html"/>
<script src="../../../scripts/util.js"></script>
-<link rel="import" href="gr-diff-comment-thread.html">
+<link rel="import" href="gr-comment-thread.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
- <gr-diff-comment-thread></gr-diff-comment-thread>
+ <gr-comment-thread></gr-comment-thread>
</template>
</test-fixture>
<test-fixture id="withComment">
<template>
- <gr-diff-comment-thread></gr-diff-comment-thread>
+ <gr-comment-thread></gr-comment-thread>
</template>
</test-fixture>
<script>
- suite('gr-diff-comment-thread tests', () => {
+ suite('gr-comment-thread tests', () => {
let element;
let sandbox;
@@ -248,7 +248,7 @@
});
test('reply', () => {
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
const reportStub = sandbox.stub(element.$.reporting,
'recordDraftInteraction');
assert.ok(commentEl);
@@ -267,7 +267,7 @@
});
test('quote reply', () => {
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
const reportStub = sandbox.stub(element.$.reporting,
'recordDraftInteraction');
assert.ok(commentEl);
@@ -300,7 +300,7 @@
}];
flushAsynchronousOperations();
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const quoteBtn = element.$.quoteBtn;
@@ -323,7 +323,7 @@
element.changeNum = '42';
element.patchNum = '1';
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const ackBtn = element.$.ackBtn;
@@ -346,7 +346,7 @@
'recordDraftInteraction');
element.changeNum = '42';
element.patchNum = '1';
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const doneBtn = element.$.doneBtn;
@@ -368,12 +368,12 @@
element.changeNum = '42';
element.patchNum = '1';
element.path = '/path/to/file.txt';
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
- element.$$('gr-diff-comment')._fireSave();
+ element.$$('gr-comment')._fireSave();
flush(() => {
assert.isTrue(saveOrDiscardStub.called);
@@ -389,7 +389,7 @@
test('please fix', done => {
element.changeNum = '42';
element.patchNum = '1';
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
assert.ok(commentEl);
commentEl.addEventListener('create-fix-comment', () => {
const drafts = element._orderedComments.filter(c => {
@@ -420,7 +420,7 @@
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl =
- Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
+ Polymer.dom(element.root).querySelectorAll('gr-comment')[1];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
const drafts = element.comments.filter(c => {
@@ -452,7 +452,7 @@
const saveOrDiscardStub = sandbox.stub();
element.addEventListener('thread-changed', saveOrDiscardStub);
const draftEl =
- Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[0];
+ Polymer.dom(element.root).querySelectorAll('gr-comment')[0];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
assert.equal(element.comments.length, 0);
@@ -534,7 +534,7 @@
flushAsynchronousOperations();
const draftEl =
- Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1];
+ Polymer.dom(element.root).querySelectorAll('gr-comment')[1];
assert.ok(draftEl);
draftEl.addEventListener('comment-discard', () => {
assert.isFalse(storageStub.called);
@@ -546,7 +546,7 @@
});
test('comment-update', () => {
- const commentEl = element.$$('gr-diff-comment');
+ const commentEl = element.$$('gr-comment');
const updatedComment = {
id: element.comments[0].id,
foo: 'bar',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
similarity index 97%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
rename to polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
index 72c7285..a470285 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.html
@@ -35,7 +35,7 @@
<link rel="import" href="../gr-confirm-delete-comment-dialog/gr-confirm-delete-comment-dialog.html">
<script src="../../../scripts/rootElement.js"></script>
-<dom-module id="gr-diff-comment">
+<dom-module id="gr-comment">
<template>
<style include="shared-styles">
:host {
@@ -232,10 +232,7 @@
display: block;
}
</style>
- <div id="container"
- class="container"
- on-mouseenter="_handleMouseEnter"
- on-mouseleave="_handleMouseLeave">
+ <div id="container" class="container">
<div class="header" id="header" on-tap="_handleToggleCollapsed">
<div class="headerLeft">
<span class="authorName">[[comment.author.name]]</span>
@@ -387,5 +384,5 @@
<gr-storage id="storage"></gr-storage>
<gr-reporting id="reporting"></gr-reporting>
</template>
- <script src="gr-diff-comment.js"></script>
+ <script src="gr-comment.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
similarity index 97%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
rename to polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
index 5165db0..a576583 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment.js
@@ -32,7 +32,7 @@
const FILE = 'FILE';
Polymer({
- is: 'gr-diff-comment',
+ is: 'gr-comment',
/**
* Fired when the create fix comment action is triggered.
@@ -59,14 +59,6 @@
*/
/**
- * @event comment-mouse-over
- */
-
- /**
- * @event comment-mouse-out
- */
-
- /**
* Fired when the comment's timestamp is tapped.
*
* @event comment-anchor-tap
@@ -610,14 +602,6 @@
}
},
- _handleMouseEnter(e) {
- this.fire('comment-mouse-over', this._getEventPayload());
- },
-
- _handleMouseLeave(e) {
- this.fire('comment-mouse-out', this._getEventPayload());
- },
-
_handleToggleResolved() {
this.$.reporting.recordDraftInteraction();
this.resolved = !this.resolved;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
similarity index 98%
rename from polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
rename to polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
index 912e615..7ca5242 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.html
@@ -17,7 +17,7 @@
-->
<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
-<title>gr-diff-comment</title>
+<title>gr-comment</title>
<script src="../../../bower_components/webcomponentsjs/webcomponents-lite.min.js"></script>
<script src="../../../bower_components/web-component-tester/browser.js"></script>
@@ -25,19 +25,19 @@
<script src="../../../bower_components/page/page.js"></script>
<script src="../../../scripts/util.js"></script>
-<link rel="import" href="gr-diff-comment.html">
+<link rel="import" href="gr-comment.html">
<script>void(0);</script>
<test-fixture id="basic">
<template>
- <gr-diff-comment></gr-diff-comment>
+ <gr-comment></gr-comment>
</template>
</test-fixture>
<test-fixture id="draft">
<template>
- <gr-diff-comment draft="true"></gr-diff-comment>
+ <gr-comment draft="true"></gr-comment>
</template>
</test-fixture>
@@ -48,7 +48,7 @@
return getComputedStyle(el).getPropertyValue('display') !== 'none';
}
- suite('gr-diff-comment tests', () => {
+ suite('gr-comment tests', () => {
let element;
let sandbox;
setup(() => {
@@ -343,7 +343,7 @@
});
});
- suite('gr-diff-comment draft tests', () => {
+ suite('gr-comment draft tests', () => {
let element;
let sandbox;
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 20a4a1e..3b91650 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -103,8 +103,6 @@
'core/gr-smart-search/gr-smart-search_test.html',
'diff/gr-comment-api/gr-comment-api_test.html',
'diff/gr-diff-builder/gr-diff-builder_test.html',
- 'diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html',
- 'diff/gr-diff-comment/gr-diff-comment_test.html',
'diff/gr-diff-cursor/gr-diff-cursor_test.html',
'diff/gr-diff-highlight/gr-annotation_test.html',
'diff/gr-diff-highlight/gr-diff-highlight_test.html',
@@ -157,6 +155,8 @@
'shared/gr-button/gr-button_test.html',
'shared/gr-change-star/gr-change-star_test.html',
'shared/gr-change-status/gr-change-status_test.html',
+ 'shared/gr-comment-thread/gr-comment-thread_test.html',
+ 'shared/gr-comment/gr-comment_test.html',
'shared/gr-copy-clipboard/gr-copy-clipboard_test.html',
'shared/gr-cursor-manager/gr-cursor-manager_test.html',
'shared/gr-date-formatter/gr-date-formatter_test.html',
