Переглянути джерело

refactor(storage): lift filter evaluation to shared filter_eval.hpp

Both MemoryStore::matchesFilters (and helpers getJsonPath, compareJson,
matchesSearch, searchInJson) and document_store_lmdb.cpp's anonymous-
namespace copies (matches_filters, get_json_path, compare_json,
matches_search, search_in_json, resolve_filter_value) collapse onto a
single header-only filter_eval namespace.

Behaviour preserved bit-for-bit. The lift exists so the upcoming Get
shadow-read can attribute any divergence to LMDB state alone, not to
two private filter copies drifting apart over time.

Net: -353 lines, +13 lines across two callers + the new header.
Full test suite (12/12) stays green — test_document_store, test_views,
test_migrate_v1_to_v2 are the heaviest filter-coverage exercisers.
fszontagh 2 місяців тому
батько
коміт
4eeb784bc1

+ 6 - 200
service/src/memory_store.cpp

@@ -2,6 +2,7 @@
 
 #include "config/collection_config_manager.hpp"
 #include "persistence/history_store.hpp"
+#include "storage/filter_eval.hpp"
 
 #if defined(__GLIBC__) && !defined(__APPLE__)
 #include <malloc.h>
@@ -2127,218 +2128,23 @@ uint64_t MemoryStore::currentTimeFor(const std::string& collection) const {
 }
 
 bool MemoryStore::matchesFilters(const Document& doc, const std::vector<Filter>& filters) const {
-    for (const auto& filter : filters) {
-        // DB-system fields live on the Document protobuf, not inside doc.data.
-        // The sort path already special-cases them (see line ~847); the filter
-        // path needs the same treatment, otherwise clients can sort by
-        // `_created_at` but never filter by it (cursor-based pagination
-        // breaks — every `_created_at < X` probe returns empty).
-        std::optional<nlohmann::json> value;
-        if (filter.field == "_created_at") {
-            value = nlohmann::json(doc.createdAt);
-        } else if (filter.field == "_updated_at") {
-            value = nlohmann::json(doc.updatedAt);
-        } else if (filter.field == "_id") {
-            value = nlohmann::json(doc.id);
-        } else if (filter.field == "_version") {
-            value = nlohmann::json(doc.version);
-        } else if (filter.field.find('.') == std::string::npos) {
-            // Top-level field: fast-path probe so we avoid materialising the
-            // full tree on every filter check.
-            value = doc.field(filter.field);
-        } else {
-            value = getJsonPath(doc.data(), filter.field);
-        }
-
-        switch (filter.op) {
-            case FilterOp::EXISTS:
-                if (value.has_value() != filter.value.get<bool>()) {
-                    return false;
-                }
-                break;
-
-            case FilterOp::EQ:
-            case FilterOp::NE:
-            case FilterOp::GT:
-            case FilterOp::GTE:
-            case FilterOp::LT:
-            case FilterOp::LTE:
-                if (!value) {
-                    return false;
-                }
-                if (!compareJson(*value, filter.op, filter.value)) {
-                    return false;
-                }
-                break;
-
-            case FilterOp::IN:
-                if (!value || !filter.value.is_array()) {
-                    return false;
-                }
-                {
-                    bool found = false;
-                    for (const auto& v : filter.value) {
-                        if (*value == v) {
-                            found = true;
-                            break;
-                        }
-                    }
-                    if (!found) return false;
-                }
-                break;
-
-            case FilterOp::CONTAINS:
-                if (!value || !value->is_array()) {
-                    return false;
-                }
-                {
-                    bool found = false;
-                    for (const auto& v : *value) {
-                        if (v == filter.value) {
-                            found = true;
-                            break;
-                        }
-                    }
-                    if (!found) return false;
-                }
-                break;
-
-            case FilterOp::REGEX:
-                if (!value || !value->is_string() || !filter.value.is_string()) {
-                    return false;
-                }
-                try {
-                    std::regex re(filter.value.get<std::string>());
-                    if (!std::regex_search(value->get<std::string>(), re)) {
-                        return false;
-                    }
-                } catch (const std::regex_error&) {
-                    return false;
-                }
-                break;
-
-            case FilterOp::SEARCH:
-                // Full-text search across document ID and string fields
-                if (!filter.value.is_string()) {
-                    return false;
-                }
-                if (!matchesSearch(doc, filter.value.get<std::string>())) {
-                    return false;
-                }
-                break;
-        }
-    }
-
-    return true;
+    return smartbotic::db::storage::filter_eval::matchesFilters(doc, filters);
 }
 
 std::optional<nlohmann::json> MemoryStore::getJsonPath(const nlohmann::json& obj, const std::string& path) {
-    if (path.empty() || !obj.is_object()) {
-        return std::nullopt;
-    }
-
-    // Split path by '.'
-    std::vector<std::string> parts;
-    std::istringstream iss(path);
-    std::string part;
-    while (std::getline(iss, part, '.')) {
-        parts.push_back(part);
-    }
-
-    const nlohmann::json* current = &obj;
-    for (const auto& p : parts) {
-        if (!current->is_object() || !current->contains(p)) {
-            return std::nullopt;
-        }
-        current = &(*current)[p];
-    }
-
-    return std::make_optional(*current);
+    return smartbotic::db::storage::filter_eval::getJsonPath(obj, path);
 }
 
 bool MemoryStore::compareJson(const nlohmann::json& a, FilterOp op, const nlohmann::json& b) {
-    switch (op) {
-        case FilterOp::EQ:
-            return a == b;
-        case FilterOp::NE:
-            return a != b;
-        case FilterOp::GT:
-            return a > b;
-        case FilterOp::GTE:
-            return a >= b;
-        case FilterOp::LT:
-            return a < b;
-        case FilterOp::LTE:
-            return a <= b;
-        default:
-            return false;
-    }
+    return smartbotic::db::storage::filter_eval::compareJson(a, op, b);
 }
 
 bool MemoryStore::matchesSearch(const Document& doc, const std::string& searchTerm) const {
-    if (searchTerm.empty()) {
-        return true;  // Empty search matches everything
-    }
-
-    // Convert search term to lowercase for case-insensitive matching
-    std::string lowerSearchTerm = searchTerm;
-    std::transform(lowerSearchTerm.begin(), lowerSearchTerm.end(), lowerSearchTerm.begin(),
-                   [](unsigned char c) { return std::tolower(c); });
-
-    // Check document ID
-    std::string lowerId = doc.id;
-    std::transform(lowerId.begin(), lowerId.end(), lowerId.begin(),
-                   [](unsigned char c) { return std::tolower(c); });
-    if (lowerId.find(lowerSearchTerm) != std::string::npos) {
-        return true;
-    }
-
-    // Search in document data. Materialises the full tree (full-tree
-    // search has no field-fast-path equivalent).
-    return searchInJson(doc.data(), lowerSearchTerm);
+    return smartbotic::db::storage::filter_eval::matchesSearch(doc, searchTerm);
 }
 
 bool MemoryStore::searchInJson(const nlohmann::json& obj, const std::string& lowerSearchTerm) {
-    if (obj.is_string()) {
-        std::string lowerValue = obj.get<std::string>();
-        std::transform(lowerValue.begin(), lowerValue.end(), lowerValue.begin(),
-                       [](unsigned char c) { return std::tolower(c); });
-        return lowerValue.find(lowerSearchTerm) != std::string::npos;
-    }
-
-    if (obj.is_object()) {
-        for (const auto& [key, value] : obj.items()) {
-            // Search in keys too
-            std::string lowerKey = key;
-            std::transform(lowerKey.begin(), lowerKey.end(), lowerKey.begin(),
-                           [](unsigned char c) { return std::tolower(c); });
-            if (lowerKey.find(lowerSearchTerm) != std::string::npos) {
-                return true;
-            }
-            // Recursively search in values
-            if (searchInJson(value, lowerSearchTerm)) {
-                return true;
-            }
-        }
-    }
-
-    if (obj.is_array()) {
-        for (const auto& item : obj) {
-            if (searchInJson(item, lowerSearchTerm)) {
-                return true;
-            }
-        }
-    }
-
-    // For numbers, convert to string and search
-    if (obj.is_number()) {
-        std::string numStr = obj.dump();
-        if (numStr.find(lowerSearchTerm) != std::string::npos) {
-            return true;
-        }
-    }
-
-    return false;
+    return smartbotic::db::storage::filter_eval::searchInJson(obj, lowerSearchTerm);
 }
 
 void MemoryStore::emitEvent(EventType type, const std::string& collection, const std::string& id,

+ 7 - 153
service/src/storage/document_store_lmdb.cpp

@@ -39,6 +39,7 @@
 #include "doc_binary.hpp"
 #include "document.hpp"
 #include "json_parse.hpp"
+#include "storage/filter_eval.hpp"
 #include "storage/lmdb_dbi.hpp"
 #include "storage/lmdb_env.hpp"
 #include "storage/lmdb_txn.hpp"
@@ -97,156 +98,9 @@ smartbotic::database::Document decode_document(std::string_view bytes) {
     return smartbotic::database::Document::fromJson(j);
 }
 
-// -------------------------------------------------------------------------
-// Filter / sort / projection — ported from MemoryStore::find /
-// MemoryStore::matchesFilters / etc (service/src/memory_store.cpp). The
-// behaviour matches v1.x bit-for-bit: dotted-path resolution for nested
-// fields, special-cased struct members (_id / _created_at / _updated_at /
-// _version), and the same regex / SEARCH semantics.
-// -------------------------------------------------------------------------
-
-std::optional<nlohmann::json> get_json_path(const nlohmann::json& obj,
-                                             const std::string& path) {
-    if (path.empty() || !obj.is_object()) return std::nullopt;
-    std::vector<std::string> parts;
-    std::istringstream iss(path);
-    std::string part;
-    while (std::getline(iss, part, '.')) parts.push_back(part);
-    const nlohmann::json* current = &obj;
-    for (const auto& p : parts) {
-        if (!current->is_object() || !current->contains(p)) return std::nullopt;
-        current = &(*current)[p];
-    }
-    return std::optional<nlohmann::json>(*current);
-}
-
-bool compare_json(const nlohmann::json& a,
-                  smartbotic::database::FilterOp op,
-                  const nlohmann::json& b) {
-    using smartbotic::database::FilterOp;
-    switch (op) {
-        case FilterOp::EQ:  return a == b;
-        case FilterOp::NE:  return a != b;
-        case FilterOp::GT:  return a > b;
-        case FilterOp::GTE: return a >= b;
-        case FilterOp::LT:  return a < b;
-        case FilterOp::LTE: return a <= b;
-        default:            return false;
-    }
-}
-
-bool search_in_json(const nlohmann::json& obj,
-                    const std::string& lower_term) {
-    if (obj.is_string()) {
-        std::string lower = obj.get<std::string>();
-        std::transform(lower.begin(), lower.end(), lower.begin(),
-                       [](unsigned char c) { return std::tolower(c); });
-        return lower.find(lower_term) != std::string::npos;
-    }
-    if (obj.is_object()) {
-        for (const auto& [k, v] : obj.items()) {
-            std::string lower_key = k;
-            std::transform(lower_key.begin(), lower_key.end(), lower_key.begin(),
-                           [](unsigned char c) { return std::tolower(c); });
-            if (lower_key.find(lower_term) != std::string::npos) return true;
-            if (search_in_json(v, lower_term)) return true;
-        }
-    }
-    if (obj.is_array()) {
-        for (const auto& item : obj) {
-            if (search_in_json(item, lower_term)) return true;
-        }
-    }
-    if (obj.is_number()) {
-        std::string s = obj.dump();
-        if (s.find(lower_term) != std::string::npos) return true;
-    }
-    return false;
-}
-
-bool matches_search(const smartbotic::database::Document& doc,
-                    const std::string& term) {
-    if (term.empty()) return true;
-    std::string lower = term;
-    std::transform(lower.begin(), lower.end(), lower.begin(),
-                   [](unsigned char c) { return std::tolower(c); });
-    std::string lower_id = doc.id;
-    std::transform(lower_id.begin(), lower_id.end(), lower_id.begin(),
-                   [](unsigned char c) { return std::tolower(c); });
-    if (lower_id.find(lower) != std::string::npos) return true;
-    return search_in_json(doc.data(), lower);
-}
-
-// Resolve the filter LHS value, honoring the same special-cased struct
-// members memory_store uses (_id, _created_at, _updated_at, _version),
-// the top-level fast path (no dot in name -> doc.field()), and the
-// dotted-path slow path (descends doc.data()).
-std::optional<nlohmann::json>
-resolve_filter_value(const smartbotic::database::Document& doc,
-                     const std::string& field) {
-    if (field == "_created_at") return nlohmann::json(doc.createdAt);
-    if (field == "_updated_at") return nlohmann::json(doc.updatedAt);
-    if (field == "_id")         return nlohmann::json(doc.id);
-    if (field == "_version")    return nlohmann::json(doc.version);
-    if (field.find('.') == std::string::npos) {
-        return doc.field(field);
-    }
-    return get_json_path(doc.data(), field);
-}
-
-bool matches_filters(const smartbotic::database::Document& doc,
-                     const std::vector<smartbotic::database::Filter>& filters) {
-    using smartbotic::database::FilterOp;
-    for (const auto& filter : filters) {
-        std::optional<nlohmann::json> value = resolve_filter_value(doc, filter.field);
-        switch (filter.op) {
-            case FilterOp::EXISTS:
-                if (value.has_value() != filter.value.get<bool>()) return false;
-                break;
-            case FilterOp::EQ:
-            case FilterOp::NE:
-            case FilterOp::GT:
-            case FilterOp::GTE:
-            case FilterOp::LT:
-            case FilterOp::LTE:
-                if (!value) return false;
-                if (!compare_json(*value, filter.op, filter.value)) return false;
-                break;
-            case FilterOp::IN: {
-                if (!value || !filter.value.is_array()) return false;
-                bool found = false;
-                for (const auto& v : filter.value) {
-                    if (*value == v) { found = true; break; }
-                }
-                if (!found) return false;
-                break;
-            }
-            case FilterOp::CONTAINS: {
-                if (!value || !value->is_array()) return false;
-                bool found = false;
-                for (const auto& v : *value) {
-                    if (v == filter.value) { found = true; break; }
-                }
-                if (!found) return false;
-                break;
-            }
-            case FilterOp::REGEX:
-                if (!value || !value->is_string() || !filter.value.is_string()) return false;
-                try {
-                    std::regex re(filter.value.get<std::string>());
-                    if (!std::regex_search(value->get<std::string>(), re)) return false;
-                } catch (const std::regex_error&) {
-                    return false;
-                }
-                break;
-            case FilterOp::SEARCH:
-                if (!filter.value.is_string()) return false;
-                if (!matches_search(doc, filter.value.get<std::string>())) return false;
-                break;
-        }
-    }
-    return true;
-}
+// Filter evaluation lives in service/src/storage/filter_eval.hpp now —
+// shared with MemoryStore so a shadow-read divergence can never be the
+// result of two private copies drifting apart.
 
 void sort_documents(std::vector<smartbotic::database::Document>& docs,
                     const smartbotic::database::Sort& sort) {
@@ -267,8 +121,8 @@ void sort_documents(std::vector<smartbotic::database::Document>& docs,
                 va = a.field(sort.field);
                 vb = b.field(sort.field);
             } else {
-                va = get_json_path(a.data(), sort.field);
-                vb = get_json_path(b.data(), sort.field);
+                va = smartbotic::db::storage::filter_eval::getJsonPath(a.data(), sort.field);
+                vb = smartbotic::db::storage::filter_eval::getJsonPath(b.data(), sort.field);
             }
             if (!va && !vb) {
                 return sort.descending ? a.id > b.id : a.id < b.id;
@@ -465,7 +319,7 @@ ScanResult LmdbDocumentStore::scan(std::string_view collection,
     int rc = mdb_cursor_get(cursor, &k, &v, MDB_FIRST);
     while (rc == MDB_SUCCESS) {
         smartbotic::database::Document doc = decode_document(to_sv(v));
-        if (matches_filters(doc, query.filters)) {
+        if (smartbotic::db::storage::filter_eval::matchesFilters(doc, query.filters)) {
             matches.push_back(std::move(doc));
         }
         rc = mdb_cursor_get(cursor, &k, &v, MDB_NEXT);

+ 188 - 0
service/src/storage/filter_eval.hpp

@@ -0,0 +1,188 @@
+// v2.0 Stage 4 — shared filter evaluation (lifted from both
+// MemoryStore::matchesFilters and document_store_lmdb.cpp's private copy).
+//
+// Both v1.x MemoryStore queries and v2.0 LmdbDocumentStore scans run the
+// exact same predicate against a Document: same operators (EQ/NE/GT/GTE/
+// LT/LTE/IN/CONTAINS/EXISTS/REGEX/SEARCH), same nested-path resolution,
+// same special-case handling of `_id` / `_created_at` / `_updated_at` /
+// `_version`, same case-insensitive SEARCH semantics. Keeping two private
+// copies was a behavioural-drift hazard — a silent divergence here would
+// show up in shadow-read logs as a filter mismatch that's impossible to
+// attribute to one side or the other.
+//
+// Header-only, inline. Pure functions; no state. Safe to call from any
+// thread, including inside LMDB read txns.
+
+#pragma once
+
+#include <algorithm>
+#include <cctype>
+#include <optional>
+#include <regex>
+#include <sstream>
+#include <string>
+#include <vector>
+
+#include <nlohmann/json.hpp>
+
+#include "document.hpp"
+
+namespace smartbotic::db::storage::filter_eval {
+
+// Resolve `path` (dot-separated for nested fields) against a JSON object.
+// Returns nullopt if the path doesn't exist or `obj` isn't an object.
+inline std::optional<nlohmann::json>
+getJsonPath(const nlohmann::json& obj, const std::string& path) {
+    if (path.empty() || !obj.is_object()) return std::nullopt;
+    std::vector<std::string> parts;
+    std::istringstream iss(path);
+    std::string part;
+    while (std::getline(iss, part, '.')) parts.push_back(part);
+    const nlohmann::json* current = &obj;
+    for (const auto& p : parts) {
+        if (!current->is_object() || !current->contains(p)) return std::nullopt;
+        current = &(*current)[p];
+    }
+    return std::optional<nlohmann::json>(*current);
+}
+
+// Compare two JSON values under one of the relational operators. Returns
+// false for non-relational ops (the caller must dispatch those separately).
+inline bool compareJson(const nlohmann::json& a,
+                        smartbotic::database::FilterOp op,
+                        const nlohmann::json& b) {
+    using smartbotic::database::FilterOp;
+    switch (op) {
+        case FilterOp::EQ:  return a == b;
+        case FilterOp::NE:  return a != b;
+        case FilterOp::GT:  return a > b;
+        case FilterOp::GTE: return a >= b;
+        case FilterOp::LT:  return a < b;
+        case FilterOp::LTE: return a <= b;
+        default:            return false;
+    }
+}
+
+// Recursive lowercase-substring search through a JSON value. `lower_term`
+// must already be lowercased by the caller (cheaper than re-lowercasing
+// on every recursion). Numbers are dumped to string and matched too.
+inline bool searchInJson(const nlohmann::json& obj,
+                         const std::string& lower_term) {
+    if (obj.is_string()) {
+        std::string lower = obj.get<std::string>();
+        std::transform(lower.begin(), lower.end(), lower.begin(),
+                       [](unsigned char c) { return std::tolower(c); });
+        return lower.find(lower_term) != std::string::npos;
+    }
+    if (obj.is_object()) {
+        for (const auto& [k, v] : obj.items()) {
+            std::string lower_key = k;
+            std::transform(lower_key.begin(), lower_key.end(), lower_key.begin(),
+                           [](unsigned char c) { return std::tolower(c); });
+            if (lower_key.find(lower_term) != std::string::npos) return true;
+            if (searchInJson(v, lower_term)) return true;
+        }
+    }
+    if (obj.is_array()) {
+        for (const auto& item : obj) {
+            if (searchInJson(item, lower_term)) return true;
+        }
+    }
+    if (obj.is_number()) {
+        std::string s = obj.dump();
+        if (s.find(lower_term) != std::string::npos) return true;
+    }
+    return false;
+}
+
+// FilterOp::SEARCH semantics — case-insensitive substring across doc.id
+// + every string/key/number inside doc.data(). Empty term matches all.
+inline bool matchesSearch(const smartbotic::database::Document& doc,
+                          const std::string& term) {
+    if (term.empty()) return true;
+    std::string lower = term;
+    std::transform(lower.begin(), lower.end(), lower.begin(),
+                   [](unsigned char c) { return std::tolower(c); });
+    std::string lower_id = doc.id;
+    std::transform(lower_id.begin(), lower_id.end(), lower_id.begin(),
+                   [](unsigned char c) { return std::tolower(c); });
+    if (lower_id.find(lower) != std::string::npos) return true;
+    return searchInJson(doc.data(), lower);
+}
+
+// Resolve a filter's LHS value, honouring:
+//  - DB-system fields stored on the Document struct (_id, _created_at,
+//    _updated_at, _version) — never inside doc.data().
+//  - Top-level field fast path (no dot, uses doc.field() to avoid
+//    materialising the full JSON tree).
+//  - Nested dotted path (descends doc.data()).
+inline std::optional<nlohmann::json>
+resolveFilterValue(const smartbotic::database::Document& doc,
+                   const std::string& field) {
+    if (field == "_created_at") return nlohmann::json(doc.createdAt);
+    if (field == "_updated_at") return nlohmann::json(doc.updatedAt);
+    if (field == "_id")         return nlohmann::json(doc.id);
+    if (field == "_version")    return nlohmann::json(doc.version);
+    if (field.find('.') == std::string::npos) {
+        return doc.field(field);
+    }
+    return getJsonPath(doc.data(), field);
+}
+
+// AND-evaluate every filter in `filters` against `doc`. Returns true iff
+// every filter matches; an empty filter list matches.
+inline bool matchesFilters(const smartbotic::database::Document& doc,
+                           const std::vector<smartbotic::database::Filter>& filters) {
+    using smartbotic::database::FilterOp;
+    for (const auto& filter : filters) {
+        std::optional<nlohmann::json> value = resolveFilterValue(doc, filter.field);
+        switch (filter.op) {
+            case FilterOp::EXISTS:
+                if (value.has_value() != filter.value.get<bool>()) return false;
+                break;
+            case FilterOp::EQ:
+            case FilterOp::NE:
+            case FilterOp::GT:
+            case FilterOp::GTE:
+            case FilterOp::LT:
+            case FilterOp::LTE:
+                if (!value) return false;
+                if (!compareJson(*value, filter.op, filter.value)) return false;
+                break;
+            case FilterOp::IN: {
+                if (!value || !filter.value.is_array()) return false;
+                bool found = false;
+                for (const auto& v : filter.value) {
+                    if (*value == v) { found = true; break; }
+                }
+                if (!found) return false;
+                break;
+            }
+            case FilterOp::CONTAINS: {
+                if (!value || !value->is_array()) return false;
+                bool found = false;
+                for (const auto& v : *value) {
+                    if (v == filter.value) { found = true; break; }
+                }
+                if (!found) return false;
+                break;
+            }
+            case FilterOp::REGEX:
+                if (!value || !value->is_string() || !filter.value.is_string()) return false;
+                try {
+                    std::regex re(filter.value.get<std::string>());
+                    if (!std::regex_search(value->get<std::string>(), re)) return false;
+                } catch (const std::regex_error&) {
+                    return false;
+                }
+                break;
+            case FilterOp::SEARCH:
+                if (!filter.value.is_string()) return false;
+                if (!matchesSearch(doc, filter.value.get<std::string>())) return false;
+                break;
+        }
+    }
+    return true;
+}
+
+}  // namespace smartbotic::db::storage::filter_eval