Add a --task--preview flag to the query command
This new flag takes arguments for a patchset. Tasks will then be
evaluated as if the patchset were already merged. Multiple changes
may be previewed together.
Change-Id: I87fae8a1bbb91706d170f2fda69b8cfbef84533b
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 061acb7..e50a308 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/Modules.java
@@ -20,6 +20,10 @@
import com.google.gerrit.server.restapi.change.QueryChanges;
import com.google.gerrit.sshd.commands.Query;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
+import java.util.ArrayList;
+import java.util.List;
import org.kohsuke.args4j.Option;
public class Modules {
@@ -52,5 +56,23 @@
@Option(name = "--applicable", usage = "Include only applicable tasks in the output")
public boolean onlyApplicable = false;
+
+ @Option(
+ name = "--preview",
+ metaVar = "{CHANGE,PATCHSET}",
+ usage = "list of patch sets to preview task evaluation for")
+ public void addPatchSet(String token) {
+ PatchSetArgument psa = patchSetArgumentFactory.createForArgument(token);
+ patchSetArguments.add(psa);
+ }
+
+ public List<PatchSetArgument> patchSetArguments = new ArrayList<>();
+
+ public PatchSetArgument.Factory patchSetArgumentFactory;
+
+ @Inject
+ public MyOptions(PatchSetArgument.Factory patchSetArgumentFactory) {
+ this.patchSetArgumentFactory = patchSetArgumentFactory;
+ }
}
}
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 a4af8b5..b55f861 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -30,6 +30,7 @@
import com.google.inject.Inject;
import com.googlesource.gerrit.plugins.task.TaskConfig.External;
import com.googlesource.gerrit.plugins.task.TaskConfig.Task;
+import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.Paths;
@@ -94,6 +95,9 @@
public PluginDefinedInfo create(ChangeData c, ChangeQueryProcessor qp, String plugin) {
options = (Modules.MyOptions) qp.getDynamicBean(plugin);
if (options.all || options.onlyApplicable) {
+ for (PatchSetArgument psa : options.patchSetArguments) {
+ taskFactory.masquerade(psa);
+ }
try {
return createWithExceptions(c);
} catch (OrmException e) {
@@ -137,9 +141,11 @@
throws OrmException {
try {
boolean applicable = match(c, def.applicable);
- if (!applicable && !def.isVisible && !options.onlyApplicable) {
- tasks.add(unknown());
- return;
+ if (!def.isVisible) {
+ if (!def.isTrusted || (!applicable && !options.onlyApplicable)) {
+ tasks.add(unknown());
+ return;
+ }
}
if (applicable || !options.onlyApplicable) {
@@ -171,7 +177,8 @@
List<TaskAttribute> subTasks = new ArrayList<>();
for (String file : parent.subTasksFiles) {
try {
- tasks.addAll(getTasks(parent.config.getBranch(), resolveTaskFileName(file)));
+ tasks.addAll(
+ getTasks(parent.config.getBranch(), resolveTaskFileName(file), parent.isTrusted));
} catch (ConfigInvalidException | IOException e) {
subTasks.add(invalid());
}
@@ -182,7 +189,7 @@
if (ext == null) {
subTasks.add(invalid());
} else {
- tasks.addAll(getTasks(ext));
+ tasks.addAll(getTasks(ext, parent.isTrusted));
}
} catch (ConfigInvalidException | IOException e) {
subTasks.add(invalid());
@@ -225,14 +232,15 @@
return tasks;
}
- protected List<Task> getTasks(External external)
+ protected List<Task> getTasks(External external, boolean isTrusted)
throws ConfigInvalidException, IOException, OrmException {
- return getTasks(resolveUserBranch(external.user), resolveTaskFileName(external.file));
+ return getTasks(
+ resolveUserBranch(external.user), resolveTaskFileName(external.file), isTrusted);
}
- protected List<Task> getTasks(Branch.NameKey branch, String file)
+ protected List<Task> getTasks(Branch.NameKey branch, String file, boolean isTrusted)
throws ConfigInvalidException, IOException {
- return taskFactory.getTaskConfig(branch, file).getTasks();
+ return taskFactory.getTaskConfig(branch, file, isTrusted).getTasks();
}
protected String resolveTaskFileName(String file) throws ConfigInvalidException {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
index 204a636..64add65 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfig.java
@@ -44,9 +44,11 @@
public List<String> subTasksFiles;
public boolean isVisible;
+ public boolean isTrusted;
- public Task(SubSection s, boolean isVisible) {
+ public Task(SubSection s, boolean isVisible, boolean isTrusted) {
this.isVisible = isVisible;
+ this.isTrusted = isTrusted;
applicable = getString(s, KEY_APPLICABLE, null);
fail = getString(s, KEY_FAIL, null);
failHint = getString(s, KEY_FAIL_HINT, null);
@@ -89,10 +91,12 @@
protected static final String KEY_USER = "user";
public boolean isVisible;
+ public boolean isTrusted;
- public TaskConfig(Branch.NameKey branch, String fileName, boolean isVisible) {
+ public TaskConfig(Branch.NameKey branch, String fileName, boolean isVisible, boolean isTrusted) {
super(branch, fileName);
this.isVisible = isVisible;
+ this.isTrusted = isTrusted;
}
public List<Task> getRootTasks() {
@@ -107,7 +111,7 @@
List<Task> tasks = new ArrayList<>();
// No need to get a task with no name (what would we call it?)
for (String task : cfg.getSubsections(type)) {
- tasks.add(new Task(new SubSection(type, task), isVisible));
+ tasks.add(new Task(new SubSection(type, task), isVisible, isTrusted));
}
return tasks;
}
@@ -122,7 +126,7 @@
}
public Task getTask(String name) {
- return new Task(new SubSection(SECTION_TASK, name), isVisible);
+ return new Task(new SubSection(SECTION_TASK, name), isVisible, isTrusted);
}
public External getExternal(String name) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
index 1d9be90..418df6e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskConfigFactory.java
@@ -24,7 +24,10 @@
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject;
+import com.googlesource.gerrit.plugins.task.cli.PatchSetArgument;
import java.io.IOException;
+import java.util.HashMap;
+import java.util.Map;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
@@ -42,6 +45,8 @@
protected final CurrentUser user;
protected final AllProjectsName allProjects;
+ protected final Map<Branch.NameKey, PatchSetArgument> psaMasquerades = new HashMap<>();
+
@Inject
protected TaskConfigFactory(
AllProjectsName allProjects,
@@ -55,17 +60,30 @@
}
public TaskConfig getRootConfig() throws ConfigInvalidException, IOException {
- return getTaskConfig(getRootBranch(), DEFAULT);
+ return getTaskConfig(getRootBranch(), DEFAULT, true);
+ }
+
+ public void masquerade(PatchSetArgument psa) {
+ psaMasquerades.put(psa.change.getDest(), psa);
}
protected Branch.NameKey getRootBranch() {
return new Branch.NameKey(allProjects, "refs/meta/config");
}
- public TaskConfig getTaskConfig(Branch.NameKey branch, String fileName)
+ public TaskConfig getTaskConfig(Branch.NameKey branch, String fileName, boolean isTrusted)
throws ConfigInvalidException, IOException {
- TaskConfig cfg = new TaskConfig(branch, fileName, canRead(branch));
+ PatchSetArgument psa = psaMasquerades.get(branch);
+ boolean visible = true; // invisible psas are filtered out by commandline
+ if (psa == null) {
+ visible = canRead(branch);
+ } else {
+ isTrusted = false;
+ branch = new Branch.NameKey(psa.change.getProject(), psa.patchSet.getId().toRefName());
+ }
+
Project.NameKey project = branch.getParentKey();
+ TaskConfig cfg = new TaskConfig(branch, fileName, visible, isTrusted);
try (Repository git = gitMgr.openRepository(project)) {
cfg.load(project, git);
} catch (IOException e) {
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
new file mode 100644
index 0000000..a7e9063
--- /dev/null
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/cli/PatchSetArgument.java
@@ -0,0 +1,106 @@
+// 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.googlesource.gerrit.plugins.task.cli;
+
+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;
+import com.google.gerrit.server.permissions.ChangePermission;
+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;
+
+ @Inject
+ 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;
+ }
+
+ public PatchSetArgument createForArgument(String token) {
+ 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));
+ } catch (PermissionBackendException | AuthException e) {
+ throw new IllegalArgumentException("database error", e);
+ } catch (UnloggedFailure e) {
+ throw new IllegalArgumentException(e.getMessage(), e);
+ } catch (OrmException e) {
+ throw new IllegalArgumentException("database error", e);
+ }
+ }
+
+ protected PatchSet.Id parsePatchSet(String patchIdentity) throws UnloggedFailure, OrmException {
+ // By older style change,patchset
+ if (patchIdentity.matches("^[1-9][0-9]*,[1-9][0-9]*$")) {
+ try {
+ return PatchSet.Id.parse(patchIdentity);
+ } catch (IllegalArgumentException e) {
+ throw error("\"" + patchIdentity + "\" is not a valid patch set");
+ }
+ }
+ throw error("\"" + patchIdentity + "\" is not a valid patch set");
+ }
+
+ protected String noSuchPatchSet(String patchIdentity) {
+ return "\"" + patchIdentity + "\" no such patch set";
+ }
+
+ protected static UnloggedFailure error(final String msg) {
+ return new UnloggedFailure(1, msg);
+ }
+ }
+
+ public final PatchSet patchSet;
+ public final Change change;
+
+ public PatchSetArgument(Change change, PatchSet patchSet) {
+ this.patchSet = patchSet;
+ this.change = change;
+ }
+
+ public void ensureLatest() {
+ if (!change.currentPatchSetId().equals(patchSet.getId())) {
+ throw new IllegalArgumentException(patchSet + " is not the latest patch set");
+ }
+ }
+}
diff --git a/src/main/resources/Documentation/task.md b/src/main/resources/Documentation/task.md
index e4c2abf..13095d8 100644
--- a/src/main/resources/Documentation/task.md
+++ b/src/main/resources/Documentation/task.md
@@ -265,7 +265,7 @@
Change Query Output
-------------------
It is possible to add a task section to the query output of changes using
-the task plugin switches. The following switches are available:
+the task plugin switches. The following switches are available:
**\-\-@PLUGIN@\-\-applicable**
@@ -275,10 +275,17 @@
**\-\-@PLUGIN@\-\-all**
This switch is meant as a debug switch, it outputs all tasks visible to the
-calling user, whether they apply to a change or not. When this flag is used,
+calling user, whether they apply to a change or not. When this flag is used,
an additional 'applicable' property is included in each task output to
indicate whether the task actually met its applicability criteria or not.
+**\-\-@PLUGIN@\-\-preview**
+
+This switch is meant as a debug switch for previewing changes to task configs.
+This switch outputs tasks as if the supplied change were already merged. This
+makes it possible to preview the effect of proposed changes before going live
+with them. Multiple changes may be previewed together.
+
When tasks are appended to changes, they will have a "task" section under
the plugins section like below:
diff --git a/src/main/resources/Documentation/task_states.md b/src/main/resources/Documentation/task_states.md
index 953693a..f417b6d 100644
--- a/src/main/resources/Documentation/task_states.md
+++ b/src/main/resources/Documentation/task_states.md
@@ -139,7 +139,7 @@
applicable = NOT is:open
[external "user special"]
- user = current-user
+ user = testuser
file = special.config
[external "user missing"]
@@ -147,7 +147,7 @@
file = special.config
[external "file missing"]
- user = current-user
+ user = testuser
file = missing
```
diff --git a/test/check_task_statuses.sh b/test/check_task_statuses.sh
index 410603e..3e90f83 100755
--- a/test/check_task_statuses.sh
+++ b/test/check_task_statuses.sh
@@ -18,13 +18,30 @@
# --------
+q() { "$@" > /dev/null 2>&1 ; } # cmd [args...] # quiet a command
+
# Run a test setup command quietly, exit on failure
q_setup() { # cmd [args...]
local out ; out=$("$@" 2>&1) || { echo "$out" ; exit ; }
}
+replace_user() { # < text_with_testuser > text_with_$USER
+ sed -e"s/testuser/$USER/"
+}
+
example() { # example_num
- awk '/```/{Q++;E=(Q+1)/2};E=='"$1" < "$DOC_STATES" | grep -v '```'
+ awk '/```/{Q++;E=(Q+1)/2};E=='"$1" < "$DOC_STATES" | grep -v '```' | replace_user
+}
+
+get_change_num() { # < gerrit_push_response > changenum
+ local url=$(awk '/New Changes:/ { getline; print $2 }')
+ echo "${url##*\/}" | tr -d -c '[:digit:]'
+}
+
+install_changeid_hook() { # repo
+ local hook=$(git rev-parse --git-dir)/hooks/commit-msg
+ scp -p -P "$PORT" "$SERVER":hooks/commit-msg "$hook"
+ chmod +x "$hook"
}
setup_repo() { # repo remote ref
@@ -32,6 +49,7 @@
git init "$repo"
(
cd "$repo"
+ install_changeid_hook "$repo"
git fetch "$remote" "$ref"
git checkout FETCH_HEAD
)
@@ -47,6 +65,16 @@
)
}
+create_repo_change() { # repo remote ref > change_num
+ local repo=$1 remote=$2 ref=$3
+ (
+ q cd "$repo"
+ q git add .
+ q git commit -m 'Testing task plugin'
+ git push "$remote" HEAD:"refs/for/$ref" 2>&1 | get_change_num
+ )
+}
+
query() { # query
ssh -x -p "$PORT" "$SERVER" gerrit query "$@" \
--format json | head -1 | python -c "import sys, json; \
@@ -120,3 +148,7 @@
query="status:open limit:1"
test_tasks statuses "$EXPECTED" --task--applicable "$query"
test_file all --task--all "$query"
+
+replace_user < "$MYDIR"/root.change > "$ROOT_CFG"
+cnum=$(create_repo_change "$ALL" "$REMOTE_ALL" "$REF_ALL")
+test_file preview --task--preview "$cnum,1" --task--all "$query"
diff --git a/test/preview b/test/preview
new file mode 100644
index 0000000..8effb1a
--- /dev/null
+++ b/test/preview
@@ -0,0 +1,177 @@
+ "plugins" : [
+ {
+ "name" : "task",
+ "roots" : [
+ {
+ "applicable" : true,
+ "name" : "INVALIDS",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "No PASS criteria",
+ "status" : "INVALID"
+ },
+ {
+ "applicable" : true,
+ "name" : "WAITING (subtask INVALID)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "WAITING (subtask missing)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "MISSING",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Grouping WAITING (subtask INVALID)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Grouping WAITING (subtask missing)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "MISSING",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Root PASS",
+ "status" : "PASS"
+ },
+ {
+ "applicable" : true,
+ "hint" : "You must now run the ready task",
+ "name" : "Root READY (subtask PASS)",
+ "status" : "READY",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "Subtask PASS",
+ "status" : "PASS"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Subtasks External",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "userfile task/special.config PASS",
+ "status" : "PASS"
+ },
+ {
+ "applicable" : true,
+ "name" : "userfile task/special.config FAIL",
+ "status" : "FAIL"
+ }
+ ]
+ },
+ {
+ "applicable" : false,
+ "name" : "Root NA Pass",
+ "status" : "PASS"
+ },
+ {
+ "applicable" : false,
+ "name" : "NA INVALIDS",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "No PASS criteria",
+ "status" : "INVALID"
+ },
+ {
+ "applicable" : true,
+ "name" : "WAITING (subtask INVALID)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "WAITING (subtask missing)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "MISSING",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Grouping WAITING (subtask INVALID)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Grouping WAITING (subtask missing)",
+ "status" : "WAITING",
+ "subTasks" : [
+ {
+ "applicable" : true,
+ "name" : "MISSING",
+ "status" : "INVALID"
+ }
+ ]
+ },
+ {
+ "applicable" : true,
+ "name" : "Subtask INVALID",
+ "status" : "INVALID"
+ }
+ ]
+ }
+ ]
+ }
+ ],
diff --git a/test/root.change b/test/root.change
new file mode 100644
index 0000000..01bf55b
--- /dev/null
+++ b/test/root.change
@@ -0,0 +1,33 @@
+[root "INVALIDS"]
+ applicable = is:open
+ subtasks-file = invalids.config
+
+[root "Root PASS"]
+ applicable = is:open
+ pass = True
+
+[root "Root READY (subtask PASS)"]
+ applicable = is:open
+ pass = -is:open
+ subtask = Subtask PASS
+ ready-hint = You must now run the ready task
+
+[root "Subtasks External"]
+ applicable = is:open
+ subtasks-external = user special
+
+[root "Root NA Pass"]
+ applicable = -is:open
+ pass = True
+
+[root "NA INVALIDS"]
+ applicable = -is:open
+ subtasks-file = invalids.config
+
+[task "Subtask PASS"]
+ applicable = is:open
+ pass = is:open
+
+[external "user special"]
+ user = testuser
+ file = special.config
