Kaynağa Gözat

fix: the workflows list reported the page size as the total

GET /workflows answered total 1 to ?pageSize=1 against eight workflows, and
hasMore was the literal false. Anything paginating the list was working from
a total that was wrong by construction.

The query already returned a real total_count and has_more; both were thrown
away and replaced with the size of the page. The comment blamed an in-memory
filter, but that filter only runs for one case - "ungrouped", which means
groupId null, missing or empty, and the database can only be asked for one of
those at a time because filters are ANDed.

So that case now decides over the whole collection rather than over one page,
and pages the result itself. Reading them all is fine here in a way it would
not be for executions: a workflows collection is the handful of things
somebody built, and it does not grow on its own.

Two more from the same handler, the same pair already fixed on executions:
page and pageSize went through std::stoi, so ?page=abc threw out of the
handler as an unexplained 500, and pageSize was unbounded.

Measured: total is 8 at every page size, paging walks 3 + 3 + 2 with hasMore
turning false on the last page and no workflow appearing twice, ?groupId=
gives 3 of 8, ?page=abc is a 200, and ?pageSize=99999 returns 8 rows clamped
to a page size of 200.
fszontagh 1 ay önce
ebeveyn
işleme
4e4d00d519
1 değiştirilmiş dosya ile 51 ekleme ve 18 silme
  1. 51 18
      src/webserver/api/workflow_controller.cpp

+ 51 - 18
src/webserver/api/workflow_controller.cpp

@@ -1,3 +1,4 @@
+#include <algorithm>
 #include "workflow_controller.hpp"
 #include "common/uuid.hpp"
 #include "common/time_utils.hpp"
@@ -124,16 +125,38 @@ void WorkflowController::listWorkflows(const httplib::Request& req, httplib::Res
         // Don't add a filter - we'll filter in memory below
     }
 
-    int page = 1;
-    int page_size = 20;
-    if (req.has_param("page")) {
-        page = std::stoi(req.get_param_value("page"));
-    }
-    if (req.has_param("pageSize")) {
-        page_size = std::stoi(req.get_param_value("pageSize"));
+    // std::stoi throws on anything that is not a number, which turned
+    // "?page=abc" into an unexplained 500.
+    auto intParam = [&req](const char* name, int fallback) {
+        if (!req.has_param(name)) return fallback;
+        try {
+            return std::stoi(req.get_param_value(name));
+        } catch (const std::exception&) {
+            return fallback;
+        }
+    };
+
+    const int page = std::max(1, intParam("page", 1));
+    const int page_size = std::clamp(intParam("pageSize", 20), 1, 200);
+
+    // "Ungrouped" means groupId is null, missing, or empty, and the database
+    // can only be asked for one of those at a time - the filters are ANDed. So
+    // that case is decided here instead, over the whole collection rather than
+    // over one page: filtering a page and then calling its size the total is
+    // what made ?pageSize=1 answer "total: 1" against eight workflows.
+    //
+    // Reading them all is fine here in a way it would not be for executions. A
+    // workflows collection is the handful of things somebody built; it does not
+    // grow on its own.
+    const bool ungrouped_only = req.has_param("groupId") && req.get_param_value("groupId").empty();
+
+    if (ungrouped_only) {
+        options.page = 1;
+        options.page_size = 1000;
+    } else {
+        options.page = page;
+        options.page_size = page_size;
     }
-    options.page = page;
-    options.page_size = page_size;
     options.sorts.push_back({"updatedAt", false});
 
     auto result = storage_.query("workflows", options);
@@ -142,27 +165,37 @@ void WorkflowController::listWorkflows(const httplib::Request& req, httplib::Res
         return;
     }
 
-    // If filtering for ungrouped workflows (empty groupId param), filter in memory
-    // to include workflows with null, empty string, or missing groupId
     nlohmann::json workflows = result.value().documents;
-    if (req.has_param("groupId") && req.get_param_value("groupId").empty()) {
-        nlohmann::json filtered = nlohmann::json::array();
+    int64_t total = result.value().total_count;
+    bool has_more = result.value().has_more;
+
+    if (ungrouped_only) {
+        nlohmann::json kept = nlohmann::json::array();
         for (const auto& wf : workflows) {
-            // Include if groupId is null, empty string, or doesn't exist
             if (!wf.contains("groupId") || wf["groupId"].is_null() ||
                 (wf["groupId"].is_string() && wf["groupId"].get<std::string>().empty())) {
-                filtered.push_back(wf);
+                kept.push_back(wf);
             }
         }
-        workflows = filtered;
+
+        total = static_cast<int64_t>(kept.size());
+
+        // Paged here too, since the query above deliberately was not.
+        const auto first = static_cast<size_t>(page - 1) * static_cast<size_t>(page_size);
+        nlohmann::json wanted = nlohmann::json::array();
+        for (size_t i = first; i < kept.size() && wanted.size() < static_cast<size_t>(page_size); ++i) {
+            wanted.push_back(kept[i]);
+        }
+        has_more = first + wanted.size() < kept.size();
+        workflows = wanted;
     }
 
     nlohmann::json response;
     response["workflows"] = workflows;
-    response["total"] = workflows.size();  // Update total after filtering
+    response["total"] = total;
     response["page"] = page;
     response["pageSize"] = page_size;
-    response["hasMore"] = false;  // Pagination not accurate after in-memory filter
+    response["hasMore"] = has_more;
 
     sendJson(res, response);
 }