Prechádzať zdrojové kódy

fix: the executions listing reports the collection's total, not the page's

It answered "20 of 20" against sixteen hundred records while hasMore said
otherwise, because the total was the length of the page after a per-row
filter.

The database was never the problem. Asked directly it reports the real
count for the filters given - 1,589 unfiltered, 1,452 for
status=completed, unchanged by the limit. The number arrived correctly and
was replaced here.

**A correction to the previous commit.** I wrote that the executions
listing "had no reachability check at all". It has one:
canSeeExecution() has always checked, per row, that the caller can read
the workflow's project. What my change actually did was move that
restriction into the query rather than applying it after the rows came
back - which matters for paging and totals, not for access. It was not a
security gap, and I should have read the function before saying so.

Applying it in the query is what makes the total trustworthy now: nothing
is dropped afterwards, so the count the database gives describes what is
being shown. If the two ever disagree the listing says so in the log
rather than letting the number drift.

One behaviour change worth naming: executions whose workflow has been
deleted were visible to an instance admin through the per-row check, and
the query filter excludes them, since it matches on the ids of workflows
that exist. There are none now - they were cleared earlier today - but an
admin looking for the history of a deleted workflow will not find it here.

70 passed, 0 failed.
fszontagh 1 mesiac pred
rodič
commit
b3bb4a78dc
1 zmenil súbory, kde vykonal 19 pridanie a 1 odobranie
  1. 19 1
      src/webserver/api/execution_controller.cpp

+ 19 - 1
src/webserver/api/execution_controller.cpp

@@ -298,7 +298,25 @@ void ExecutionController::listExecutions(const httplib::Request& req, httplib::R
         if (canSeeExecution(ctx, execution)) visible.push_back(execution);
     }
     response["executions"] = visible;
-    response["total"] = visible.size();
+
+    // The count of the collection, not of this page.
+    //
+    // It used to be visible.size(), so a page of twenty answered "20 of 20"
+    // against sixteen hundred records while hasMore said otherwise. The database
+    // reports the real total for the filters given - checked directly: 1,589
+    // unfiltered and 1,452 for status=completed, whatever the limit - so the
+    // number was being thrown away here rather than never known.
+    //
+    // The per-row check above no longer removes anything on this path, because
+    // the query is now restricted to workflows the caller can reach. If the two
+    // ever disagree the total would overstate what was shown, so say so rather
+    // than let it drift quietly.
+    const auto dropped = result.value().documents.size() - visible.size();
+    if (dropped > 0) {
+        LOG_WARN("Executions listing dropped {} row(s) after the query; the total is the "
+                 "query's and now overstates what is visible", dropped);
+    }
+    response["total"] = result.value().total_count;
     response["page"] = page;
     response["pageSize"] = page_size;
     response["hasMore"] = result.value().has_more;