瀏覽代碼

fix(api): stop leaking qualified names in relation error messages

- documents.cpp: 409 delete-blocked-by-relation now returns a fixed
  "delete blocked by a relation" message instead of forwarding the DB's
  own error string, which contained project-qualified names the API
  never accepts. Actionable data stays in details.impacts (unqualified).
- relations.cpp: delete-impact 404 now returns a fixed "document not
  found" message instead of forwarding raw DB text for the same reason.
- tests: DeleteBlockedByRelationReturns409WithImpacts and
  RelationsEnforcedOnParentPermitsDelete now use an RAII guard (same
  idiom as AdminReadonlyLockBlocksWritesThenReleases) to drop the
  relations they create and restore relations_enforced on cust2.
  tmpName() is deterministic and dropProject does not purge collection
  data, so leftover relations previously persisted across test runs.
- fix stray em-dashes in comments introduced by this branch.
fszontagh 1 月之前
父節點
當前提交
ffe1c5ad5d
共有 4 個文件被更改,包括 37 次插入 和 7 次删除
  1. 2 2
      src/handlers/documents.cpp
  2. 1 2
      src/handlers/relations.cpp
  3. 1 1
      src/handlers/settings.cpp
  4. 33 2
      tests/test_api_integration.cpp

+ 2 - 2
src/handlers/documents.cpp

@@ -17,7 +17,7 @@ std::string scoped(ServerDeps* d, const httplib::Request& req, int projIdx, int
     return qualify(project, name);
 }
 
-// Parses ?ttl_seconds=. Returns nullopt when absent — the caller must
+// Parses ?ttl_seconds=. Returns nullopt when absent - the caller must
 // distinguish "not supplied" (leave expiry alone) from 0 (clear the expiry).
 std::optional<uint32_t> ttlParam(const httplib::Request& req) {
     if (!req.has_param("ttl_seconds")) return std::nullopt;
@@ -97,7 +97,7 @@ void registerDocumentRoutes(ApiServer& s) {
             auto why = d->db.client().describeDelete(c, id);
             if (why.success && why.wouldBeBlocked)
                 throw ApiError(ErrCode::Conflict, "relation_restricted",
-                               err.empty() ? "delete blocked by a relation" : err,
+                               "delete blocked by a relation",
                                {{"impacts", impactsJson(project, why)}});
             throw ApiError(ErrCode::NotFound, "not_found", "no such document");
         }

+ 1 - 2
src/handlers/relations.cpp

@@ -116,8 +116,7 @@ void registerRelationRoutes(ApiServer& s) {
         // about data, so it follows the collection's read grant.
         requireCapability(*d, k, req, project, coll, KeyOp::Read);
         auto r = d->db.client().describeDelete(qualify(project, coll), id);
-        if (!r.success) throw ApiError(ErrCode::NotFound, "not_found",
-                                       r.error.empty() ? "document not found" : r.error);
+        if (!r.success) throw ApiError(ErrCode::NotFound, "not_found", "document not found");
         sendJson(res, 200, {{"would_be_blocked", r.wouldBeBlocked},
                             {"impacts", impactsJson(project, r)}});
     });

+ 1 - 1
src/handlers/settings.cpp

@@ -68,7 +68,7 @@ void registerSettingsRoutes(ApiServer& s) {
     });
 
     // Server-global read-only lock (not per project). GET reflects the DB's own
-    // authoritative state — the DB can also enter read-only by itself after a
+    // authoritative state - the DB can also enter read-only by itself after a
     // failed/degraded recovery, not only via an operator's PUT here, so `reason`
     // (when non-empty) tells the caller which it was.
     svr.Get(R"(/api/v1/admin/readonly)", [d](const httplib::Request& req, httplib::Response& res) {

+ 33 - 2
tests/test_api_integration.cpp

@@ -1108,6 +1108,18 @@ TEST_F(ApiFixture, RelationRejectsBadOnDelete) {
 
 TEST_F(ApiFixture, DeleteBlockedByRelationReturns409WithImpacts) {
     auto c = admin();
+    std::string relUrl = "/api/v1/projects/" + project_ + "/relations/inv_cust";
+
+    // RAII guard: this test creates a restrict relation that must not survive the
+    // test, since tmpName()'s project name is deterministic and dropProject does
+    // not purge collection data (see db_test_util.hpp). Runs on early ASSERT_*
+    // returns too. A missing relation on cleanup is fine (idempotent delete).
+    struct RelationGuard {
+        httplib::Client& c;
+        std::string url;
+        ~RelationGuard() { c.Delete(url.c_str()); }
+    } relGuard{c, relUrl};
+
     std::string base = "/api/v1/projects/" + project_ + "/collections";
     ASSERT_EQ(c.Post(base.c_str(), nlohmann::json{{"name","inv"},{"kind","json"}}.dump(),
                      "application/json")->status, 201);
@@ -1139,17 +1151,36 @@ TEST_F(ApiFixture, DeleteBlockedByRelationReturns409WithImpacts) {
 }
 
 // relations_enforced is read on the collection being deleted FROM (the
-// PARENT side of the relation) — Delete() checks
+// PARENT side of the relation) - Delete() checks
 // config_manager_.configFor(request->collection()), i.e. the parent's own
 // flag. Disabling it on the CHILD collection has no effect on deletes of
 // the parent's documents: the child's relations_enforced only governs the
 // child's own reverse-index arming / validate_on_write, a separate
 // mechanism. So the toggle here targets "cust2" (the parent), not "inv2"
-// (the child) — this is deliberately non-obvious and every API consumer
+// (the child) - this is deliberately non-obvious and every API consumer
 // will trip on it otherwise (see task-8-report.md for the source trace).
 TEST_F(ApiFixture, RelationsEnforcedOnParentPermitsDelete) {
     auto c = admin();
     std::string base = "/api/v1/projects/" + project_ + "/collections";
+    std::string relUrl = "/api/v1/projects/" + project_ + "/relations/inv2_cust2";
+    std::string enforcedUrl = base + "/cust2/relations-enforced";
+
+    // RAII guard: this test creates a restrict relation and flips relations_enforced
+    // to false on cust2 - both must be undone unconditionally, since tmpName()'s
+    // project name is deterministic and dropProject does not purge collection data
+    // (see db_test_util.hpp). Runs on early ASSERT_* returns too. A missing relation
+    // on cleanup is fine (idempotent delete); restoring enforced=true is idempotent too.
+    struct RelationEnforcedGuard {
+        httplib::Client& c;
+        std::string relUrl;
+        std::string enforcedUrl;
+        ~RelationEnforcedGuard() {
+            c.Delete(relUrl.c_str());
+            c.Put(enforcedUrl.c_str(), nlohmann::json{{"enforced", true}}.dump(),
+                 "application/json");
+        }
+    } relGuard{c, relUrl, enforcedUrl};
+
     ASSERT_EQ(c.Post(base.c_str(), nlohmann::json{{"name","inv2"},{"kind","json"}}.dump(),
                      "application/json")->status, 201);
     ASSERT_EQ(c.Post(base.c_str(), nlohmann::json{{"name","cust2"},{"kind","json"}}.dump(),