Internal: Clean arguments to paper_set and paper_result.

* All paperId arguments are int|list<int>. * Add SearchSelection::paper_set() and use it.
ubc · May 13, 2020 · 7b811b0 · 7b811b0
1 parent d6fe995
commit 7b811b0
Show file tree

Hide file tree

Showing 25 changed files with 94 additions and 86 deletions.
diff --git a/batch/deletepapers.php b/batch/deletepapers.php
@@ -23,7 +23,7 @@
 $pids = $search->paper_ids();
 if (($pids = $search->paper_ids())) {
     $ndeleted = false;
-    foreach ($user->paper_set($pids) as $prow) {
+    foreach ($user->paper_set(["paperId" => $pids]) as $prow) {
         $pid = "#{$prow->paperId}";
         if ($prow->title !== "")
             $pid .= " (" . UnicodeHelper::utf8_abbreviate($prow->title, 40) . ")";

diff --git a/manualassign.php b/manualassign.php
@@ -60,7 +60,7 @@ function saveAssignments($qreq, $reviewer) {
 
     $confset = $Conf->conflict_types();
     $assignments = [];
-    foreach ($Me->paper_set($pids, ["reviewSignatures" => true]) as $row) {
+    foreach ($Me->paper_set(["paperId" => $pids, "reviewSignatures" => true]) as $row) {
         $name = "assrev" . $row->paperId . "u" . $rcid;
         if (!isset($qreq[$name])
             || ($assrev = cvtint($qreq[$name], null)) === null) {

diff --git a/paper.php b/paper.php
@@ -204,7 +204,7 @@ function update_paper(Qrequest $qreq, $action) {
         $webnotes .= " <ul><li>" . join("</li><li>", $ps->landmarked_message_texts()) . "</li></ul>";
     }
 
-    $new_prow = $Me->conf->fetch_paper(["paperId" => $ps->paperId, "topics" => true, "options" => true], $Me);
+    $new_prow = $Me->conf->fetch_paper($ps->paperId, $Me, ["topics" => true, "options" => true]);
     if (!$new_prow) {
         $Conf->msg($Conf->_("Your submission was not saved. Please correct these errors and save again.") . $webnotes, "merror");
         return false;

diff --git a/src/assignmentset.php b/src/assignmentset.php
@@ -295,7 +295,7 @@ function fetch_prows($pids, $initial_load = false) {
         }
         assert($initial_load || empty($fetch_pids));
         if (!empty($fetch_pids)) {
-            foreach ($this->user->paper_set($fetch_pids) as $prow) {
+            foreach ($this->user->paper_set(["paperId" => $fetch_pids]) as $prow) {
                 $this->prows[$prow->paperId] = $prow;
             }
             foreach ($fetch_pids as $pid) {

diff --git a/src/capability.php b/src/capability.php
@@ -157,7 +157,7 @@ static function apply_hoturl_capability($name, $isadd) {
     }
 
     static function apply_old_author_view(Contact $user, $uf, $isadd) {
-        if (($prow = $user->conf->fetch_paper(["paperId" => $uf->match_data[1]]))
+        if (($prow = $user->conf->fetch_paper((int) $uf->match_data[1]))
             && ($uf->name === self::capability_text($prow, "a"))
             && !$user->conf->opt("disableCapabilities")) {
             $user->set_capability("@av{$prow->paperId}", $isadd ? true : null);

diff --git a/src/conference.php b/src/conference.php
@@ -3303,23 +3303,31 @@ function make_csvg($basename, $flags = 0) {
     // Paper search
     //
 
-    /** @return list<int> */
-    static private function _cvt_numeric_set($optarr) {
-        $ids = [];
-        if (is_object($optarr)) {
-            $optarr = $optarr->selection();
+    /** @param ?list<int> $paperset
+     * @param list<int>|mysqli_result|Dbl_Result $arg
+     * @return list<int> */
+    static private function _merge_paperset($paperset, $arg) {
+        if (is_object($arg)) {
+            $ids = [];
+            while (($row = $arg->fetch_row())) {
+                $ids[] = (int) $row[0];
+            }
+        } else {
+            $ids = $arg;
         }
-        foreach (mkarray($optarr) as $x) {
-            if (($x = cvtint($x)) > 0)
-                $ids[] = $x;
+        if ($paperset === null) {
+            return $ids;
+        } else {
+            return array_values(array_intersect($paperset, $ids));
         }
-        return $ids;
     }
 
     function query_all_reviewer_preference() {
         return "group_concat(contactId,' ',preference,' ',coalesce(expertise,'.'))";
     }
 
+    /** @param array{paperId?:list<int>} $options
+     * @return mysqli_result|Dbl_Result */
     function paper_result($options, Contact $user = null) {
         // Options:
         //   "paperId" => $pid  Only paperId $pid (if array, any of those)
@@ -3350,28 +3358,32 @@ function paper_result($options, Contact $user = null) {
         }
 
         // paper selection
-        $paperset = [];
-        '@phan-var list<list<int>> $paperset';
+        $paperset = null;
+        '@phan-var ?list<int> $paperset';
+        assert(!isset($options["paperId"]) || is_array($options["paperId"]));
+        assert(!isset($options["reviewId"]));
+        assert(!isset($options["commentId"]));
         if (isset($options["paperId"])) {
-            $paperset[] = self::_cvt_numeric_set($options["paperId"]);
+            if (is_array($options["paperId"])) {
+                // XXX this is the only valid expected signature
+                $paperset = self::_merge_paperset($paperset, $options["paperId"]);
+            } else if (is_int($options["paperId"]) || is_string($options["paperId"])) {
+                $paperset = self::_merge_paperset($paperset, [cvtint($options["paperId"])]);
+            } else {
+                $paperset = self::_merge_paperset($paperset, $options["paperId"]->selection());
+            }
         }
         if (isset($options["reviewId"])) {
             if (is_numeric($options["reviewId"])) {
-                $result = $this->qe("select paperId from PaperReview where reviewId=?", $options["reviewId"]);
-                $paperset[] = self::_cvt_numeric_set(edb_first_columns($result));
+                $paperset = self::_merge_paperset($paperset, $this->qe("select paperId from PaperReview where reviewId=?", $options["reviewId"]));
             } else if (preg_match('/^(\d+)([A-Z][A-Z]?)$/i', $options["reviewId"], $m)) {
-                $result = $this->qe("select paperId from PaperReview where paperId=? and reviewOrdinal=?", $m[1], parseReviewOrdinal($m[2]));
-                $paperset[] = self::_cvt_numeric_set(edb_first_columns($result));
+                $paperset = self::_merge_paperset($paperset, $this->qe("select paperId from PaperReview where paperId=? and reviewOrdinal=?", $m[1], parseReviewOrdinal($m[2])));
             } else {
-                $paperset[] = [];
+                $paperset = [];
             }
         }
         if (isset($options["commentId"])) {
-            $result = $this->qe("select paperId from PaperComment where commentId?a", self::_cvt_numeric_set($options["commentId"]));
-            $paperset[] = self::_cvt_numeric_set(edb_first_columns($result));
-        }
-        if (count($paperset) > 1) {
-            $paperset = array(call_user_func_array("array_intersect", $paperset));
+            $paperset = self::_merge_paperset($paperset, $this->qe("select paperId from PaperComment where commentId?a", mkarray($options["commentId"])));
         }
 
         // prepare query: basic tables
@@ -3483,8 +3495,8 @@ function paper_result($options, Contact $user = null) {
         }
 
         // conditions
-        if (!empty($paperset)) {
-            $where[] = "Paper.paperId" . sql_in_numeric_set($paperset[0]);
+        if ($paperset !== null) {
+            $where[] = "Paper.paperId" . sql_in_numeric_set($paperset);
         }
         if ($options["finalized"] ?? false) {
             $where[] = "timeSubmitted>0";
@@ -3549,6 +3561,8 @@ function paper_result($options, Contact $user = null) {
         return $this->qe_raw($pq);
     }
 
+    /** @param array{paperId?:list<int>} $options
+     * @return PaperInfoSet */
     function paper_set($options, Contact $user = null) {
         $rowset = new PaperInfoSet;
         $result = $this->paper_result($options, $user);
@@ -3559,7 +3573,10 @@ function paper_set($options, Contact $user = null) {
         return $rowset;
     }
 
-    function fetch_paper($options, Contact $user = null) {
+    /** @param int $pid
+     * @return ?PaperInfo */
+    function fetch_paper($pid, Contact $user = null, $options = []) {
+        $options["paperId"] = [$pid];
         $result = $this->paper_result($options, $user);
         $prow = PaperInfo::fetch($result, $user, $this);
         Dbl::free($result);

diff --git a/src/contact.php b/src/contact.php
@@ -1882,7 +1882,7 @@ function is_author() {
     function authored_papers() {
         $this->check_rights_version();
         if ($this->_authored_papers === null) {
-            $this->_authored_papers = $this->is_author() ? $this->conf->paper_set(["author" => true, "tags" => true], $this)->as_array() : [];
+            $this->_authored_papers = $this->is_author() ? $this->paper_set(["author" => true, "tags" => true])->as_array() : [];
         }
         return $this->_authored_papers;
     }
@@ -4302,27 +4302,11 @@ function has_reportable_deadline() {
 
     // papers
 
-    /** @return PaperInfoSet|Iterable<PaperInfo> */
-    function paper_set($pids, $options = []) {
-        $ssel = null;
-        if (is_int($pids)) {
-            $options["paperId"] = $pids;
-        } else if (is_array($pids)
-                   && !is_associative_array($pids)
-                   && (!empty($pids) || $options !== null)) {
-            $options["paperId"] = $pids;
-        } else if (is_object($pids) && $pids instanceof SearchSelection) {
-            $ssel = $pids;
-            $options["paperId"] = $pids->selection();
-        } else {
-            assert($options === []);
-            $options = $pids;
-        }
-        $prows = $this->conf->paper_set($options, $this);
-        if ($ssel) {
-            $prows->sort_by([$ssel, "order_compare"]);
-        }
-        return $prows;
+    /** @param array{paperId?:int|list<int>} $options
+     * @return PaperInfoSet|Iterable<PaperInfo> */
+    function paper_set($options = []) {
+        assert(func_num_args() <= 1);
+        return $this->conf->paper_set($options, $this);
     }
 
     function hide_reviewer_identity_pids() {

diff --git a/src/listaction.php b/src/listaction.php
@@ -73,7 +73,8 @@ static function call($name, Contact $user, Qrequest $qreq, $selection) {
     }
 
 
-    static function pcassignments_csv_data(Contact $user, $selection) {
+    /** @param list<int> $pids */
+    static function pcassignments_csv_data(Contact $user, $pids) {
         require_once("assignmentset.php");
         $pcm = $user->conf->pc_members();
         $token_users = [];
@@ -82,7 +83,7 @@ static function pcassignments_csv_data(Contact $user, $selection) {
         $any_round = $any_token = false;
 
         $texts = [];
-        foreach ($user->paper_set($selection, ["reviewSignatures" => true]) as $prow) {
+        foreach ($user->paper_set(["paperId" => $pids, "reviewSignatures" => true]) as $prow) {
             if (!$user->allow_administer($prow)) {
                 $texts[] = [];
                 $texts[] = ["paper" => $prow->paperId,

diff --git a/src/listactions/la_get_rev.php b/src/listactions/la_get_rev.php
@@ -100,7 +100,7 @@ function run(Contact $user, $qreq, $ssel) {
         }
 
         $texts = $errors = [];
-        foreach ($user->paper_set($ssel) as $prow) {
+        foreach ($ssel->paper_set($user) as $prow) {
             $whyNot = $user->perm_review($prow, null);
             if ($whyNot
                 && !isset($whyNot["deadline"])
@@ -149,7 +149,7 @@ function run(Contact $user, $qreq, $ssel) {
             Contact::update_rights();
         }
         $errors = $texts = $pids = [];
-        foreach ($user->paper_set($ssel) as $prow) {
+        foreach ($ssel->paper_set($user) as $prow) {
             if (($whyNot = $user->perm_view_paper($prow))) {
                 $errors["#$prow->paperId: " . whyNotText($whyNot, true)] = true;
                 continue;

diff --git a/src/listactions/la_get_sub.php b/src/listactions/la_get_sub.php
@@ -54,7 +54,7 @@ function run(Contact $user, $qreq, $ssel) {
 class GetCheckFormat_ListAction extends ListAction {
     function run(Contact $user, $qreq, $ssel) {
         $papers = [];
-        foreach ($user->paper_set($ssel) as $prow) {
+        foreach ($ssel->paper_set($user) as $prow) {
             if ($user->can_view_pdf($prow))
                 $papers[$prow->paperId] = $prow;
         }
@@ -149,7 +149,7 @@ static function render(PaperInfo $prow, Contact $user) {
     function run(Contact $user, $qreq, $ssel) {
         $texts = [];
         $lastpid = null;
-        foreach ($user->paper_set($ssel, ["topics" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["topics" => 1]) as $prow) {
             if (($whyNot = $user->perm_view_paper($prow))) {
                 Conf::msg_error(whyNotText($whyNot));
             } else {
@@ -180,7 +180,7 @@ function run(Contact $user, $qreq, $ssel) {
         $contact_map = self::contact_map($user->conf, $ssel);
         $texts = array();
         $want_contacttype = false;
-        foreach ($user->paper_set($ssel, ["allConflictType" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["allConflictType" => 1]) as $prow) {
             if (!$user->allow_view_authors($prow)) {
                 continue;
             }
@@ -224,7 +224,7 @@ function allow(Contact $user, Qrequest $qreq) {
     function run(Contact $user, $qreq, $ssel) {
         $contact_map = GetAuthors_ListAction::contact_map($user->conf, $ssel);
         $texts = [];
-        foreach ($user->paper_set($ssel, ["allConflictType" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["allConflictType" => 1]) as $prow) {
             if ($user->allow_administer($prow)) {
                 foreach ($prow->contacts() as $cid => $c) {
                     $a = $contact_map[$cid];
@@ -248,7 +248,7 @@ function run(Contact $user, $qreq, $ssel) {
         $pcm = $user->conf->pc_members();
         $texts = array();
         $old_overrides = $user->add_overrides(Contact::OVERRIDE_CONFLICT);
-        foreach ($user->paper_set($ssel, ["allConflictType" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["allConflictType" => 1]) as $prow) {
             if ($user->can_view_conflicts($prow)) {
                 $m = [];
                 foreach ($prow->conflicts() as $cid => $cflt) {
@@ -272,7 +272,7 @@ function run(Contact $user, $qreq, $ssel) {
 class GetTopics_ListAction extends ListAction {
     function run(Contact $user, $qreq, $ssel) {
         $texts = array();
-        foreach ($user->paper_set($ssel, ["topics" => 1]) as $row) {
+        foreach ($ssel->paper_set($user, ["topics" => 1]) as $row) {
             if ($user->can_view_paper($row)) {
                 $out = array();
                 foreach ($row->topic_map() as $t) {

diff --git a/src/listactions/la_getallrevpref.php b/src/listactions/la_getallrevpref.php
@@ -10,7 +10,7 @@ function run(Contact $user, $qreq, $ssel) {
         $texts = array();
         $pcm = $user->conf->pc_members();
         $has_conflict = $has_expertise = $has_topic_score = false;
-        foreach ($user->paper_set($ssel, ["allReviewerPreference" => 1, "allConflictType" => 1, "topics" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["allReviewerPreference" => 1, "allConflictType" => 1, "topics" => 1]) as $prow) {
             if (!$user->allow_administer($prow)) {
                 continue;
             }

diff --git a/src/listactions/la_getdocument.php b/src/listactions/la_getdocument.php
@@ -42,7 +42,7 @@ function run(Contact $user, $qreq, $ssel) {
         $downloads = $errors = [];
         $old_overrides = $user->add_overrides(Contact::OVERRIDE_CONFLICT);
         $opt = $user->conf->paper_opts->get($this->dt);
-        foreach ($user->paper_set($ssel) as $row) {
+        foreach ($ssel->paper_set($user) as $row) {
             if (($whyNot = $user->perm_view_option($row, $opt))) {
                 $errors[] = self::error_document($opt, $row, whyNotText($whyNot));
             } else if (($doc = $row->document($opt->id))) {

diff --git a/src/listactions/la_getjson.php b/src/listactions/la_getjson.php
@@ -25,7 +25,7 @@ function run(Contact $user, $qreq, $ssel) {
             $this->zipdoc = new ZipDocument($user->conf->download_prefix . "data.zip");
             $ps->on_document_export([$this, "document_callback"]);
         }
-        foreach ($user->paper_set($ssel, ["topics" => true, "options" => true]) as $prow) {
+        foreach ($ssel->paper_set($user, ["topics" => true, "options" => true]) as $prow) {
             $pj1 = $ps->paper_json($prow);
             if ($pj1)
                 $pj[] = $pj1;

diff --git a/src/listactions/la_getjsonrqc.php b/src/listactions/la_getjsonrqc.php
@@ -15,7 +15,7 @@ function run(Contact $user, $qreq, $ssel) {
         $results["reviewform"] = $rf->unparse_json(0, VIEWSCORE_REVIEWERONLY);
         $pj = [];
         $ps = new PaperStatus($user->conf, $user, ["hide_docids" => true]);
-        foreach ($user->paper_set($ssel, ["topics" => true, "options" => true]) as $prow) {
+        foreach ($ssel->paper_set($user, ["topics" => true, "options" => true]) as $prow) {
             if ($user->allow_administer($prow)) {
                 $pj[] = $j = $ps->paper_json($prow);
                 $prow->ensure_full_reviews();

diff --git a/src/listactions/la_getlead.php b/src/listactions/la_getlead.php
@@ -13,8 +13,8 @@ function allow(Contact $user, Qrequest $qreq) {
     function run(Contact $user, $qreq, $ssel) {
         $key = $this->type . "ContactId";
         $can_view = "can_view_" . $this->type;
-        $texts = array();
-        foreach ($user->paper_set($ssel) as $row) {
+        $texts = [];
+        foreach ($ssel->paper_set($user) as $row) {
             if ($row->$key && $user->$can_view($row, true)) {
                 $name = $user->name_object_for($row->$key);
                 $texts[$row->paperId][] = [$row->paperId, $row->title, $name->firstName, $name->lastName, $name->email];

diff --git a/src/listactions/la_getrank.php b/src/listactions/la_getrank.php
@@ -14,7 +14,7 @@ function run(Contact $user, $qreq, $ssel) {
         $tagger = new Tagger($user);
         if (($tag = $tagger->check($qreq->tag, Tagger::NOVALUE | Tagger::NOCHAIR))) {
             $real = $null = "";
-            $pset = $user->paper_set($ssel, ["tags" => true]);
+            $pset = $ssel->paper_set($user, ["tags" => true]);
             $pset->sort_by(function ($p1, $p2) use ($tag) {
                 $tv1 = $p1->tag_value($tag);
                 $tv2 = $p2->tag_value($tag);

diff --git a/src/listactions/la_getreviewcsv.php b/src/listactions/la_getreviewcsv.php
@@ -19,7 +19,7 @@ function run(Contact $user, $qreq, $ssel) {
         }
         $errors = $items = $fields = $pids = [];
         $has_id = $has_ordinal = false;
-        foreach ($user->paper_set($ssel) as $prow) {
+        foreach ($ssel->paper_set($user) as $prow) {
             if (($whyNot = $user->perm_view_paper($prow))) {
                 $errors["#$prow->paperId: " . whyNotText($whyNot, true)] = true;
                 continue;

diff --git a/src/listactions/la_getrevpref.php b/src/listactions/la_getrevpref.php
@@ -39,7 +39,7 @@ function run(Contact $user, $qreq, $ssel) {
         $not_me = $user->contactId !== $Rev->contactId;
         $has_conflict = false;
         $texts = [];
-        foreach ($user->paper_set($ssel, ["topics" => 1, "reviewerPreference" => 1]) as $prow) {
+        foreach ($ssel->paper_set($user, ["topics" => 1, "reviewerPreference" => 1]) as $prow) {
             if ($not_me && !$user->allow_administer($prow))
                 continue;
             $item = ["paper" => $prow->paperId, "title" => $prow->title];

diff --git a/src/listactions/la_getscores.php b/src/listactions/la_getscores.php
@@ -12,7 +12,7 @@ function run(Contact $user, $qreq, $ssel) {
         // compose scores; NB chair is always forceShow
         $errors = $texts = $any_scores = array();
         $any_decision = $any_reviewer_identity = $any_ordinal = false;
-        foreach ($user->paper_set($ssel) as $row) {
+        foreach ($ssel->paper_set($user) as $row) {
             if (($whyNot = $user->perm_view_paper($row)))
                 $errors[] = "#$row->paperId: " . whyNotText($whyNot);
             else if (($whyNot = $user->perm_view_review($row, null)))

diff --git a/src/paperapi.php b/src/paperapi.php
@@ -91,7 +91,7 @@ static function settags_api(Contact $user, $qreq, $prow) {
         } else if ($ok) {
             $p = [];
             if ($pids) {
-                foreach ($user->paper_set(array_keys($pids)) as $pr) {
+                foreach ($user->paper_set(["paperId" => array_keys($pids)]) as $pr) {
                     $p[$pr->paperId] = (object) [];
                     $pr->add_tag_info_json($p[$pr->paperId], $user);
                 }

diff --git a/src/paperevents.php b/src/paperevents.php
@@ -168,7 +168,7 @@ private function load_papers() {
                 $need[$crow->paperId] = true;
         }
         if (!empty($need)) {
-            $this->prows->take_all($this->user->paper_set(array_keys($need), ["watch" => true]));
+            $this->prows->take_all($this->user->paper_set(["paperId" => array_keys($need), "watch" => true]));
         }
     }