show task name in case of bad applicability query
Before the change, 'UNKNOWN' is shown as task name when task is invalid
due to bad applicability query, which makes it difficult for users to
identify the misconfigured task. Update logic to output valid task name
instead of 'UNKNOWN' in case of bad applicability query in task config,
if the user has read permission on it. Add tests for the aforementioned
case.
Change-Id: Ibde48bff31ca95f94b95c77880bb3872cf3f0b77
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 d9d4039..5affd56 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/task/TaskAttributeFactory.java
@@ -174,7 +174,12 @@
attribute.evaluationMilliSeconds = millis();
}
- boolean applicable = node.match(task.applicable);
+ boolean applicable;
+ try {
+ applicable = node.match(task.applicable);
+ } catch (QueryParseException e) {
+ return Optional.of(invalid());
+ }
if (!task.isVisible) {
if (!node.isTrusted() || (!applicable && !options.onlyApplicable)) {
return Optional.of(unknown());
@@ -218,12 +223,20 @@
}
}
}
- } catch (IOException | QueryParseException | RuntimeException e) {
+ } catch (IOException | RuntimeException e) {
return Optional.of(invalid()); // bad applicability query
}
return Optional.empty();
}
+ protected TaskAttribute invalid() {
+ TaskAttribute invalid = TaskAttributeFactory.invalid();
+ if (task.isVisible) {
+ invalid.name = task.name();
+ }
+ return invalid;
+ }
+
public void addStatistics(TaskAttribute.Statistics statistics) {
if (statistics != null) {
statistics.isApplicableRefreshRequired = node.properties.isApplicableRefreshRequired();
@@ -316,7 +329,7 @@
for (Node subNode :
options.onlyApplicable ? node.getApplicableSubNodes() : node.getSubNodes()) {
if (subNode instanceof Node.Invalid) {
- subTasks.add(invalid());
+ subTasks.add(TaskAttributeFactory.invalid());
} else {
new AttributeFactory(subNode).create().ifPresent(t -> subTasks.add(t));
}
diff --git a/src/main/resources/Documentation/test/preview.md b/src/main/resources/Documentation/test/preview.md
index 55c26eb..ec5967f 100644
--- a/src/main/resources/Documentation/test/preview.md
+++ b/src/main/resources/Documentation/test/preview.md
@@ -114,6 +114,10 @@
]
},
{
+ "name" : "Bad APPLICABLE query",
+ "status" : "INVALID"
+ },
+ {
"applicable" : false,
"hasPass" : true,
"name" : "NA Bad PASS query",
@@ -470,6 +474,10 @@
]
},
{
+ "name" : "Bad APPLICABLE query",
+ "status" : "INVALID"
+ },
+ {
"applicable" : false,
"hasPass" : true,
"name" : "NA Bad PASS query",
diff --git a/src/main/resources/Documentation/test/task_states.md b/src/main/resources/Documentation/test/task_states.md
index c5881c6..286af97 100644
--- a/src/main/resources/Documentation/test/task_states.md
+++ b/src/main/resources/Documentation/test/task_states.md
@@ -2278,6 +2278,11 @@
]
},
{
+ "name" : "Bad APPLICABLE query", # Only Test Suite: visible
+ "name" : "UNKNOWN", # Only Test Suite: !visible
+ "status" : "INVALID"
+ },
+ {
"applicable" : false,
"hasPass" : true,
"name" : "NA Bad PASS query",
@@ -2569,6 +2574,11 @@
]
},
{
+ "name" : "Bad APPLICABLE query", # Only Test Suite: visible
+ "name" : "UNKNOWN", # Only Test Suite: !visible
+ "status" : "INVALID"
+ },
+ {
"applicable" : false,
"hasPass" : true,
"name" : "NA Bad PASS query",
@@ -2769,6 +2779,10 @@
pass = True
subtask =
+[task "Bad APPLICABLE query"]
+ applicable = bad:query
+ fail = True
+
[task "NA Bad PASS query"]
applicable = NOT is:open # Assumes test query is "is:open"
fail = True
diff --git a/test/check_task_statuses.sh b/test/check_task_statuses.sh
index 89722a3..1880432 100755
--- a/test/check_task_statuses.sh
+++ b/test/check_task_statuses.sh
@@ -316,11 +316,14 @@
cat <<-EOF
Usage:
"$MYPROG" --server <gerrit_host> --non-secret-user <non-secret user>
+ --untrusted-user <untrusted user>
--help|-h help text
--server|-s gerrit host
--non-secret-user user who don't have permission
to view other user refs.
+ --untrusted-user user who doesn't have permission
+ to view refs/meta/config ref on All-Projects repo
EOF
[ -n "$1" ] && { echo "Error: $1" ; exit 1 ; }
@@ -355,6 +358,7 @@
--help|-h) usage ;;
--server|-s) shift ; SERVER=$1 ;;
--non-secret-user) shift ; NON_SECRET_USER=$1 ;;
+ --untrusted-user) shift ; UNTRUSTED_USER=$1 ;;
*) usage "invalid argument $1" ;;
esac
shift
@@ -362,6 +366,7 @@
[ -z "$SERVER" ] && usage "You must specify --server"
[ -z "$NON_SECRET_USER" ] && usage "You must specify --non-secret-user"
+[ -z "$UNTRUSTED_USER" ] && usage "You must specify --untrusted-user"
PORT=29418
@@ -423,17 +428,26 @@
"NEW" \
"")
-no_all_json=$(echo "$all_pjson" | remove_suites all)
-no_all2_json=$(echo "$all2_pjson" | remove_suites all)
+no_all_visible_json=$(echo "$all_pjson" | remove_suites "all" "!visible")
+no_all_no_visible_json=$(echo "$all_pjson" | remove_suites "all" "visible")
+no_all_visible2_json=$(echo "$all2_pjson" | remove_suites "all" "!visible")
+no_all_no_visible2_json=$(echo "$all2_pjson" | remove_suites "all" "visible")
-echo "$no_all_json" | strip_non_applicable | \
+echo "$no_all_visible_json" | strip_non_applicable | \
grep -v "\"applicable\" :" > "$EXPECTED".applicable
-echo "$no_all2_json" | strip_non_applicable | \
+
+echo "$no_all_no_visible_json" | strip_non_applicable | \
+ grep -v "\"applicable\" :" > "$EXPECTED".applicable-visibility
+
+echo "$no_all_visible2_json" | strip_non_applicable | \
grep -v "\"applicable\" :" > "$EXPECTED".applicable2
-echo "$all_pjson" | remove_not_suite all | ensure json_pp > "$EXPECTED".all
+echo "$no_all_no_visible2_json" | strip_non_applicable | \
+ grep -v "\"applicable\" :" > "$EXPECTED".applicable-visibility2
-echo "$no_all_json" | strip_non_invalid > "$EXPECTED".invalid
+echo "$all_pjson" | remove_suites "!all" "!visible" | ensure json_pp > "$EXPECTED".all
+
+echo "$no_all_visible_json" | strip_non_invalid > "$EXPECTED".invalid
strip_non_invalid < "$EXPECTED".applicable > "$EXPECTED".invalid-applicable
@@ -454,6 +468,7 @@
RESULT=0
query="(change:$change3_number OR change:$change4_number) status:open"
test_2generated applicable --task--applicable "$query"
+test_2generated applicable-visibility -l "$UNTRUSTED_USER" --task--applicable "$query"
test_generated all --task--all "$query"
test_generated invalid --task--invalid "$query"
diff --git a/test/docker/run_tests/create-one-time-test-data.sh b/test/docker/run_tests/create-one-time-test-data.sh
index fc841c4..9d9fcbd 100755
--- a/test/docker/run_tests/create-one-time-test-data.sh
+++ b/test/docker/run_tests/create-one-time-test-data.sh
@@ -6,21 +6,30 @@
gssh() { ssh -x -p "$SSH_PORT" "$GERRIT_HOST" gerrit "$@" ; } # run a gerrit ssh command
-create_test_user() {
- echo "Creating test user ..."
+create_test_users_and_group() {
+ echo "Creating test users and group ..."
gssh create-account "$NON_SECRET_USER" --full-name "$NON_SECRET_USER" \
- --email "$NON_SECRET_USER"@example.com --ssh-key - < ~/.ssh/id_rsa.pub
+ --email "$NON_SECRET_USER"@example.com --ssh-key - < ~/.ssh/id_rsa.pub
+
+ gssh create-account "$UNTRUSTED_USER" --full-name "$UNTRUSTED_USER" \
+ --email "$UNTRUSTED_USER"@example.com --ssh-key - < ~/.ssh/id_rsa.pub
+
+ gssh create-group "Visible-All-Projects-Config" --member "$NON_SECRET_USER"
}
setup_all_projects_repo() {
echo "Updating All-Projects repo ..."
+ local uuid=$(gssh ls-groups -v | awk '-F\t' '$1 == "Visible-All-Projects-Config" {print $2}')
( cd "$WORKSPACE"
q git clone ssh://"$GERRIT_HOST":"$SSH_PORT"/All-Projects allProjects
cd allProjects
q git fetch origin refs/meta/config ; q git checkout FETCH_HEAD
- git config -f "project.config" --add access."refs/meta/config".read "group Registered Users"
- git config -f "project.config" --add capability.viewTaskPaths "group Administrators"
+ echo -e "$uuid\tVisible-All-Projects-Config" >> groups
+ git config -f "project.config" \
+ --add access."refs/meta/config".read "group Visible-All-Projects-Config"
+ git config -f "project.config" \
+ --add capability.viewTaskPaths "group Administrators"
q git add . && q git commit -m "project config update"
q git push origin HEAD:refs/meta/config
)
@@ -31,14 +40,16 @@
while (( "$#" )) ; do
case "$1" in
--non-secret-user) shift ; NON_SECRET_USER="$1" ;;
+ --untrusted-user) shift ; UNTRUSTED_USER="$1" ;;
*) die "invalid argument '$1'" ;;
esac
shift
done
[ -z "$NON_SECRET_USER" ] && die "non-secret-user not set"
+[ -z "$UNTRUSTED_USER" ] && die "untrusted-user not set"
"$USER_RUN_TESTS_DIR"/create-test-project-and-changes.sh
"$USER_RUN_TESTS_DIR"/update-all-users-project.sh
-create_test_user
+create_test_users_and_group
setup_all_projects_repo
\ No newline at end of file
diff --git a/test/docker/run_tests/start.sh b/test/docker/run_tests/start.sh
index a5f33d0..1d31d0b 100755
--- a/test/docker/run_tests/start.sh
+++ b/test/docker/run_tests/start.sh
@@ -25,9 +25,12 @@
is_plugin_loaded "task" || die "Task plugin is not installed"
NON_SECRET_USER="non_secret_user"
-"$USER_RUN_TESTS_DIR"/create-one-time-test-data.sh --non-secret-user "$NON_SECRET_USER"
+UNTRUSTED_USER="untrusted_user"
+"$USER_RUN_TESTS_DIR"/create-one-time-test-data.sh --non-secret-user "$NON_SECRET_USER" \
+ --untrusted-user "$UNTRUSTED_USER"
echo "Running Task plugin tests ..."
cd "$USER_RUN_TESTS_DIR"/../../ && ./check_task_statuses.sh \
- --server "$GERRIT_HOST" --non-secret-user "$NON_SECRET_USER"
+ --server "$GERRIT_HOST" --non-secret-user "$NON_SECRET_USER" \
+ --untrusted-user "$UNTRUSTED_USER"
