Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/MeshWeaver.Hosting.PostgreSql/PostgreSqlMeshQuery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ public IObservable<QueryResultChange<T>> Query<T>(MeshQueryRequest request, Json
// The Initial snapshot itself uses request.Queries via QueryAsync, but the
// change-detection layer must observe the union of all branches' shapes.
var effectiveQueries = request.EffectiveQueries;
var parsedFilters = new List<(string BasePath, QueryScope Scope)>(effectiveQueries.Count);
var parsedFilters = new List<(string BasePath, QueryScope Scope, IReadOnlyList<string> Namespaces)>(effectiveQueries.Count);
ParsedQuery firstParsed = null!;
for (var qi = 0; qi < effectiveQueries.Count; qi++)
{
Expand All @@ -606,7 +606,10 @@ public IObservable<QueryResultChange<T>> Query<T>(MeshQueryRequest request, Json
effectiveScope = QueryScope.Children;
}
var normalizedBasePath = effectivePath?.Trim('/') ?? "";
parsedFilters.Add((normalizedBasePath, effectiveScope));
// Capture the query's NAMESPACE filters too: a namespace-only query (no path:) leaves
// BasePath empty, so the path/scope check alone judges deep changes out of scope. The
// namespaces let a delete/update/create under {owner}/*_Thread trigger a re-query.
parsedFilters.Add((normalizedBasePath, effectiveScope, pq.ExtractNamespacePatterns()));
if (qi == 0) firstParsed = pq;
}

Expand Down Expand Up @@ -651,7 +654,7 @@ public IObservable<QueryResultChange<T>> Query<T>(MeshQueryRequest request, Json

var earlySubscription = _adapter.Changes
.Where(n => parsedFilters.Any(f =>
PathMatcher.ShouldNotify(n.Path, f.BasePath, f.Scope)))
PathMatcher.ShouldNotifyForQuery(n.Path, f.BasePath, f.Scope, f.Namespaces)))
.Subscribe(n =>
{
lock (earlyLock)
Expand Down Expand Up @@ -680,7 +683,7 @@ public IObservable<QueryResultChange<T>> Query<T>(MeshQueryRequest request, Json
disposables.Add(
_adapter.Changes
.Where(n => parsedFilters.Any(f =>
PathMatcher.ShouldNotify(n.Path, f.BasePath, f.Scope)))
PathMatcher.ShouldNotifyForQuery(n.Path, f.BasePath, f.Scope, f.Namespaces)))
.Subscribe(changeBuffer));
// 🚨 Strict unit-of-work + zero debounce: every change
// triggers its own RunQuery, serialised via Concat so the
Expand Down
66 changes: 66 additions & 0 deletions src/MeshWeaver.Hosting/Persistence/Query/PathMatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,4 +138,70 @@ private static string[] GetSegments(string path)

return path.Split('/', StringSplitOptions.RemoveEmptyEntries);
}

/// <summary>
/// Relevance test for a live synced query: a CRUD change at <paramref name="changedPath"/> should
/// trigger a re-query when EITHER its path matches the query's path + <paramref name="scope"/>
/// (<see cref="ShouldNotify"/>), OR the changed node's NAMESPACE matches one of the query's namespace
/// filters (<paramref name="queryNamespaces"/>).
/// <para>The path-only <see cref="ShouldNotify"/> is blind to NAMESPACE-filtered queries: the
/// "open threads" catalog queries <c>namespace:{owner}/*_Thread</c> with NO <c>path:</c> term, so its
/// base path is empty and ShouldNotify degrades to "direct children of root". A thread deleted three
/// levels deep (<c>{owner}/_Thread/{id}</c>) is then judged out of scope → no re-query → the catalog
/// never refreshes on delete. Matching the changed node's namespace (the path minus its last segment)
/// against the query namespaces closes that gap for create/update/delete alike. Broadening relevance
/// is correctness-safe: the worst case is a redundant re-query, never a missed change.</para>
/// </summary>
public static bool ShouldNotifyForQuery(
string changedPath, string queryBasePath, QueryScope scope, IReadOnlyList<string>? queryNamespaces)
=> ShouldNotify(changedPath, queryBasePath, scope)
|| NamespaceInScope(NamespaceOf(changedPath), queryNamespaces);

/// <summary>The namespace a node at <paramref name="path"/> lives in — the path minus its last segment
/// (a thread <c>{owner}/_Thread/{id}</c> → <c>{owner}/_Thread</c>). Empty for a top-level node.</summary>
public static string NamespaceOf(string path)
{
var normalized = NormalizePath(path);
var i = normalized.LastIndexOf('/');
return i > 0 ? normalized[..i] : "";
}

/// <summary>
/// True when <paramref name="ns"/> matches any of <paramref name="patterns"/>. A pattern may carry a
/// single <c>*</c> glob (<c>{owner}/*_Thread</c> → starts-with the pre-<c>*</c> part AND ends-with the
/// post-<c>*</c> part); a pattern without <c>*</c> matches when <paramref name="ns"/> equals it OR is a
/// sub-namespace under it (robust to whether the parser hands back the glob pattern or its base).
/// </summary>
public static bool NamespaceInScope(string ns, IReadOnlyList<string>? patterns)
{
if (patterns is null || patterns.Count == 0 || string.IsNullOrEmpty(ns))
return false;
var value = NormalizePath(ns);
for (var i = 0; i < patterns.Count; i++)
if (GlobMatch(value, NormalizePath(patterns[i])))
return true;
return false;
}

private static bool GlobMatch(string value, string pattern)
{
if (string.IsNullOrEmpty(pattern))
return false;
// The query parser emits a namespace wildcard as SQL-LIKE '%' (it rewrites the user's '*' →
// '%'); accept either. A single wildcard splits the pattern into a prefix + suffix (e.g.
// "{owner}/%_Thread" → "{owner}/" … "_Thread"); the surrounding literals (incl. the satellite
// "_" in "_Thread") are matched verbatim, which is what real satellite namespaces need.
var star = pattern.IndexOfAny(['*', '%']);
if (star >= 0)
{
var prefix = pattern[..star];
var suffix = pattern[(star + 1)..];
return value.Length >= prefix.Length + suffix.Length
&& value.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)
&& value.EndsWith(suffix, StringComparison.OrdinalIgnoreCase);
}
// No glob: exact namespace, or a sub-namespace under it.
return string.Equals(value, pattern, StringComparison.OrdinalIgnoreCase)
|| value.StartsWith(pattern + "/", StringComparison.OrdinalIgnoreCase);
}
}
33 changes: 33 additions & 0 deletions src/MeshWeaver.Mesh.Contract/Query/ParsedQuery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,39 @@ private static void ExtractNamespacesFromNode(QueryNode node, List<string> colle
}
}

/// <summary>
/// Namespace filter values INCLUDING wildcard (<c>Like</c>) patterns — e.g. <c>{owner}/*_Thread</c>.
/// <see cref="ExtractNamespaces"/> deliberately collects only concrete <c>Equal</c>/<c>In</c>
/// namespaces (the aggregator ROUTES on those, and can't route a glob); this variant ALSO returns the
/// <c>Like</c> glob patterns, for the live change-relevance filter that decides whether a CRUD event
/// under a namespace should refresh a synced query (see <c>PathMatcher.ShouldNotifyForQuery</c>). The
/// glob is preserved verbatim (the parser keeps the <c>*</c>), so a matcher can apply it directly.
/// </summary>
public IReadOnlyList<string> ExtractNamespacePatterns()
{
if (Filter == null) return Array.Empty<string>();
var collected = new List<string>();
ExtractNamespacePatternsFromNode(Filter, collected);
return collected;
}

private static void ExtractNamespacePatternsFromNode(QueryNode node, List<string> collected)
{
switch (node)
{
case QueryComparison c when c.Condition.Selector.Equals("namespace", StringComparison.OrdinalIgnoreCase)
&& c.Condition.Operator is QueryOperator.Equal or QueryOperator.In or QueryOperator.Like:
collected.AddRange(c.Condition.Values);
break;
case QueryAnd and:
foreach (var child in and.Children) ExtractNamespacePatternsFromNode(child, collected);
break;
case QueryOr or:
foreach (var child in or.Children) ExtractNamespacePatternsFromNode(child, collected);
break;
}
}

/// <summary>
/// Projects an item down to only the requested properties.
/// Returns a dictionary with the selected property names and their values.
Expand Down
90 changes: 90 additions & 0 deletions test/MeshWeaver.Hosting.Test/QueryChangeRelevanceTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
using System.Collections.Generic;
using MeshWeaver.Hosting.Persistence.Query;
using MeshWeaver.Mesh;
using MeshWeaver.Mesh.Services;
using Xunit;

namespace MeshWeaver.Hosting.Test;

/// <summary>
/// The reactive change-feed RELEVANCE test — "does this CRUD broadcast event fit the query, so we should
/// re-run it?" (<see cref="PathMatcher.ShouldNotifyForQuery"/>). Pins the fix for the "open threads"
/// catalog not refreshing on delete.
///
/// <para><b>The bug.</b> The catalog queries by NAMESPACE — <c>namespace:{owner}/*_Thread</c> with NO
/// <c>path:</c> term — so its base path is empty and the old path-only <see cref="PathMatcher.ShouldNotify"/>
/// degraded to "direct children of root". A thread deleted three levels deep at
/// <c>{owner}/_Thread/{id}</c> was judged out of scope → no re-query → the catalog kept showing the
/// deleted thread. <see cref="PathMatcher.ShouldNotifyForQuery"/> also matches the changed node's
/// namespace against the query namespaces, so create / update / delete of a thread all refresh it.</para>
///
/// <para>Pure logic (path + parsed query), so every CRUD case is a unit test — no Postgres, no change
/// feed. The PG provider only supplies the path in its broadcast events (pg_notify carries the key, not
/// the row), which is exactly what this test drives.</para>
/// </summary>
public class QueryChangeRelevanceTest
{
private static readonly QueryParser Parser = new();
private const string Owner = "rbuergi";
private const string ThreadPath = Owner + "/_Thread/do-you-have-guardrails-6b4c";

// The exact "open threads" catalog query (UserActivityLayoutAreas.BuildOpenThreads).
private static (string BasePath, QueryScope Scope, IReadOnlyList<string> Namespaces) CatalogFilter()
{
var pq = Parser.Parse($"namespace:{Owner}/*_Thread nodeType:Thread -content.status:Done sort:LastModified-desc");
// Namespace-only query → no path: term → empty base path (mirrors PostgreSqlMeshQuery's derivation).
return (pq.Path ?? "", pq.Scope, pq.ExtractNamespacePatterns());
}

private static bool InScope(DataChangeNotification change)
{
var (basePath, scope, namespaces) = CatalogFilter();
return PathMatcher.ShouldNotifyForQuery(change.Path, basePath, scope, namespaces);
}

// ── All CRUD, in-scope (a thread under {owner}/_Thread) → must refresh the catalog ──

[Fact]
public void Created_thread_refreshes_catalog()
=> Assert.True(InScope(DataChangeNotification.Created(ThreadPath, entity: null)));

[Fact]
public void Updated_thread_refreshes_catalog() // e.g. status → Done: re-query then drops it (leave-scope)
=> Assert.True(InScope(DataChangeNotification.Updated(ThreadPath, entity: null)));

[Fact]
public void Deleted_thread_refreshes_catalog() // ← the reported bug
=> Assert.True(InScope(DataChangeNotification.Deleted(ThreadPath)));

// ── Out of scope → must NOT trigger a redundant re-query ──

[Fact]
public void Change_to_non_thread_node_same_partition_is_out_of_scope()
=> Assert.False(InScope(DataChangeNotification.Updated($"{Owner}/Docs/readme", entity: null)));

[Fact]
public void Delete_of_non_thread_node_same_partition_is_out_of_scope()
=> Assert.False(InScope(DataChangeNotification.Deleted($"{Owner}/Docs/readme")));

[Fact]
public void Change_in_a_different_partition_is_out_of_scope()
=> Assert.False(InScope(DataChangeNotification.Deleted("acme/_Thread/other")));

// ── The namespace glob directly ──

[Fact]
public void Namespace_glob_matches_satellite_thread_namespace()
=> Assert.True(PathMatcher.NamespaceInScope($"{Owner}/_Thread", new[] { $"{Owner}/*_Thread" }));

[Fact]
public void Namespace_glob_rejects_non_matching_namespace()
=> Assert.False(PathMatcher.NamespaceInScope($"{Owner}/Docs", new[] { $"{Owner}/*_Thread" }));

[Fact]
public void NamespaceOf_strips_the_last_segment()
=> Assert.Equal($"{Owner}/_Thread", PathMatcher.NamespaceOf(ThreadPath));

[Fact]
public void Namespace_glob_matches_the_parser_emitted_like_pattern() // parser rewrites * → SQL-LIKE %
=> Assert.True(PathMatcher.NamespaceInScope($"{Owner}/_Thread", new[] { $"{Owner}/%_Thread" }));
}
Loading