Skip to content

Commit

Permalink
Improve plan for resolving qualified extensions
Browse files Browse the repository at this point in the history 
We want "merge" extensions to apply differently to "get" and
"post" for API. Old plan didn't allow that. New plan does.
Also, new plan checks "get"/"post" on aliases too.

And more API spec.
  • Loading branch information
@kohler
kohler committed Sep 15, 2024
1 parent ac8f976 commit 7a6192b
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 119 deletions.
73 changes: 32 additions & 41 deletions etc/apifunctions.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"parameters": "?p =assignments"
},
{
"name": "claimreview", "paper": true, "post": true, "redirect": true,
"name": "claimreview", "post": true, "paper": true, "redirect": true,
"function": "RequestReview_API::claimreview",
"parameters": "r email",
"response": "action review_site_relative"
Expand All @@ -39,14 +39,14 @@
"parameters": "?p ?=accept =clickthrough_id =clickthrough_type =clickthrough_time"
},
{
"name": "comment", "paper": true, "get": true,
"name": "comment", "get": true, "paper": true,
"function": "Comment_API::run",
"parameters": "c",
"response": "?comment",
"response_deprecated": "cmt"
},
{
"name": "comment", "paper": true, "post": true,
"name": "comment", "post": true, "paper": true,
"function": "Comment_API::run",
"parameters": "c ?override ?=text ?=response ?=ready ?=topic ?=draft ?=blind ?=tags ?=visibility ?=:attachment ?=by_author ?=review_token ?delete",
"response": "?comment ?conflict",
Expand All @@ -59,12 +59,12 @@
"response": "action ?reason review_site_relative"
},
{
"name": "decision", "paper": true, "get": true,
"name": "decision", "get": true, "paper": true,
"function": "Decision_API::run",
"response": "decision decision_html ?editable"
},
{
"name": "decision", "paper": true, "post": true,
"name": "decision", "post": true, "paper": true,
"function": "Decision_API::run",
"parameters": "=decision",
"response": "decision decision_html ?editable",
Expand All @@ -87,13 +87,13 @@
"response": "fields data ?stat ?classes ?attr"
},
{
"name": "follow", "paper": true, "post": true, "xxx": true,
"name": "follow", "post": true, "paper": true, "xxx": true,
"function": "Follow_API::run",
"parameters": "?u =following",
"response": "following"
},
{
"name": "formatcheck", "get": true, "post": true,
"name": "formatcheck", "get": true,
"function": "FormatCheck_API::run",
"parameters": "?p ?doc ?soft ?dt ?docid",
"response": "npages nwords problem_fields has_error docid",
Expand All @@ -110,42 +110,42 @@
"response": "update_at ?status *"
},
{
"name": "jserror", "get": true,
"name": "jserror", "post": true,
"function": "Error_API::jserror",
"parameters": "=error ?=url ?=lineno ?=colno ?=stack"
},
{
"name": "lead", "paper": true, "get": true,
"name": "lead", "get": true, "paper": true,
"function": "PaperPC_API::lead_api",
"response": "lead lead_html ?color_classes"
},
{
"name": "lead", "paper": true, "post": true,
"name": "lead", "post": true, "paper": true,
"function": "PaperPC_API::lead_api",
"parameters": "=lead",
"response": "lead lead_html ?color_classes",
"response_deprecated": "value result"
},
{
"name": "mailtext", "get": true, "post": true,
"name": "mailtext", "get": true,
"function": "Mail_API::mailtext",
"parameters": "?template ?p ?r ?email ?given_name ?family_name ?affiliation ?reason ?width ?text ?subject ?body",
"response": "?templates ?subject ?body"
},
{
"name": "manager", "paper": true, "get": true,
"name": "manager", "get": true, "paper": true,
"function": "PaperPC_API::manager_api",
"response": "manager manager_html ?color_classes",
"response_deprecated": "value result"
},
{
"name": "manager", "paper": true, "post": true,
"name": "manager", "post": true, "paper": true,
"function": "PaperPC_API::manager_api",
"parameters": "=manager",
"response": "manager manager_html ?color_classes"
},
{
"name": "administrator", "alias": "manager"
"name": "administrator", "alias": "manager", "get": true, "post": true
},
{
"name": "namedformula", "get": true,
Expand All @@ -169,7 +169,7 @@
"response": "searches"
},
{
"name": "mentioncompletion", "get": true, "post": true,
"name": "mentioncompletion", "get": true,
"function": "Completion_API::mentioncompletion_api",
"parameters": "?p",
"response": "mentioncompletion"
Expand All @@ -187,21 +187,21 @@
"function": "Paper_API::run"
},
{
"name": "pc", "get": true, "post": true,
"name": "pc", "get": true,
"function": "PaperPC_API::pc_api",
"response": "pc"
},
{
"name": "requestreview", "paper": true, "post": true,
"name": "requestreview", "post": true, "paper": true,
"function": "RequestReview_API::requestreview",
"parameters": "?round =email ?=given_name ?=family_name ?=name ?=affiliation ?=override ?=reason"
},
{
"name": "review", "paper": true, "get": true, "post": true,
"name": "review", "get": true, "post": true, "paper": true,
"function": "Review_API::review"
},
{
"name": "reviewhistory", "paper": true, "get": true, "post": true,
"name": "reviewhistory", "get": true, "paper": true,
"function": "Review_API::reviewhistory"
},
{
Expand All @@ -210,19 +210,19 @@
"response": "samples types"
},
{
"name": "reviewrating", "paper": true, "get": true,
"name": "reviewrating", "get": true, "paper": true,
"function": "Review_API::reviewrating",
"parameters": "r",
"response": "?ratings ?user_rating"
},
{
"name": "reviewrating", "paper": true, "post": true,
"name": "reviewrating", "post": true, "paper": true,
"function": "Review_API::reviewrating",
"parameters": "r =user_rating",
"response": "?ratings ?user_rating"
},
{
"name": "reviewround", "paper": true, "post": true, "xxx": true,
"name": "reviewround", "post": true, "xxx": true, "paper": true,
"function": "Review_API::reviewround"
},
{
Expand All @@ -242,10 +242,10 @@
"response": "value pref ?prefexp ?topic_score"
},
{
"name": "reviewpref", "alias": "revpref"
"name": "reviewpref", "alias": "revpref", "get": true, "post": true
},
{
"name": "pref", "alias": "revpref"
"name": "pref", "alias": "revpref", "get": true, "post": true
},
{
"name": "search", "get": true,
Expand All @@ -269,19 +269,16 @@
"parameters": "v",
"response": "postvalue sessioninfo"
},
{
"name": "settags", "alias": "tags"
},
{
"name": "tags", "get": true, "paper": true,
"function": "Tags_API::run",
"response": "pid tags tags_edit_text tags_view_html tag_decoration_html color_classes"
"response": "pid tags tags_edit_text tags_view_html tag_decoration_html color_classes ?tags_conflicted ?color_classes_conflicted"
},
{
"name": "tags", "post": true,
"function": "Tags_API::run",
"parameters": "?p ?=tags ?=addtags ?=deltags ?=tagassignment ?=search",
"response": "?pid ?tags ?tags_edit_text ?tags_view_html ?tag_decoration_html ?color_classes ?p ?ids ?groups ?hotlist ?search_params"
"response": "?pid ?tags ?tags_edit_text ?tags_view_html ?tag_decoration_html ?color_classes ?tags_conflicted ?color_classes_conflicted ?p ?ids ?groups ?hotlist ?search_params"
},
{
"name": "settingdescriptions", "get": true,
Expand All @@ -295,12 +292,12 @@
"response": "?dry_run ?changes"
},
{
"name": "shepherd", "paper": true, "get": true,
"name": "shepherd", "get": true, "paper": true,
"function": "PaperPC_API::shepherd_api",
"response": "shepherd shepherd_html ?color_classes"
},
{
"name": "shepherd", "paper": true, "post": true,
"name": "shepherd", "post": true, "paper": true,
"function": "PaperPC_API::shepherd_api",
"parameters": "=shepherd",
"response": "shepherd shepherd_html ?color_classes",
Expand All @@ -322,13 +319,10 @@
"parameters": "tag"
},
{
"name": "tagmessages", "paper": true, "get": true,
"name": "tagmessages", "get": true, "paper": true,
"function": "Tags_API::tagmessages_api",
"response": "pid"
},
{
"name": "tagreport", "alias": "tagmessages"
},
{
"name": "track", "post": true,
"parameters": "track ?tracker_start_at ?p ?=hotlist-info"
Expand All @@ -340,14 +334,14 @@
"response": "?tracker ?tracker_recent ?tracker_status ?now tracker_status_at tracker_eventid ?new_trackerid ?tracker_site"
},
{
"name": "trackerstatus", "get": true, "post": true,
"name": "trackerstatus", "get": true,
"function": "MeetingTracker::trackerstatus_api"
},
{
"name": "upload", "post": true,
"function": "Upload_API::run",
"parameters": "?p ?dtype ?start ?offset ?finish ?token ?cancel ?=size ?=mimetype ?=filename ?@blob",
"response": "token dtype filename mimetype size ranges hash server_progress_loaded server_progress_max"
"response": "token ?dtype ?filename ?mimetype ?size ?ranges ?hash ?server_progress_loaded ?server_progress_max"

},
{
Expand All @@ -369,10 +363,7 @@
"response": "report display_current display_default display_difference display_default_message_list"
},
{
"name": "listreport", "alias": "viewoptions"
},
{
"name": "votereport", "paper": true, "get": true, "post": true,
"name": "votereport", "get": true, "paper": true,
"function": "Tags_API::votereport_api",
"parameters": "tag",
"response": "tag report"
Expand Down
8 changes: 4 additions & 4 deletions etc/listactions.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,31 @@
"function": "+GetPcassignments_ListAction"
},
{
"name": "get/revform", "get": true, "paper_optional": true,
"name": "get/revform", "get": true,
"title": "Review assignments/Your review forms",
"order": 20000,
"allow_if": "reviewer",
"function": "+GetReviewForms_ListAction",
"zip": false, "all": false
},
{
"name": "get/revformz", "get": true, "paper_optional": true,
"name": "get/revformz", "get": true,
"title": "Review assignments/Your review forms (zip)",
"order": 20001,
"allow_if": "reviewer",
"function": "+GetReviewForms_ListAction",
"zip": true, "all": false
},
{
"name": "get/allrevform", "get": true, "paper_optional": true,
"name": "get/allrevform", "get": true,
"title": "Review assignments/All review forms",
"order": 20002,
"allow_if": "manager",
"function": "+GetReviewForms_ListAction",
"zip": false, "all": true
},
{
"name": "get/allrevformz", "get": true, "paper_optional": true,
"name": "get/allrevformz", "get": true,
"title": "Review assignments/All review forms (zip)",
"order": 20003,
"allow_if": "manager",
Expand Down
6 changes: 5 additions & 1 deletion src/conference.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5472,8 +5472,12 @@ function has_api($fn, ?Contact $user = null, $method = null) {
return !!$this->api($fn, $user, $method);
}
function api($fn, ?Contact $user = null, $method = null) {
$xtp = (new XtParams($this, $user))->add_allow_checker_method($method);
$xtp = (new XtParams($this, $user))->set_require_key_for_method($method);
$uf = $xtp->search_name($this->api_map(), $fn);
if (!$uf && $method === "POST") {
$xtp->set_require_key_for_method("GET"); // XXX should not do this
$uf = $xtp->search_name($this->api_map(), $fn);
}
return self::xt_enabled($uf) ? $uf : null;
}
/** @return JsonResult */
Expand Down
42 changes: 25 additions & 17 deletions src/listaction.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,25 @@ static function eperm() {
}

/** @return ComponentSet */
static function grouped_extensions(Contact $user) {
$gex = new ComponentSet($user, ["etc/listactions.json"], $user->conf->opt("listActions"));
foreach ($gex->members("__expand") as $gj) {
if (!isset($gj->allow_if) || $gex->allowed($gj->allow_if, $gj)) {
static function components(Contact $user) {
$cs = new ComponentSet($user, ["etc/listactions.json"], $user->conf->opt("listActions"));
foreach ($cs->members("__expand") as $gj) {
if (!isset($gj->allow_if) || $cs->allowed($gj->allow_if, $gj)) {
Conf::xt_resolve_require($gj);
call_user_func($gj->expand_function, $gex, $gj);
call_user_func($gj->expand_function, $cs, $gj);
}
}
return $gex;
return $cs;
}

/** @param ComponentSet $gex
/** @param ComponentSet $cs
* @param string $group
* @return list */
static function members_selector_options($gex, $group) {
static function members_selector_options($cs, $group) {
$last_group = null;
$sel_opt = [];
$p = strlen($group) + 1;
foreach ($gex->members($group) as $rf) {
foreach ($cs->members($group) as $rf) {
if (!str_starts_with($rf->name, "__")) {
$as = strpos($rf->title, "/");
if ($as === false) {
Expand Down Expand Up @@ -78,16 +78,24 @@ static private function do_call($name, Contact $user, Qrequest $qreq, $selection
return JsonResult::make_error(403, "<0>Missing credentials");
}
$conf = $user->conf;
$gex = self::grouped_extensions($user);
$gex->xtp->add_allow_checker_method($qreq->method());
$uf = $gex->get($name);
if (!$uf && ($slash = strpos($name, "/"))) {
$uf = $gex->get(substr($name, 0, $slash));
$cs = self::components($user);
$slash = strpos($name, "/");
$cs->xtp->set_require_key_for_method($qreq->method());
$uf = $cs->get($name);
if (!$uf && $slash > 0) {
$uf = $cs->get(substr($name, 0, $slash));
}
// XXX allow `POST` requests to `get`
if (!$uf && $qreq->is_post()) {
$cs->xtp->set_require_key_for_method("GET");
$uf = $cs->get($name);
if (!$uf && $slash > 0) {
$uf = $cs->get(substr($name, 0, $slash));
}
}
if (!$uf) {
$gex->reset_context();
$gex->xtp->allow_checkers = [];
$uf1 = $gex->get($name);
$cs->reset_context()->set_require_key_for_method(null);
$uf1 = $cs->get($name);
if ($uf1) {
return JsonResult::make_error(405, "<0>Method not supported");
}
Expand Down
Loading

0 comments on commit 7a6192b

Please sign in to comment.