Explorar el Código

fix(relations): T5 review - share the per-relation count lookup

findRelationBlocks() and describeDeleteImpacts() each independently
resolved the bare relation name, counted children, and sampled ids -
the exact two-copies-of-a-count pattern behind the v2.4.4 misfiled-rows
incident and the v2.7.1 divergent-count bug. Factored the shared step
into a file-local lookupRelationCounts() helper; both public signatures
are unchanged, only the duplicated body is gone.

test_relation_enforcement 45/0, test_subdb_identity 236/0 - unchanged
pass counts, behaviour identical, duplication removed.
fszontagh hace 1 mes
padre
commit
0257ffb220
Se han modificado 1 ficheros con 51 adiciones y 32 borrados
  1. 51 32
      service/src/relations/relation_enforcement.cpp

+ 51 - 32
service/src/relations/relation_enforcement.cpp

@@ -7,6 +7,47 @@
 
 #include <spdlog/spdlog.h>
 
+namespace {
+
+// Shared per-relation lookup step used by both findRelationBlocks() and
+// describeDeleteImpacts() - factored out after a review flagged the two as
+// independently duplicating bare-name resolution + child count + sampling.
+// Two copies of a count query is exactly the failure pattern behind the
+// v2.4.4 misfiled-rows incident and the v2.7.1 divergent-count bug: they
+// only need to disagree once for DescribeDelete to say "safe" while an
+// actual delete blocks, or vice versa. One implementation, two call sites.
+struct RelationLookup {
+    std::string bareRelation;              // resolveCollection(r.name).collection
+    uint64_t childCount = 0;
+    std::vector<std::string> sampleChildIds;   // populated only if childCount > 0
+    bool resolvable = false;               // false => r.name could not be resolved; caller must skip
+};
+
+RelationLookup lookupRelationCounts(smartbotic::db::storage::LmdbDocumentStore& store,
+                                     const smartbotic::database::RelationInfo& r,
+                                     const std::string& parentId) {
+    RelationLookup out;
+
+    // relation_index_* take the BARE relation name; RelationInfo::name is
+    // project-qualified. Malformed names should never happen (createRelation
+    // validates), but neither a delete nor a describe query is the place to
+    // throw over a data problem in an unrelated declaration - skip it.
+    try {
+        out.bareRelation = smartbotic::database::resolveCollection(r.name).collection;
+    } catch (const std::exception&) {
+        return out; // resolvable stays false
+    }
+    out.resolvable = true;
+
+    out.childCount = store.relation_index_child_count(out.bareRelation, parentId);
+    if (out.childCount > 0) {
+        out.sampleChildIds = store.relation_index_children(out.bareRelation, parentId, 5);
+    }
+    return out;
+}
+
+} // namespace
+
 namespace smartbotic::database {
 
 std::vector<RelationBlock> findRelationBlocks(
@@ -22,26 +63,15 @@ std::vector<RelationBlock> findRelationBlocks(
         // permissive until then - see the file header.
         if (r.onDelete != OnDelete::Restrict) continue;
 
-        // relation_index_* take the BARE relation name; RelationInfo::name
-        // is project-qualified.
-        std::string bareRelation;
-        try {
-            bareRelation = resolveCollection(r.name).collection;
-        } catch (const std::exception&) {
-            // Malformed relation name should never happen (createRelation
-            // validates it), but a delete is not the place to throw over a
-            // data problem in an unrelated declaration - skip it.
-            continue;
-        }
-
-        uint64_t count = store.relation_index_child_count(bareRelation, parentId);
-        if (count == 0) continue; // no reference -> nothing to block on
+        const auto lookup = lookupRelationCounts(store, r, parentId);
+        if (!lookup.resolvable) continue;
+        if (lookup.childCount == 0) continue; // no reference -> nothing to block on
 
         RelationBlock block;
         block.relation = r.name;
         block.childCollection = r.child;
-        block.childCount = count;
-        block.sampleChildIds = store.relation_index_children(bareRelation, parentId, 5);
+        block.childCount = lookup.childCount;
+        block.sampleChildIds = lookup.sampleChildIds;
         out.push_back(std::move(block));
     }
 
@@ -56,28 +86,17 @@ std::vector<RelationImpact> describeDeleteImpacts(
     std::vector<RelationImpact> out;
 
     for (const auto& r : relations.relationsWithParent(qualifiedParentCollection)) {
-        // relation_index_* take the BARE relation name; RelationInfo::name
-        // is project-qualified. Same guard as findRelationBlocks: a
-        // malformed name should never happen (createRelation validates),
-        // but a describe query is not the place to throw over a data
-        // problem in an unrelated declaration - skip it.
-        std::string bareRelation;
-        try {
-            bareRelation = resolveCollection(r.name).collection;
-        } catch (const std::exception&) {
-            continue;
-        }
+        const auto lookup = lookupRelationCounts(store, r, parentId);
+        if (!lookup.resolvable) continue;
 
         RelationImpact impact;
         impact.relation = r.name;
         impact.childCollection = r.child;
         impact.childField = r.childField;
         impact.onDelete = r.onDelete;
-        impact.childCount = store.relation_index_child_count(bareRelation, parentId);
-        if (impact.childCount > 0) {
-            impact.sampleChildIds = store.relation_index_children(bareRelation, parentId, 5);
-        }
-        impact.blocks = (r.onDelete == OnDelete::Restrict) && (impact.childCount > 0);
+        impact.childCount = lookup.childCount;
+        impact.sampleChildIds = lookup.sampleChildIds;
+        impact.blocks = (r.onDelete == OnDelete::Restrict) && (lookup.childCount > 0);
 
         out.push_back(std::move(impact));
     }