Merge branch 'stable-2.16' into stable-3.0
* stable-2.16: (49 commits)
Add change number to task output
Only copy required folders for testing
Add missing copyright/license header
Use a bash associative array to speedup task tests
plugins/task: Move TaskExpression iteration to Preloader
Add bazel rule for junit tests
Introduce TaskKey, SubSectionKey and FileKey
plugins/task: Fix cache name in Preloader
Only reload nodes when needed
Skip re-expanding properties for local properties
Use a lazy loading task property expansion model
Refresh TaskTree.Nodes when getting them
Cache preloaded tasks
Rename a bunch of TaskTree addNode() methods
Add preload-task to external file tests
Add Root Preload tasks-factory test
Create a TaskExpression with unit tests
Do not modify definition during preload
Return Optional<Task> in Task.Config
Make task config collection fields immutable
...
Change-Id: I4908973e957bba36f81ac2d7c836320faba18b35
diff --git a/.bazelversion b/.bazelversion
index fd2a018..7c69a55 100644
--- a/.bazelversion
+++ b/.bazelversion
@@ -1 +1 @@
-3.1.0
+3.7.0
diff --git a/BUILD b/BUILD
index dbe73f9..39b3d0d 100644
--- a/BUILD
+++ b/BUILD
@@ -37,8 +37,6 @@
"Implementation-Title: Task Plugin",
"Implementation-URL: https://gerrit-review.googlesource.com/#/admin/projects/plugins/" + plugin_name,
"Gerrit-Module: com.googlesource.gerrit.plugins.task.Modules$Module",
- "Gerrit-SshModule: com.googlesource.gerrit.plugins.task.Modules$SshModule",
- "Gerrit-HttpModule: com.googlesource.gerrit.plugins.task.Modules$HttpModule",
],
resource_jars = [":gr-task-plugin-static"],
resources = glob(["src/main/resources/**/*"]),
diff --git a/WORKSPACE b/WORKSPACE
index b254afa..0c9c56f 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -3,7 +3,7 @@
load("//:bazlets.bzl", "load_bazlets")
load_bazlets(
- commit = "0ccc066431ad7e88a5cd9e06000ce677de1116ee",
+ commit = "a88174652e6f853ead5bbc5dacc1030dbb2d50c3",
#local_path = "/home/<user>/projects/bazlets",
)
@@ -51,20 +51,11 @@
sha1 = "eff48ed53995db2dadf0456426cc1f8700136f86",
)
-# Release Plugin API
+# Load plugin API
load(
"@com_googlesource_gerrit_bazlets//:gerrit_api.bzl",
"gerrit_api",
)
-# Snapshot Plugin API
-#load(
-# "@com_googlesource_gerrit_bazlets//:gerrit_api_maven_local.bzl",
-# "gerrit_api_maven_local",
-#)
-
-# Load release Plugin API
+# Release Plugin API
gerrit_api()
-
-# Load snapshot Plugin API
-#gerrit_api_maven_local()
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java b/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
index 53c41e1..45fe46d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/MatchCache.java
@@ -14,9 +14,9 @@
package com.googlesource.gerrit.plugins.task;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gwtorm.server.OrmException;
import java.util.HashMap;
import java.util.Map;
@@ -31,7 +31,7 @@
this.changeData = changeData;
}
- protected boolean match(String query) throws OrmException, QueryParseException {
+ protected boolean match(String query) throws StorageException, QueryParseException {
if (query == null) {
return true;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java b/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
index 6346a73..11786e6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
@@ -19,7 +19,8 @@
import com.google.gerrit.extensions.webui.JavaScriptPlugin;
import com.google.gerrit.extensions.webui.WebUiPlugin;
import com.google.gerrit.server.DynamicOptions.DynamicBean;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
+import com.google.gerrit.server.restapi.change.GetChange;
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.gerrit.sshd.commands.Query;
import com.google.inject.AbstractModule;
@@ -36,19 +37,9 @@
bind(ChangeAttributeFactory.class)
.annotatedWith(Exports.named("task"))
.to(TaskAttributeFactory.class);
- }
- }
- public static class SshModule extends AbstractModule {
- @Override
- protected void configure() {
+ bind(DynamicBean.class).annotatedWith(Exports.named(GetChange.class)).to(MyOptions.class);
bind(DynamicBean.class).annotatedWith(Exports.named(Query.class)).to(MyOptions.class);
- }
- }
-
- public static class HttpModule extends AbstractModule {
- @Override
- protected void configure() {
bind(DynamicBean.class).annotatedWith(Exports.named(QueryChanges.class)).to(MyOptions.class);
DynamicSet.bind(binder(), WebUiPlugin.class)
.toInstance(new JavaScriptPlugin("gr-task-plugin.html"));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java b/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
index aa3da13..7896417 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/PredicateCache.java
@@ -14,12 +14,12 @@
package com.googlesource.gerrit.plugins.task;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.util.HashMap;
import java.util.Map;
@@ -37,7 +37,7 @@
this.cqb = cqb;
}
- public boolean match(ChangeData c, String query) throws OrmException, QueryParseException {
+ public boolean match(ChangeData c, String query) throws StorageException, QueryParseException {
if (query == null) {
return true;
}
@@ -48,14 +48,14 @@
if (query != null) {
try {
return matchWithExceptions(c, query);
- } catch (OrmException | QueryParseException | RuntimeException e) {
+ } catch (QueryParseException | RuntimeException e) {
}
}
return null;
}
protected boolean matchWithExceptions(ChangeData c, String query)
- throws QueryParseException, OrmException {
+ throws QueryParseException, StorageException {
if ("true".equalsIgnoreCase(query)) {
return true;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
index 57c12ee..5f9a8d5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Properties.java
@@ -15,9 +15,9 @@
package com.googlesource.gerrit.plugins.task;
import com.google.common.collect.Sets;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gwtorm.server.OrmException;
import com.googlesource.gerrit.plugins.task.TaskConfig.NamesFactory;
import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
import java.lang.reflect.Field;
@@ -57,7 +57,7 @@
}
/** Use to expand properties specifically for Tasks. */
- public Task getTask(ChangeData changeData) throws OrmException {
+ public Task getTask(ChangeData changeData) throws StorageException {
if (loader != null && loader.isNonTaskDefinedPropertyLoaded()) {
// To detect NamesFactories dependent on non task defined properties, the checking must be
// done after subnodes are fully loaded, which unfortunately happens after getTask() is
@@ -141,14 +141,14 @@
if (changeProp != name) {
try {
return change(changeProp);
- } catch (OrmException e) {
+ } catch (StorageException e) {
throw new RuntimeException(e);
}
}
return "";
}
- protected String change(String changeProp) throws OrmException {
+ protected String change(String changeProp) throws StorageException {
switch (changeProp) {
case "number":
return String.valueOf(change().getId().get());
@@ -171,7 +171,7 @@
if (change == null) {
try {
change = changeData.change();
- } catch (OrmException e) {
+ } catch (StorageException e) {
throw new RuntimeException(e);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
index 41f87d2..08add9f 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -15,12 +15,12 @@
package com.googlesource.gerrit.plugins.task;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.common.PluginDefinedInfo;
import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.DynamicOptions.BeanProvider;
+import com.google.gerrit.server.change.ChangeAttributeFactory;
import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor;
-import com.google.gerrit.server.query.change.ChangeQueryProcessor.ChangeAttributeFactory;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
import com.googlesource.gerrit.plugins.task.TaskTree.Node;
@@ -77,8 +77,8 @@
}
@Override
- public PluginDefinedInfo create(ChangeData c, ChangeQueryProcessor qp, String plugin) {
- options = (Modules.MyOptions) qp.getDynamicBean(plugin);
+ public PluginDefinedInfo create(ChangeData c, BeanProvider beanProvider, String plugin) {
+ options = (Modules.MyOptions) beanProvider.getDynamicBean(plugin);
if (options.all || options.onlyApplicable || options.onlyInvalid) {
for (PatchSetArgument psa : options.patchSetArguments) {
definitions.masquerade(psa);
@@ -98,7 +98,7 @@
new AttributeFactory(node).create().ifPresent(t -> a.roots.add(t));
}
}
- } catch (ConfigInvalidException | IOException | OrmException e) {
+ } catch (ConfigInvalidException | IOException | StorageException e) {
a.roots.add(invalid());
}
@@ -172,7 +172,6 @@
}
} catch (ConfigInvalidException
| IOException
- | OrmException
| QueryParseException
| RuntimeException e) {
return Optional.of(invalid()); // bad applicability query
@@ -180,7 +179,7 @@
return Optional.empty();
}
- protected Status getStatusWithExceptions() throws OrmException, QueryParseException {
+ protected Status getStatusWithExceptions() throws StorageException, QueryParseException {
if (isAllNull(task.pass, task.fail, attribute.subTasks)) {
// A leaf def has no defined subdefs.
boolean hasDefinedSubtasks =
@@ -244,13 +243,13 @@
protected Status getStatus() {
try {
return getStatusWithExceptions();
- } catch (OrmException | QueryParseException | RuntimeException e) {
+ } catch (QueryParseException | RuntimeException e) {
return Status.INVALID;
}
}
protected List<TaskAttribute> getSubTasks()
- throws ConfigInvalidException, IOException, OrmException {
+ throws ConfigInvalidException, IOException, StorageException {
List<TaskAttribute> subTasks = new ArrayList<>();
for (Node subNode : node.getSubNodes()) {
if (subNode == null) {
@@ -275,7 +274,7 @@
matchCache.match(task.fail);
matchCache.match(task.pass);
return true;
- } catch (OrmException | QueryParseException | RuntimeException e) {
+ } catch (QueryParseException | RuntimeException e) {
return false;
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
index 5746c10..2bd8cf6 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskTree.java
@@ -15,6 +15,8 @@
package com.googlesource.gerrit.plugins.task;
import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Branch;
@@ -26,7 +28,6 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeQueryProcessor;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.googlesource.gerrit.plugins.task.TaskConfig.External;
@@ -101,7 +102,7 @@
}
public List<Node> getRootNodes(ChangeData changeData)
- throws ConfigInvalidException, IOException, OrmException {
+ throws ConfigInvalidException, IOException, StorageException {
this.changeData = changeData;
root.path = Collections.emptyList();
return root.getSubNodes();
@@ -114,22 +115,22 @@
protected List<Node> nodes;
protected Set<String> names = new HashSet<>();
- protected void addSubNodes() throws ConfigInvalidException, IOException, OrmException {
+ protected void addSubNodes() throws ConfigInvalidException, IOException, StorageException {
addPreloaded(preloader.getRootTasks(taskFactory.getRootConfig()));
}
- protected void addPreloaded(List<Task> defs) throws ConfigInvalidException, OrmException {
+ protected void addPreloaded(List<Task> defs) throws ConfigInvalidException, StorageException {
for (Task def : defs) {
addPreloaded(def);
}
}
- protected void addPreloaded(Task def) throws ConfigInvalidException, OrmException {
+ protected void addPreloaded(Task def) throws ConfigInvalidException, StorageException {
addPreloaded(def, (parent, definition) -> new Node(parent, definition));
}
protected void addPreloaded(Task def, NodeFactory nodeFactory)
- throws ConfigInvalidException, OrmException {
+ throws ConfigInvalidException, StorageException {
if (def != null) {
try {
Node node = cachedNodeByTask.get(def.key());
@@ -157,7 +158,7 @@
nodes.add(null); // null node indicates invalid
}
- protected List<Node> getSubNodes() throws ConfigInvalidException, IOException, OrmException {
+ protected List<Node> getSubNodes() throws ConfigInvalidException, IOException, StorageException {
if (nodes == null) {
nodes = new ArrayList<>();
addSubNodes();
@@ -167,7 +168,7 @@
return nodes;
}
- public void refreshSubNodes() throws ConfigInvalidException, OrmException {
+ public void refreshSubNodes() throws ConfigInvalidException, StorageException {
if (nodes != null) {
for (Node node : nodes) {
if (node != null) {
@@ -192,7 +193,7 @@
protected final Properties properties;
protected final TaskKey taskKey;
- public Node(NodeList parent, Task task) throws ConfigInvalidException, OrmException {
+ public Node(NodeList parent, Task task) throws ConfigInvalidException, StorageException {
this.parent = parent;
taskKey = task.key();
properties = new Properties(task, parent.getProperties());
@@ -206,7 +207,7 @@
/* The task needs to be refreshed before a node is used, however
subNode refreshing can wait until they are fetched since they may
not be needed. */
- public void refreshTask() throws ConfigInvalidException, OrmException {
+ public void refreshTask() throws ConfigInvalidException, StorageException {
this.path = new LinkedList<>(parent.path);
this.path.add(key());
@@ -221,14 +222,14 @@
}
@Override
- protected void addSubNodes() throws ConfigInvalidException, OrmException {
+ protected void addSubNodes() throws ConfigInvalidException, StorageException {
addSubTasks();
addSubTasksFactoryTasks();
addSubTasksFiles();
addSubTasksExternals();
}
- protected void addSubTasks() throws ConfigInvalidException, OrmException {
+ protected void addSubTasks() throws ConfigInvalidException, StorageException {
for (String expression : task.subTasks) {
try {
Optional<Task> def =
@@ -242,7 +243,7 @@
}
}
- protected void addSubTasksFiles() throws ConfigInvalidException, OrmException {
+ protected void addSubTasksFiles() throws ConfigInvalidException, StorageException {
for (String file : task.subTasksFiles) {
try {
addPreloaded(
@@ -253,7 +254,7 @@
}
}
- protected void addSubTasksExternals() throws ConfigInvalidException, OrmException {
+ protected void addSubTasksExternals() throws ConfigInvalidException, StorageException {
for (String external : task.subTasksExternals) {
try {
External ext = task.config.getExternal(external);
@@ -268,7 +269,7 @@
}
}
- protected void addSubTasksFactoryTasks() throws ConfigInvalidException, OrmException {
+ protected void addSubTasksFactoryTasks() throws ConfigInvalidException, StorageException {
for (String tasksFactoryName : task.subTasksFactories) {
TasksFactory tasksFactory = task.config.getTasksFactory(tasksFactoryName);
if (tasksFactory != null) {
@@ -290,14 +291,14 @@
}
protected void addStaticTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
- throws ConfigInvalidException, OrmException {
+ throws ConfigInvalidException, StorageException {
for (String name : namesFactory.names) {
addPreloaded(preloader.preload(task.config.new Task(tasksFactory, name)));
}
}
protected void addChangeTypeTasks(TasksFactory tasksFactory, NamesFactory namesFactory)
- throws ConfigInvalidException, OrmException {
+ throws ConfigInvalidException, StorageException {
try {
if (namesFactory.changes != null) {
List<ChangeData> changeDataList =
@@ -324,7 +325,7 @@
}
return;
}
- } catch (OrmException e) {
+ } catch (StorageException e) {
log.atSevere().withCause(e).log("ERROR: running changes query: " + namesFactory.changes);
} catch (QueryParseException e) {
}
@@ -332,7 +333,7 @@
}
protected List<Task> getPreloadedTasks(External external)
- throws ConfigInvalidException, IOException, OrmException {
+ throws ConfigInvalidException, IOException, StorageException {
return getPreloadedTasks(
FileKey.create(resolveUserBranch(external.user), resolveTaskFileName(external.file)));
}
@@ -364,14 +365,16 @@
}
protected Branch.NameKey resolveUserBranch(String user)
- throws ConfigInvalidException, IOException, OrmException {
+ throws ConfigInvalidException, IOException, StorageException {
if (user == null) {
throw new ConfigInvalidException("External user not defined");
}
- Account acct = accountResolver.find(user);
- if (acct == null) {
+ Account.Id acct;
+ try {
+ acct = accountResolver.resolve(user).asUnique().getAccount().getId();
+ } catch (UnprocessableEntityException e) {
throw new ConfigInvalidException("Cannot resolve user: " + user);
}
- return new Branch.NameKey(allUsers.get(), RefNames.refsUsers(acct.getId()));
+ return new Branch.NameKey(allUsers.get(), RefNames.refsUsers(acct));
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/cli/PatchSetArgument.java b/src/main/java/com/googlesource/gerrit/plugins/task/cli/PatchSetArgument.java
index bcbb972..9fbddeb 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/cli/PatchSetArgument.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/cli/PatchSetArgument.java
@@ -13,10 +13,10 @@
// limitations under the License.
package com.googlesource.gerrit.plugins.task.cli;
+import com.google.gerrit.exceptions.StorageException;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.notedb.ChangeNotes;
@@ -24,14 +24,12 @@
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.sshd.BaseCommand.UnloggedFailure;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
public class PatchSetArgument {
public static class Factory {
protected final PermissionBackend permissionBackend;
protected final ChangeNotes.Factory notesFactory;
- protected final ReviewDb reviewDb;
protected final PatchSetUtil psUtil;
protected final CurrentUser user;
@@ -39,12 +37,10 @@
protected Factory(
ChangeNotes.Factory notesFactory,
PermissionBackend permissionBackend,
- ReviewDb reviewDb,
PatchSetUtil psUtil,
CurrentUser user) {
this.notesFactory = notesFactory;
this.permissionBackend = permissionBackend;
- this.reviewDb = reviewDb;
this.psUtil = psUtil;
this.user = user;
}
@@ -53,18 +49,13 @@
try {
PatchSet.Id patchSetId = parsePatchSet(token);
ChangeNotes changeNotes = notesFactory.createChecked(patchSetId.getParentKey());
- permissionBackend
- .user(user)
- .database(reviewDb)
- .change(changeNotes)
- .check(ChangePermission.READ);
- return new PatchSetArgument(
- changeNotes.getChange(), psUtil.get(reviewDb, changeNotes, patchSetId));
+ permissionBackend.user(user).change(changeNotes).check(ChangePermission.READ);
+ return new PatchSetArgument(changeNotes.getChange(), psUtil.get(changeNotes, patchSetId));
} catch (PermissionBackendException | AuthException e) {
throw new IllegalArgumentException("database error", e);
} catch (UnloggedFailure e) {
throw new IllegalArgumentException(e.getMessage(), e);
- } catch (OrmException e) {
+ } catch (StorageException e) {
throw new IllegalArgumentException("database error", e);
}
}
diff --git a/test/check_task_statuses.sh b/test/check_task_statuses.sh
index a28a15d..1d3efbd 100755
--- a/test/check_task_statuses.sh
+++ b/test/check_task_statuses.sh
@@ -223,7 +223,7 @@
}
get_change_num() { # < gerrit_push_response > changenum
- local url=$(awk '/New Changes:/ { getline; print $2 }')
+ local url=$(awk '$NF ~ /\[NEW\]/ { print $2 }')
echo "${url##*\/}" | tr -d -c '[:digit:]'
}
diff --git a/test/docker/gerrit/Dockerfile b/test/docker/gerrit/Dockerfile
index 059f3c0..b9d1715 100755
--- a/test/docker/gerrit/Dockerfile
+++ b/test/docker/gerrit/Dockerfile
@@ -1,4 +1,4 @@
-FROM gerritcodereview/gerrit:2.16.27-ubuntu16
+FROM gerritcodereview/gerrit:3.0.16-ubuntu18
USER root
