From 20be3590a6793959dc58319010edf37e08b8c72f Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Wed, 21 Oct 2026 12:01:05 +0300 Subject: [PATCH 1/4] fix: show which subscriptions use a global value set The subscription list withholds adapter properties, so the admin UI found no references and called every value set unused and safe to delete. The list now names the sets, keys and partner properties each subscription references, and deleting a set still in use is refused. --- SW.Bitween.Api/Helpers/StartupValuesFiller.cs | 36 ++++ .../GlobalAdapterValuesSets/Delete.cs | 46 +++++ .../Resources/Subscriptions/Search.cs | 16 +- .../Tests/GlobalValueReferenceTests.cs | 175 ++++++++++++++++++ SW.Bitween.Sdk/Model/Subscription.cs | 24 +++ SW.Bitween.UnitTests/ReferenceTokensTests.cs | 51 +++++ .../ClientApp/src/api/http/globalValues.ts | 17 +- .../ClientApp/src/api/http/references.ts | 44 ++--- .../ClientApp/src/api/http/subscriptions.ts | 16 +- .../global-values/__tests__/UsedBy.test.tsx | 53 ++++++ docs/adapters.md | 2 +- docs/api-reference.md | 4 +- 12 files changed, 426 insertions(+), 58 deletions(-) create mode 100644 SW.Bitween.IntegrationTests/Tests/GlobalValueReferenceTests.cs create mode 100644 SW.Bitween.UnitTests/ReferenceTokensTests.cs create mode 100644 SW.Bitween.Web/ClientApp/src/pages/global-values/__tests__/UsedBy.test.tsx diff --git a/SW.Bitween.Api/Helpers/StartupValuesFiller.cs b/SW.Bitween.Api/Helpers/StartupValuesFiller.cs index b1ebb2b5..acff1b53 100644 --- a/SW.Bitween.Api/Helpers/StartupValuesFiller.cs +++ b/SW.Bitween.Api/Helpers/StartupValuesFiller.cs @@ -83,6 +83,42 @@ public static IEnumerable SourcePathsIn(IEnumerable values) => .SelectMany(v => SourceToken.Matches(v).Select(m => m.Groups[1].Value)) .Distinct(); + // As permissive as the resolver below: it splits a globals token on the first dot only and + // restricts neither half. + private static readonly Regex GlobalToken = new(@"\{\{globals\.([^.}]+)\.([^}]+)\}\}", RegexOptions.IgnoreCase); + private static readonly Regex PartnerToken = new(@"\{\{partner\.([^}]+)\}\}", RegexOptions.IgnoreCase); + + /// + /// The global values and partner properties name, by + /// {{globals.SET.KEY}} and {{partner.KEY}}. Names only, never what they resolve + /// to, so this can go where the values themselves can't. + /// + /// + /// Set ids and keys are told apart ignoring case, the way the resolver looks them up, and kept + /// as first written. + /// + public static Model.SubscriptionReferences ReferencesIn(IEnumerable values) + { + var present = (values ?? []).Where(v => v != null).ToList(); + + var globals = present + .SelectMany(v => GlobalToken.Matches(v)) + .GroupBy(m => m.Groups[1].Value, StringComparer.OrdinalIgnoreCase) + .Select(set => new Model.GlobalValuesReference + { + SetId = set.Key, + Keys = set.Select(m => m.Groups[2].Value).Distinct(StringComparer.OrdinalIgnoreCase).ToList() + }) + .ToList(); + + var partnerProperties = present + .SelectMany(v => PartnerToken.Matches(v).Select(m => m.Groups[1].Value)) + .Distinct(StringComparer.OrdinalIgnoreCase) + .ToList(); + + return new Model.SubscriptionReferences { Globals = globals, PartnerProperties = partnerProperties }; + } + public static Dictionary Fill(this IDictionary inputTemplated, Partner partner, GlobalAdapterValuesSet[] globals) { diff --git a/SW.Bitween.Api/Resources/GlobalAdapterValuesSets/Delete.cs b/SW.Bitween.Api/Resources/GlobalAdapterValuesSets/Delete.cs index cabbbb81..cae8de7c 100644 --- a/SW.Bitween.Api/Resources/GlobalAdapterValuesSets/Delete.cs +++ b/SW.Bitween.Api/Resources/GlobalAdapterValuesSets/Delete.cs @@ -1,4 +1,8 @@ +using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; using SW.Bitween.Domain; using SW.Bitween.Model; using SW.PrimitiveTypes; @@ -17,10 +21,52 @@ public async Task Handle(string key, DeleteGlobalAdapterValuesSetModel _ if (entity is null) throw new SWValidationException("NOT_FOUND", $"GlobalAdapterValuesSet with id {key} was not found"); + await EnsureNothingReferencesIt(key); + dbContext.Remove(entity); await dbContext.SaveChangesAsync(); await cache.BroadcastRevoke(); return null; } + + /// Refuses while a subscription or a notification channel names the set in a token. + /// + /// A token whose set is gone is no error anywhere: it reaches the adapter as written, and the + /// adapter fails at its next run on a password of {{globals.smtp.password}}, with + /// nothing pointing back here. Read in memory because the properties are encrypted at rest, + /// so the database can't search them. + /// + private async Task EnsureNothingReferencesIt(string setId) + { + bool References(params IReadOnlyDictionary[] properties) => + StartupValuesFiller.ReferencesIn(properties.Where(p => p != null).SelectMany(p => p.Values)) + .Globals.Any(g => g.SetId.Equals(setId, StringComparison.OrdinalIgnoreCase)); + + var subscriptions = (await dbContext.Set().AsNoTracking() + .Select(s => new { s.Name, s.HandlerProperties, s.MapperProperties, s.ReceiverProperties, s.ValidatorProperties }) + .ToListAsync()) + .Where(s => References(s.HandlerProperties, s.MapperProperties, s.ReceiverProperties, s.ValidatorProperties)) + .Select(s => s.Name); + + var channels = (await dbContext.Set().AsNoTracking() + .Select(c => new { c.Name, c.HandlerProperties }) + .ToListAsync()) + .Where(c => References(c.HandlerProperties)) + .Select(c => $"the channel {c.Name}"); + + var users = subscriptions.Concat(channels).ToList(); + if (users.Count == 0) return; + + throw new SWValidationException("GLOBAL_VALUES_IN_USE", + $"'{setId}' is still referenced by {Describe(users)}. Change those references first."); + } + + /// A few names and how many more - a set can be used by dozens of subscriptions. + private static string Describe(List names) => names.Count switch + { + 1 => names[0], + <= 3 => $"{string.Join(", ", names.Take(names.Count - 1))} and {names[^1]}", + _ => $"{string.Join(", ", names.Take(3))} and {names.Count - 3} more" + }; } } diff --git a/SW.Bitween.Api/Resources/Subscriptions/Search.cs b/SW.Bitween.Api/Resources/Subscriptions/Search.cs index 7bc6c4e0..ad1edb4d 100644 --- a/SW.Bitween.Api/Resources/Subscriptions/Search.cs +++ b/SW.Bitween.Api/Resources/Subscriptions/Search.cs @@ -134,7 +134,10 @@ join document in _dbContext.Set() on subscriber.DocumentId equals docu }; } - /// Empties the adapter property collections on every returned row. + /// + /// Empties the adapter property collections on every returned row, keeping only the names + /// of the global values and partner properties they reference. + /// /// /// Adapter properties hold credentials - a storage key, an OAuth secret, the password for a /// partner system. Subscriptions/Get replaces those with a sentinel before answering; @@ -152,11 +155,22 @@ join document in _dbContext.Set() on subscriber.DocumentId equals docu /// [Secure], and the external adapters declare nothing - masking here would have /// looked like a fix while leaving those integrations exactly as exposed. /// + /// + /// The references are read here, from the same properties, because the admin UI answers + /// "used by" for value sets and partner properties from this list. When it scanned the + /// properties itself, emptying them left every value set looking unused and safe to delete. + /// /// private static void StripAdapterProperties(List rows) { foreach (var row in rows) { + row.References = StartupValuesFiller.ReferencesIn( + new[] { row.HandlerProperties, row.MapperProperties, row.ReceiverProperties, row.ValidatorProperties } + .Where(properties => properties != null) + .SelectMany(properties => properties) + .Select(property => property.Value)); + row.HandlerProperties = []; row.MapperProperties = []; row.ReceiverProperties = []; diff --git a/SW.Bitween.IntegrationTests/Tests/GlobalValueReferenceTests.cs b/SW.Bitween.IntegrationTests/Tests/GlobalValueReferenceTests.cs new file mode 100644 index 00000000..a47e809e --- /dev/null +++ b/SW.Bitween.IntegrationTests/Tests/GlobalValueReferenceTests.cs @@ -0,0 +1,175 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using SW.Bitween.Domain; +using SW.Bitween.IntegrationTests.Fixtures; +using SW.Bitween.Model; +using SW.PrimitiveTypes; +using Xunit; + +namespace SW.Bitween.IntegrationTests.Tests; + +/// +/// Who uses a global value set. The subscription list withholds adapter properties, since they hold +/// credentials, so it says by name which value sets and partner properties each subscription +/// references - and deleting a set that something still references is refused. +/// +/// +/// Before this the admin UI read the references out of the list's properties itself. Once those were +/// withheld it found none, and every value set read "Not referenced anywhere yet - safe to delete" +/// while dozens of subscriptions resolved their credentials from it. +/// +[Collection("Bitween")] +public class GlobalValueReferenceTests(BitweenFixture fixture) +{ + [Fact] + public async Task The_list_names_what_a_subscription_references_and_still_withholds_the_values() + { + var subscriptionId = await CreateSubscriptionAsync( + handler: new() { ["Password"] = "{{globals.Spaces.secretKey}}", ["Url"] = "https://{{globals.spaces.host}}/up" }, + mapper: new() { ["Token"] = "{{GLOBALS.spaces.SECRETKEY}}" }, + receiver: new() { ["ApiKey"] = "{{partner.apiKey}}", ["Plain"] = "sk-live-9" }); + + var row = await ListRowAsync(subscriptionId); + + var set = Assert.Single(row.References.Globals); + Assert.Equal("Spaces", set.SetId, ignoreCase: true); + // Two spellings of one key are one key, as the resolver reads them. + Assert.Equal(new[] { "host", "secretKey" }, set.Keys.Order(StringComparer.OrdinalIgnoreCase), StringComparer.OrdinalIgnoreCase); + Assert.Equal(new[] { "apiKey" }, row.References.PartnerProperties); + + // Names only: the values, the credential beside them included, still don't come back. + Assert.Empty(row.HandlerProperties); + Assert.Empty(row.MapperProperties); + Assert.Empty(row.ReceiverProperties); + } + + [Fact] + public async Task A_subscription_that_references_nothing_lists_empty_references() + { + var subscriptionId = await CreateSubscriptionAsync(handler: new() { ["Url"] = "https://plain.example.test" }); + + var row = await ListRowAsync(subscriptionId); + + Assert.Empty(row.References.Globals); + Assert.Empty(row.References.PartnerProperties); + } + + [Fact] + public async Task A_set_a_subscription_references_is_not_deleted() + { + var setId = await CreateSetAsync(); + var name = $"Uses the set {Guid.NewGuid():N}"; + // A different case from the set's id, as the resolver still finds it. + await CreateSubscriptionAsync(name, receiver: new() { ["Password"] = $"{{{{globals.{setId.ToUpperInvariant()}.password}}}}" }); + + var refused = await Assert.ThrowsAsync(() => DeleteSetAsync(setId)); + + Assert.Contains(name, refused.Message); + Assert.True(await SetExistsAsync(setId)); + } + + [Fact] + public async Task A_set_a_notification_channel_references_is_not_deleted() + { + var setId = await CreateSetAsync(); + var channel = new NotificationChannel + { + Name = $"Ops mail {Guid.NewGuid():N}", + HandlerId = "NativeSmtpHandler", + HandlerProperties = new Dictionary { ["Password"] = $"{{{{globals.{setId}.password}}}}" }, + }; + await using (var scope = fixture.CreateScope()) + { + var db = scope.ServiceProvider.GetRequiredService(); + db.Add(channel); + await db.SaveChangesAsync(); + } + + var refused = await Assert.ThrowsAsync(() => DeleteSetAsync(setId)); + + Assert.Contains(channel.Name, refused.Message); + Assert.True(await SetExistsAsync(setId)); + } + + [Fact] + public async Task A_set_nothing_references_is_deleted_even_when_another_set_is_in_use() + { + var unused = await CreateSetAsync(); + var used = await CreateSetAsync(); + await CreateSubscriptionAsync(handler: new() { ["Password"] = $"{{{{globals.{used}.password}}}}" }); + + await DeleteSetAsync(unused); + + Assert.False(await SetExistsAsync(unused)); + } + + // ------------------------------------------------------------------- helpers + + private async Task CreateSubscriptionAsync(string name = null, + Dictionary handler = null, Dictionary mapper = null, + Dictionary receiver = null) + { + await using var scope = fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + + var doc = new Document(null, $"Reference Doc {Guid.NewGuid():N}", DocumentFormat.Json); + db.Set().Add(doc); + await db.SaveChangesAsync(); + + var sub = new Subscription(name ?? $"Reference Sub {Guid.NewGuid():N}", doc.Id); + sub.SetDictionaries( + handler: handler ?? new Dictionary(), + mapper: mapper ?? new Dictionary(), + receiver: receiver ?? new Dictionary(), + document: new Dictionary(), + validator: new Dictionary()); + db.Set().Add(sub); + await db.SaveChangesAsync(); + return sub.Id; + } + + private async Task ListRowAsync(int subscriptionId) + { + await using var scope = fixture.CreateScope(); + scope.Superuser(); + var handler = ActivatorUtilities.CreateInstance(scope.ServiceProvider); + var response = (SearchyResponse)await handler.Handle( + new SearchyRequest($"filter=Id:1:{subscriptionId}")); + return Assert.Single(response.Result); + } + + private async Task CreateSetAsync() + { + await using var scope = fixture.CreateScope(); + scope.Superuser(); + var handler = ActivatorUtilities.CreateInstance(scope.ServiceProvider); + var id = $"set{Guid.NewGuid():N}"; + await handler.Handle(new GlobalAdapterValuesSetCreate + { + Id = id, + Name = id, + Values = new Dictionary { ["password"] = "s3cret" }, + SecretProperties = ["password"] + }); + return id; + } + + private async Task DeleteSetAsync(string id) + { + await using var scope = fixture.CreateScope(); + scope.Superuser(); + var handler = ActivatorUtilities.CreateInstance(scope.ServiceProvider); + await handler.Handle(id, new DeleteGlobalAdapterValuesSetModel()); + } + + private async Task SetExistsAsync(string id) + { + await using var scope = fixture.CreateScope(); + var db = scope.ServiceProvider.GetRequiredService(); + return await db.Set().AsNoTracking().AnyAsync(s => s.Id == id); + } +} diff --git a/SW.Bitween.Sdk/Model/Subscription.cs b/SW.Bitween.Sdk/Model/Subscription.cs index 5c968ee9..b3ef99ad 100644 --- a/SW.Bitween.Sdk/Model/Subscription.cs +++ b/SW.Bitween.Sdk/Model/Subscription.cs @@ -338,6 +338,30 @@ public class SubscriptionSearch : SubscriptionGet public int Id { get; set; } public string DocumentName { get; set; } = null!; public bool? IsRunning { get; set; } + + /// + /// The global values and partner properties its adapters reference. The list leaves the + /// adapter properties themselves out, since they hold credentials, so this is how a reader + /// of the list tells which subscriptions use a value set or a partner property. + /// + public SubscriptionReferences References { get; set; } = new(); + } + + /// What a subscription's adapter properties reference, by name only. + public class SubscriptionReferences + { + /// Each value set named in a {{globals.SET.KEY}} token, with the keys used from it. + public List Globals { get; set; } = new(); + + /// The keys named in {{partner.KEY}} tokens. + public List PartnerProperties { get; set; } = new(); + } + + public class GlobalValuesReference + { + /// As written in the token. Matched ignoring case, as the resolver does. + public string SetId { get; set; } = null!; + public List Keys { get; set; } = new(); } public class SubscriptionUpdate : SubscriptionConfiguration diff --git a/SW.Bitween.UnitTests/ReferenceTokensTests.cs b/SW.Bitween.UnitTests/ReferenceTokensTests.cs new file mode 100644 index 00000000..40abf439 --- /dev/null +++ b/SW.Bitween.UnitTests/ReferenceTokensTests.cs @@ -0,0 +1,51 @@ +using System.Linq; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace SW.Bitween.UnitTests; + +/// +/// Reading which global values and partner properties a set of adapter properties names, the way +/// the resolver will fill them in. +/// +[TestClass] +public class ReferenceTokensTests +{ + [TestMethod] + public void Every_token_in_a_value_counts_wherever_it_sits() + { + var refs = StartupValuesFiller.ReferencesIn([ + "https://{{globals.spaces.host}}/{{globals.spaces.bucket}}?key={{partner.apiKey}}", + null, + "{{globals.mail.password}}" + ]); + + CollectionAssert.AreEquivalent(new[] { "spaces", "mail" }, refs.Globals.Select(g => g.SetId).ToList()); + CollectionAssert.AreEquivalent(new[] { "host", "bucket" }, refs.Globals.Single(g => g.SetId == "spaces").Keys); + CollectionAssert.AreEqual(new[] { "apiKey" }, refs.PartnerProperties); + } + + [TestMethod] + public void Spellings_that_differ_only_in_case_are_one_reference() + { + var refs = StartupValuesFiller.ReferencesIn([ + "{{globals.Spaces.secretKey}}", "{{GLOBALS.spaces.SECRETKEY}}", "{{partner.ApiKey}}", "{{Partner.apikey}}" + ]); + + var set = refs.Globals.Single(); + Assert.AreEqual("Spaces", set.SetId); + CollectionAssert.AreEqual(new[] { "secretKey" }, set.Keys); + CollectionAssert.AreEqual(new[] { "ApiKey" }, refs.PartnerProperties); + } + + /// Only what the globals and partner fillers would resolve - other tokens, and a globals token with no key, name nothing. + [TestMethod] + public void Other_tokens_and_a_globals_token_without_a_key_are_not_references() + { + var refs = StartupValuesFiller.ReferencesIn([ + "{{source.order.id}}", "{{connections.crm.authorization}}", "{{globals.spaces}}", "plain value" + ]); + + Assert.AreEqual(0, refs.Globals.Count); + Assert.AreEqual(0, refs.PartnerProperties.Count); + } +} diff --git a/SW.Bitween.Web/ClientApp/src/api/http/globalValues.ts b/SW.Bitween.Web/ClientApp/src/api/http/globalValues.ts index 4bbb62ba..80fe707c 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/globalValues.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/globalValues.ts @@ -1,6 +1,6 @@ import type { ApiClient } from "../client"; import type { GlobalValuesSet, GlobalValuesSetDetail, GlobalValuesSetRow, SubscriptionType, ValueSetUsage } from "../types"; -import { referencesGlobal, scanReferenceTokens } from "./references"; +import { referencesFrom, referencesGlobal, type RawReferences } from "./references"; import { get, getEnrichment, post } from "./request"; interface SearchyResponse { @@ -13,18 +13,11 @@ interface RawValueSet { values: Record | null; secretProperties?: string[] | null; } -interface RawKeyAndValue { - key: string; - value: string; -} interface RawSubscriptionForUsage { id: number; name: string; type: number | string; - mapperProperties: RawKeyAndValue[] | null; - handlerProperties: RawKeyAndValue[] | null; - receiverProperties: RawKeyAndValue[] | null; - validatorProperties: RawKeyAndValue[] | null; + references?: RawReferences | null; } const SUB_TYPE_BY_NUM: Record = { @@ -66,11 +59,7 @@ async function fetchAllSubscriptionsForUsage(): Promise (props ?? []).map((p) => p.value), - ), - ); + const { globals } = referencesFrom(sub.references); return globals.find((g) => referencesGlobal({ globals: [g] }, setId))?.keys ?? []; } diff --git a/SW.Bitween.Web/ClientApp/src/api/http/references.ts b/SW.Bitween.Web/ClientApp/src/api/http/references.ts index 0fb1affc..33948b43 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/references.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/references.ts @@ -3,19 +3,14 @@ * * Adapters can carry `{{globals..}}` and `{{partner.}}` in any * property value; the backend substitutes them at run time - * (`Helpers/StartupValuesFiller.cs`). Nothing indexes them, so "who references - * this value set / partner property?" can only be answered by scanning the - * properties the same way the resolver parses them. + * (`Helpers/StartupValuesFiller.cs`). The subscription list withholds the + * property values, since they hold credentials, so the backend reads the tokens + * and the list carries what they name in each row's `references`. * - * The resolver splits the globals token on the **first** "." only and puts no - * character-class restriction on either half, so these patterns are equally - * permissive rather than assuming a slug charset. It also matches the prefix - * and looks up ids and keys with `OrdinalIgnoreCase` - hence the `i` flag, and - * why callers must compare what comes back case-insensitively. + * Ids and keys come back as written, and the resolver looks them up with + * `OrdinalIgnoreCase` - which is why callers must compare them + * case-insensitively. */ -const GLOBAL_TOKEN_RE = /\{\{globals\.([^.]+)\.([^}]+)\}\}/gi; -const PARTNER_TOKEN_RE = /\{\{partner\.([^}]+)\}\}/gi; - export interface ReferenceTokens { /** Global value references, grouped by the set id as written in the token. */ globals: { setId: string; keys: string[] }[]; @@ -23,26 +18,17 @@ export interface ReferenceTokens { partnerPropKeys: string[]; } -export function scanReferenceTokens(values: (string | null | undefined)[]): ReferenceTokens { - const globals = new Map>(); - const partnerPropKeys = new Set(); - - for (const value of values) { - if (!value) continue; - for (const [, setId, key] of value.matchAll(GLOBAL_TOKEN_RE)) { - const keys = globals.get(setId) ?? new Set(); - keys.add(key); - globals.set(setId, keys); - } - for (const [, key] of value.matchAll(PARTNER_TOKEN_RE)) partnerPropKeys.add(key); - } - - return { - globals: [...globals].map(([setId, keys]) => ({ setId, keys: [...keys] })), - partnerPropKeys: [...partnerPropKeys], - }; +/** A subscription list row's `references`, as `Subscriptions/Search` sends it. */ +export interface RawReferences { + globals?: { setId: string; keys: string[] | null }[] | null; + partnerProperties?: string[] | null; } +export const referencesFrom = (raw: RawReferences | null | undefined): ReferenceTokens => ({ + globals: (raw?.globals ?? []).map((g) => ({ setId: g.setId, keys: g.keys ?? [] })), + partnerPropKeys: raw?.partnerProperties ?? [], +}); + /** Case-insensitive, matching how the resolver compares set ids and keys. */ const eq = (a: string, b: string) => a.localeCompare(b, undefined, { sensitivity: "accent" }) === 0; diff --git a/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts b/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts index 4b05d9d1..790ab7f7 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts @@ -28,7 +28,7 @@ import { documentMethods } from "./documents"; import { deriveStatus, exchangeMethods } from "./exchanges"; import { gatewayMethods } from "./gateways"; import { partnerMethods } from "./partners"; -import { scanReferenceTokens } from "./references"; +import { referencesFrom, type RawReferences } from "./references"; import { get, post, request } from "./request"; import { buildListQuery, SEARCHY_RULE, SEARCHY_SORT } from "./searchQuery"; import { toMatchGroup, toRawMatchExpression, type RawMatchSpec } from "./matchExpression"; @@ -89,6 +89,8 @@ interface RawSubscription { mapperProperties: RawKeyAndValue[] | null; receiverProperties: RawKeyAndValue[] | null; validatorProperties: RawKeyAndValue[] | null; + /** List rows only: what the withheld adapter properties reference. */ + references?: RawReferences | null; documentFilter: RawKeyAndValue[] | null; matchExpression: RawMatchSpec | null; workGroupId: number | null; @@ -396,16 +398,8 @@ export const subscriptionMethods = { .map(([kind, adapterId, version]) => ({ kind, adapterId: adapterId!, version: version ?? null })), responseMessageTypeName: raw.responseMessageTypeName ?? null, responseSubscriptionId: raw.responseSubscriptionId ?? null, - // No backend endpoint indexes reference tokens, but the search rows carry - // every adapter property, so the scan costs nothing extra here. - ...scanReferenceTokens( - [ - raw.receiverProperties, - raw.validatorProperties, - raw.mapperProperties, - raw.handlerProperties, - ].flatMap((props) => (props ?? []).map((p) => p.value)), - ), + // The list withholds adapter property values, so it says what they reference instead. + ...referencesFrom(raw.references), })); }, diff --git a/SW.Bitween.Web/ClientApp/src/pages/global-values/__tests__/UsedBy.test.tsx b/SW.Bitween.Web/ClientApp/src/pages/global-values/__tests__/UsedBy.test.tsx new file mode 100644 index 00000000..235b644a --- /dev/null +++ b/SW.Bitween.Web/ClientApp/src/pages/global-values/__tests__/UsedBy.test.tsx @@ -0,0 +1,53 @@ +import { screen, within } from "@testing-library/react"; +import { http, HttpResponse } from "msw"; +import { describe, expect, it } from "vitest"; +import { apiPath, renderApp } from "../../../__tests__/support/renderApp"; +import { page } from "../../../__tests__/support/listPage"; + +const spaces = { id: "spaces", name: "Spaces", values: { url: "https://nyc3.example.test" }, secretProperties: [] }; +const unused = { id: "unused", name: "Unused", values: { url: "https://old.example.test" }, secretProperties: [] }; + +/** + * A subscription list row as the server sends it: the adapter properties withheld, and what they + * referenced named instead. The set id is in a different case from the set's, as a token may be. + */ +const listRow = { + id: 68, + name: "VFS report", + type: "Aggregation", + documentId: 1, + partnerId: null, + handlerProperties: [], + mapperProperties: [], + receiverProperties: [], + validatorProperties: [], + references: { globals: [{ setId: "Spaces", keys: ["url", "secretKey"] }], partnerProperties: [] }, +}; + +const subscriptions = http.get(apiPath("/subscriptions"), () => HttpResponse.json(page([listRow]))); + +describe("global values used by", () => { + it("names the subscriptions that reference a set from what the list says they reference", async () => { + renderApp("/global-values", { + handlers: [http.get(apiPath("/globaladaptervaluessets"), () => HttpResponse.json(page([spaces, unused]))), subscriptions], + }); + + const used = await screen.findByRole("row", { name: /Spaces/ }); + expect(await within(used).findByRole("link", { name: "VFS report" })).toBeVisible(); + expect(within(screen.getByRole("row", { name: /Unused/ })).queryByRole("link", { name: "VFS report" })).toBeNull(); + }); + + it("lists them on the set's page instead of calling it safe to delete", async () => { + renderApp("/global-values/spaces", { + handlers: [ + http.get(apiPath("/globaladaptervaluessets/spaces"), () => HttpResponse.json(spaces)), + subscriptions, + http.get(apiPath("/audit"), () => HttpResponse.json(page([]))), + ], + }); + + // Twice: in the set's "Used by" panel, and beside the url value it reads. + expect(await screen.findAllByRole("link", { name: /VFS report/ })).toHaveLength(2); + expect(screen.queryByText(/safe to delete/)).toBeNull(); + }); +}); diff --git a/docs/adapters.md b/docs/adapters.md index d6f1c905..eb77a9c8 100644 --- a/docs/adapters.md +++ b/docs/adapters.md @@ -23,7 +23,7 @@ An id starting with `native`, ignoring case, is a native adapter. For any other Each adapter declares its properties. The UI shows which are required, which are secret, and each one's description and default. -- **Tokens.** Values can contain `{{partner.KEY}}` and `{{globals.SET.KEY}}`. Bitween substitutes them ignoring case, global values first, when an exchange is created. Unresolved tokens stay as written. Receivers only get global values. Notification channels get both on each send, with the partner of the exchange being reported, and legacy notifiers get neither. A response or bus gateway subscription's handler can also use `{{source.PATH}}`, a value from the original document; see [Response routing](exchange-pipeline.md#6-response-routing). +- **Tokens.** Values can contain `{{partner.KEY}}` and `{{globals.SET.KEY}}`. Bitween substitutes them ignoring case, global values first, when an exchange is created. Unresolved tokens stay as written, so a value set can't be deleted while a subscription or notification channel references it. Receivers only get global values. Notification channels get both on each send, with the partner of the exchange being reported, and legacy notifiers get neither. A response or bus gateway subscription's handler can also use `{{source.PATH}}`, a value from the original document; see [Response routing](exchange-pipeline.md#6-response-routing). - **Connections.** `{{connections.NAME.accessToken}}` is an OAuth2 access token Bitween keeps fresh, and `{{connections.NAME.authorization}}` is `Bearer` and the token, for a header. Unlike the tokens above, they are filled in as the adapter starts, for every adapter and every kind (handlers, mappers, validators, receivers, notification channels and legacy notifiers), so a token is never stored on an exchange. See [Connections](#connections). - **Secrets.** Secret values are never sent to the browser. The API returns `__private__` instead, and sending `__private__` back keeps the stored value. If Bitween cannot describe an adapter, it masks every property. A property is secret only if its adapter says so: `[Secure]` on a native adapter, and on a published one `isPrivate: true` in `Runner.Expect` (.NET), `secret=True` in `sw.expect` (Python) or `secret: true` in `expect` (JavaScript and TypeScript), which `bitween adapter build` writes into the manifest. Anything else is returned as stored. A secret property's default is never sent either, but the adapter still applies it when the value is left empty. - **Required properties** are checked when a subscription is saved. A blank value counts as missing. diff --git a/docs/api-reference.md b/docs/api-reference.md index 43a88a50..d903673f 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -110,7 +110,7 @@ The retry tree returns `{ rootId, nodes, truncated }`. Each node has its id, `re | Method and path | Permission | |---|---| -| `GET /api/subscriptions` | `subscriptions.view`. Searchy. | +| `GET /api/subscriptions` | `subscriptions.view`. Searchy. Adapter properties come back empty, since they hold credentials; read them from `GET /api/subscriptions/{id}`. Each row's `references` names what they referenced instead: `{ globals: [{ setId, keys }], partnerProperties }`, as written in the tokens. | | `GET /api/subscriptions/{id}` | `subscriptions.view` | | `POST /api/subscriptions` | `subscriptions.create` | | `POST /api/subscriptions/{id}` | `subscriptions.edit`. Replaces the whole configuration. | @@ -280,7 +280,7 @@ See [Data sources](data-sources.md) and [Databases](databases.md). | `GET /api/globaladaptervaluessets`, `GET /api/globaladaptervaluessets/{id}` | `global-values.view` | | `POST /api/globaladaptervaluessets` | `global-values.create`. `{ id, name, values }` | | `POST /api/globaladaptervaluessets/{id}` | `global-values.edit` | -| `POST /api/globaladaptervaluessets/{id}/delete` | `global-values.delete` | +| `POST /api/globaladaptervaluessets/{id}/delete` | `global-values.delete`. Refused with `GLOBAL_VALUES_IN_USE` while a subscription or notification channel references the set in a `{{globals.…}}` token. | | `GET /api/workgroups?name=&offset=&limit=` | `workgroups.view`. Includes live queue metrics. | | `POST /api/workgroups` | `workgroups.create`. `{ name, busMessageName, options: { rabbitMqOptions: { prefetch, priority } } }` | | `POST /api/workgroups/{id}` | `workgroups.edit` | From eaef3cfc0ec2a6ab12c076bb9bdc64f7aa2f4bba Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Wed, 21 Oct 2026 12:01:05 +0300 Subject: [PATCH 2/4] fix: resolve references in any case in the adapter field hints The hint under a field matched {{globals...}} and {{partner...}} by exact case and called working references not found; the server resolves them ignoring case. --- .../src/components/config/AdapterConfig.tsx | 19 +++++++--- .../config/__tests__/AdapterConfig.test.tsx | 37 +++++++++++++++++++ 2 files changed, 50 insertions(+), 6 deletions(-) diff --git a/SW.Bitween.Web/ClientApp/src/components/config/AdapterConfig.tsx b/SW.Bitween.Web/ClientApp/src/components/config/AdapterConfig.tsx index 7d360956..fd845f24 100644 --- a/SW.Bitween.Web/ClientApp/src/components/config/AdapterConfig.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/config/AdapterConfig.tsx @@ -259,8 +259,10 @@ function ReferenceHints({ // Set ids and keys are free-form (spaces included) on the backend - match on the // `{{…}}` delimiters themselves, not a restrictive charset, or refs with a space // in the id/key (e.g. "schedule source url") silently fail to match here. - const globalRefs = [...new Set([...value.matchAll(/\{\{globals\.[^}]+\}\}/g)].map((m) => m[0]))]; - const partnerRefs = [...new Set([...value.matchAll(/\{\{partner\.([^}]+)\}\}/g)].map((m) => m[1]))]; + // Case-insensitive throughout, as the server resolves them: `{{GLOBALS.Set.KEY}}` runs fine, so + // calling it "not found" here would send someone hunting for a problem that isn't there. + const globalRefs = [...new Set([...value.matchAll(/\{\{globals\.[^}]+\}\}/gi)].map((m) => m[0]))]; + const partnerRefs = [...new Set([...value.matchAll(/\{\{partner\.([^}]+)\}\}/gi)].map((m) => m[1]))]; const sourceRefs = source ? [...new Set([...value.matchAll(/\{\{source\.([^}]+)\}\}/gi)].map((m) => m[1]))] : []; @@ -278,8 +280,9 @@ function ReferenceHints({ return (
{globalRefs.map((token) => { - const ref = globals.find((g) => g.token === token); - const setId = token.match(/\{\{globals\.([^.]+)\./)?.[1]; + const ref = globals.find((g) => sameName(g.token, token)); + // The set's own id when it is found, since that is the address its page is at. + const setId = (ref?.token ?? token).match(/\{\{globals\.([^.]+)\./i)?.[1]; return (
{ - const withValue = realPartners.filter((p) => p.propertyKeys.includes(key)); + const keyOf = (p: PartnerRow) => p.propertyKeys.find((k) => sameName(k, key)); + const withValue = realPartners.filter((p) => keyOf(p) !== undefined); const without = realPartners.length - withValue.length; const open = openKeys.has(key); return ( @@ -332,7 +336,7 @@ function ReferenceHints({ {p.name} → - + ))} {without > 0 && ( @@ -384,6 +388,9 @@ function ReferenceHints({ ); } +/** Set ids, keys and whole tokens compare ignoring case, as the server's resolver does. */ +const sameName = (a: string, b: string) => a.toLowerCase() === b.toLowerCase(); + /** * Whether a value is a reference and nothing else. Substitution happens on the * server, so what sits in the field is the pointer - worth showing even on a field diff --git a/SW.Bitween.Web/ClientApp/src/components/config/__tests__/AdapterConfig.test.tsx b/SW.Bitween.Web/ClientApp/src/components/config/__tests__/AdapterConfig.test.tsx index 08e0857e..813a5674 100644 --- a/SW.Bitween.Web/ClientApp/src/components/config/__tests__/AdapterConfig.test.tsx +++ b/SW.Bitween.Web/ClientApp/src/components/config/__tests__/AdapterConfig.test.tsx @@ -2,6 +2,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { http, HttpResponse, type JsonBodyType } from "msw"; +import { MemoryRouter } from "react-router"; import { describe, expect, it } from "vitest"; import { apiPath } from "../../../__tests__/support/renderApp"; import { server } from "../../../__tests__/support/server"; @@ -87,6 +88,42 @@ describe("AdapterConfig source values", () => { }); }); +/** + * The hint under a field says what its references resolve to. The server looks set ids, keys and + * partner properties up ignoring case, so a token spelled differently still resolves - and the + * hint has to agree, or it reports a working reference as broken. + */ +describe("AdapterConfig reference hints", () => { + it("resolves a reference spelled in another case, as the server does", async () => { + server.use( + handlerCatalog, + json("/globaladaptervaluessets", { + result: [{ id: "zz-usedby", name: "zz", values: { host: "api.example.test" }, secretProperties: [] }], + totalCount: 1, + }), + json("/partners", { result: [{ id: 7, name: "Acme", propertyKeys: ["apiKey"] }], totalCount: 1 }), + ); + render( + + + {}} + disabled={false} + /> + + , + ); + + expect(await screen.findByText("api.example.test")).toBeInTheDocument(); + expect(screen.getByRole("link", { name: "{{GLOBALS.ZZ-USEDBY.Host}}" })).toHaveAttribute("href", "/global-values/zz-usedby"); + expect(screen.queryByText(/not found/)).not.toBeInTheDocument(); + expect(screen.getByText("defined by 1 of 1 partner")).toBeInTheDocument(); + }); +}); + /** An HTTP-like handler: one required setting and a spread of optional ones, one with long help. */ const busyCatalog = json("/adapters/Catalog", [ { From 5be9c53ef09c5d06cdf3c3e2df127c514d2f75be Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Wed, 21 Oct 2026 12:01:11 +0300 Subject: [PATCH 3/4] feat: turn exchange filters round, pick several values and several properties Each filter chip picks how it matches: is or is not, contains or does not contain. Subscription, partner and information type take several values, and any number of promoted properties can be filtered on at once. The list, the status counts and select-all-matching read the same filters. --- .../Resources/Xchanges/XchangeFilters.cs | 138 +++++++-- .../Tests/ExchangeFilterOperatorTests.cs | 278 ++++++++++++++++++ .../ClientApp/src/api/http/documents.ts | 2 +- .../ClientApp/src/api/http/exchanges.ts | 38 ++- .../ClientApp/src/api/http/partners.ts | 2 +- .../ClientApp/src/api/http/subscriptions.ts | 2 +- SW.Bitween.Web/ClientApp/src/api/types.ts | 40 ++- .../src/pages/exchanges/ExchangesPage.tsx | 16 +- .../src/pages/exchanges/QueryBar.tsx | 210 ++++++++++--- .../__tests__/ExchangesQueryBar.test.tsx | 83 +++++- .../__tests__/exchangeFilters.test.ts | 109 ++++++- .../src/pages/exchanges/exchangeFilters.ts | 267 ++++++++++++++--- docs/admin-ui.md | 2 +- docs/api-reference.md | 9 +- 14 files changed, 1046 insertions(+), 150 deletions(-) create mode 100644 SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs diff --git a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs index 7043cc3f..cc247000 100644 --- a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs +++ b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs @@ -1,6 +1,10 @@ +using System; +using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Threading.Tasks; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Query; using SW.EfCoreExtensions; using SW.Bitween.Domain; using SW.Bitween.Model; @@ -204,7 +208,9 @@ internal static IQueryable ApplySpecialFilters(this IQueryable i.RetryFor != null); + query = Negated(retryFilter) + ? query.Where(i => i.RetryFor == null) + : query.Where(i => i.RetryFor != null); condition.Filters.Remove(retryFilter); } @@ -218,7 +224,9 @@ internal static IQueryable ApplySpecialFilters(this IQueryable dbContext.Set().Any(d => d.Id == i.Id)); + query = Negated(scheduledFilter) + ? query.Where(i => !dbContext.Set().Any(d => d.Id == i.Id)) + : query.Where(i => dbContext.Set().Any(d => d.Id == i.Id)); condition.Filters.Remove(scheduledFilter); } @@ -229,7 +237,9 @@ internal static IQueryable ApplySpecialFilters(this IQueryable i.AggregationXchangeId != null); + query = Negated(aggregatedFilter) + ? query.Where(i => i.AggregationXchangeId == null) + : query.Where(i => i.AggregationXchangeId != null); condition.Filters.Remove(aggregatedFilter); } @@ -247,7 +257,11 @@ internal static IQueryable ApplySpecialFilters(this IQueryable i.Exception.ToLower().Contains(value)); + // An exchange with no error doesn't contain the text either, so "does not contain" + // keeps it - as "is not" on a subscription keeps the exchanges that have none. + query = Negated(errorFilter) + ? query.Where(i => i.Exception == null || !i.Exception.ToLower().Contains(value)) + : query.Where(i => i.Exception.ToLower().Contains(value)); } condition.Filters.Remove(errorFilter); @@ -258,47 +272,129 @@ internal static IQueryable ApplySpecialFilters(this IQueryable f.Field == "PromotedPropertyKey").ToList(); var key = keyFilters.Select(f => f.Value?.ToString()).FirstOrDefault(v => !string.IsNullOrEmpty(v))?.ToLower(); + var propertiesFilters = condition.Filters + .Where(f => f.Field == "PromotedPropertiesRaw").ToList(); + + // The key and the values below are one question - "orderId contains 1001" - so "not" turns + // the whole of it round, never one part: an exchange whose orderId doesn't contain 1001, or + // that has no orderId at all. Asked for one way in one part and the other way in another, + // it has no reading worth guessing at, and bulk retry acts on whatever this selects. + var negations = keyFilters.Concat(propertiesFilters).Select(Negated).Distinct().ToList(); + if (negations.Count > 1) + throw new SWValidationException("NOT_SUPPORTED", + "A promoted property filter is negated as a whole: send its key and its value with the same rule."); + var negate = negations.SingleOrDefault(); + foreach (var keyFilter in keyFilters) condition.Filters.Remove(keyFilter); + foreach (var propertyFilter in propertiesFilters) + condition.Filters.Remove(propertyFilter); + + var match = PropertyMatch(key, propertiesFilters.Select(f => f.Value.ToString()!.ToLower())); + if (match != null) + query = query.Where(negate ? NoneOrNot(match) : match); + + // Any number of properties at once - "orderNo contains 1001" and "city is Paris" - each with + // its key and text in the one filter, as they are stored: PromotedProperty:4:orderNo:1001. + // A key alone asks whether the exchange promoted it. Every one has to hold, and rule 2 turns + // each round on its own. The pair above stays for links and callers made before this; it can + // only ask about one key, since it has no way to say which value goes with which. + var pairFilters = condition.Filters.Where(f => f.Field == "PromotedProperty").ToList(); + foreach (var pairFilter in pairFilters) + { + var text = pairFilter.Value?.ToString(); + if (!string.IsNullOrEmpty(text)) + { + if (pairFilter.Rule is not (SearchyRule.EqualsTo or SearchyRule.Contains or SearchyRule.NotEqualsTo)) + throw new SWValidationException("NOT_SUPPORTED", + "A promoted property is asked for with rule 1 or 4, or rule 2 to turn it round."); + + var colon = text.IndexOf(':'); + var pairKey = (colon < 0 ? text : text[..colon]).ToLower(); + var pairValue = colon < 0 ? "" : text[(colon + 1)..].ToLower(); + if (pairKey.Length == 0) + throw new SWValidationException("NOT_SUPPORTED", + "Name the promoted property: PromotedProperty:4:key:text. Text in any property is PromotedPropertiesRaw."); + + var pair = PropertyMatch(pairKey, pairValue.Length == 0 ? [] : [pairValue]); + query = query.Where(Negated(pairFilter) ? NoneOrNot(pair) : pair); + } + + condition.Filters.Remove(pairFilter); + } + return query; + } + + /// + /// An exchange whose promoted properties include (when there is one) + /// and contain each of - inside that key's value when there is a key, + /// anywhere when there is not. Both already lower-cased. Null when there is nothing to ask. + /// + private static Expression> PropertyMatch(string key, IEnumerable values) + { // Stored as "key:value,key:value": the key comes first or after a comma, which keeps "id" // from matching "orderId:". Asked of lower(properties_raw) as it is, not with a comma put in // front: Postgres keeps statistics for that expression (its trigram index), and without them // it guessed almost nothing matched, so a common key fetched and sorted every match rather // than reading the newest 25 - 6s against 1M exchanges. var keyTag = "," + key + ":"; + Expression> match = null; if (key != null) - query = query.Where(i => i.PromotedPropertiesRaw.ToLower().StartsWith(key + ":") - || i.PromotedPropertiesRaw.ToLower().Contains(keyTag)); + match = i => i.PromotedPropertiesRaw.ToLower().StartsWith(key + ":") + || i.PromotedPropertiesRaw.ToLower().Contains(keyTag); - var propertiesFilters = condition.Filters - .Where(f => f.Field == "PromotedPropertiesRaw").ToList(); - foreach (var propertyFilter in propertiesFilters) + foreach (var value in values) { - var value = propertyFilter.Value.ToString()!.ToLower(); - // Both sides lower-cased at query time. Promoted values keep the case the // payload had (see FilterService), so the column has to be folded here for // the search to stay case-insensitive. No index is lost: a Contains is a // leading-wildcard LIKE, which the b-tree on this column could never serve. - query = query.Where(i => i.PromotedPropertiesRaw.ToLower().Contains(value)); + match = match.And(i => i.PromotedPropertiesRaw.ToLower().Contains(value)); // With a key, only inside that key's value: what follows ",key:" up to the next comma. // A value that itself holds a comma is only searched up to it, since the stored text // cannot tell that comma from the one before the next key. IndexOf rather than Contains // for the same estimate reason as the key above: Postgres guesses a third of rows pass - // an unknown comparison but under 1% pass an unknown LIKE. + // an unknown comparison but under 1% pass an unknown LIKE. "rest" - what follows the + // key, with a comma added - is written out twice, as a query's `let` would inline it. if (key != null) - query = from i in query - let rest = ("," + i.PromotedPropertiesRaw.ToLower()) - .Substring(("," + i.PromotedPropertiesRaw.ToLower()).IndexOf(keyTag) + keyTag.Length) + "," - where rest.Substring(0, rest.IndexOf(",")).IndexOf(value) >= 0 - select i; - - condition.Filters.Remove(propertyFilter); + match = match.And(i => + (("," + i.PromotedPropertiesRaw.ToLower()) + .Substring(("," + i.PromotedPropertiesRaw.ToLower()).IndexOf(keyTag) + keyTag.Length) + ",") + .Substring(0, (("," + i.PromotedPropertiesRaw.ToLower()) + .Substring(("," + i.PromotedPropertiesRaw.ToLower()).IndexOf(keyTag) + keyTag.Length) + ",") + .IndexOf(",")) + .IndexOf(value) >= 0); } - return query; + return match; + } + + /// A special filter asked for the other way round: rule 2, "not". + private static bool Negated(SearchyFilter filter) => filter.Rule == SearchyRule.NotEqualsTo; + + /// Both, as one predicate - which the property filter needs, to be able to turn it round whole. + private static Expression> And(this Expression> left, + Expression> right) => + left == null + ? right + : Expression.Lambda>( + Expression.AndAlso(left.Body, + ReplacingExpressionVisitor.Replace(right.Parameters[0], left.Parameters[0], right.Body)), + left.Parameters); + + /// + /// The property filter turned round, keeping the exchanges that promoted nothing. Spelt out + /// rather than left to the negation: SQL's NOT of a LIKE over a null is null, which drops the row. + /// + private static Expression> NoneOrNot(Expression> match) + { + var row = match.Parameters[0]; + var raw = Expression.Property(row, nameof(XchangeRow.PromotedPropertiesRaw)); + return Expression.Lambda>( + Expression.OrElse(Expression.Equal(raw, Expression.Constant(null, typeof(string))), Expression.Not(match.Body)), + row); } /// diff --git a/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs b/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs new file mode 100644 index 00000000..4c75fb3e --- /dev/null +++ b/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs @@ -0,0 +1,278 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using SW.Bitween.Domain; +using SW.Bitween.IntegrationTests.Fixtures; +using SW.Bitween.Model; +using SW.Bitween.Resources.Xchanges; +using SW.PrimitiveTypes; +using Xunit; + +namespace SW.Bitween.IntegrationTests.Tests; + +/// +/// The exchange filters asked the other way round - "is not", "is none of", "does not contain", +/// "is missing" - and "is any of". Each is checked where it is used: the list, "select all +/// matching" (what bulk retry and export act on) and the per-status counts, which must agree. +/// +/// +/// A turned-round filter keeps the exchanges that have no value at all: "Subscription is not A" +/// keeps the ones with no subscription, "Error does not contain timeout" keeps the ones with no +/// error. Every test scopes itself to an information type of its own, since the collection shares +/// one database. +/// +[Collection("Bitween")] +public class ExchangeFilterOperatorTests(BitweenFixture fixture) +{ + [Fact] + public async Task Subscription_is_not_and_is_none_of_keep_everything_else_including_no_subscription() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Sub Doc"); + var a = await NewSubscription(db, "Operators A", doc.Id); + var b = await NewSubscription(db, "Operators B", doc.Id); + var c = await NewSubscription(db, "Operators C", doc.Id); + + var ofA = await xs.CreateXchange(a, new XchangeFile("{}")); + var ofB = await xs.CreateXchange(b, new XchangeFile("{}")); + var ofC = await xs.CreateXchange(c, new XchangeFile("{}")); + var ofNone = await xs.CreateXchange(doc, null, new XchangeFile("{}")); + await db.SaveChangesAsync(); + + var inDoc = $"DocumentId:1:{doc.Id}"; + Assert.Equal(Ids(ofB, ofC, ofNone), await Search(scope, inDoc, $"SubscriptionId:2:{a.Id}")); + + string[] noneOfAB = [inDoc, $"SubscriptionId:2:{a.Id}", $"SubscriptionId:2:{b.Id}"]; + Assert.Equal(Ids(ofC, ofNone), await Search(scope, noneOfAB)); + Assert.Equal(Ids(ofC, ofNone), await Select(scope, noneOfAB)); + // Neither has a result yet, so both count as still processing. + Assert.Equal(2, (await Summarise(scope, noneOfAB)).Counts.Processing); + + Assert.Equal(Ids(ofA, ofB), await Search(scope, inDoc, $"SubscriptionId:9:numeric|{a.Id}|{b.Id}")); + Assert.Equal(Ids(ofA, ofB), await Select(scope, inDoc, $"SubscriptionId:9:numeric|{a.Id}|{b.Id}")); + } + + [Fact] + public async Task Partner_and_information_type_take_the_same_operators() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Partner Doc"); + var other = await NewDocument(db, "Operators Other Doc"); + var p1 = new Partner($"Operators P1 {Guid.NewGuid():N}"); + var p2 = new Partner($"Operators P2 {Guid.NewGuid():N}"); + db.AddRange(p1, p2); + await db.SaveChangesAsync(); + + var withP1 = await xs.CreateXchange(await NewSubscription(db, "Operators P1 sub", doc.Id, p1.Id), new XchangeFile("{}")); + var withP2 = await xs.CreateXchange(await NewSubscription(db, "Operators P2 sub", doc.Id, p2.Id), new XchangeFile("{}")); + var noPartner = await xs.CreateXchange(await NewSubscription(db, "Operators no partner", doc.Id), new XchangeFile("{}")); + var otherSub = await NewSubscription(db, "Operators other doc sub", other.Id); + var ofOther = await xs.CreateXchange(otherSub, new XchangeFile("{}")); + await db.SaveChangesAsync(); + + var inDoc = $"DocumentId:1:{doc.Id}"; + Assert.Equal(Ids(withP2, noPartner), await Search(scope, inDoc, $"PartnerId:2:{p1.Id}")); + Assert.Equal(Ids(withP1, withP2), await Search(scope, inDoc, $"PartnerId:9:numeric|{p1.Id}|{p2.Id}")); + + // Information type turned round, kept to this test's own rows by their subscriptions. + var mine = $"SubscriptionId:9:numeric|{withP1.SubscriptionId}|{otherSub.Id}"; + Assert.Equal(Ids(ofOther), await Search(scope, mine, $"DocumentId:2:{doc.Id}")); + Assert.Equal(Ids(withP1, ofOther), await Search(scope, mine, $"DocumentId:9:numeric|{doc.Id}|{other.Id}")); + } + + [Fact] + public async Task Error_does_not_contain_keeps_the_exchanges_with_no_error() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Error Doc"); + var sub = await NewSubscription(db, "Operators Error", doc.Id); + var running = await xs.CreateXchange(sub, new XchangeFile("{}")); + var succeeded = await xs.CreateXchange(sub, new XchangeFile("{}")); + var timedOut = await xs.CreateXchange(sub, new XchangeFile("{}")); + var mapping = await xs.CreateXchange(sub, new XchangeFile("{}")); + await db.SaveChangesAsync(); + db.AddRange( + new XchangeResult(succeeded.Id, null, null), + new XchangeResult(timedOut.Id, null, null, exception: "Timeout Talking To ACME after 30s"), + new XchangeResult(mapping.Id, null, null, exception: "Mapping failed: no such field")); + await db.SaveChangesAsync(); + + string[] filters = [$"DocumentId:1:{doc.Id}", "ErrorContains:2:TIMEOUT"]; + Assert.Equal(Ids(running, succeeded, mapping), await Search(scope, filters)); + Assert.Equal(Ids(running, succeeded, mapping), await Select(scope, filters)); + var counts = (await Summarise(scope, filters)).Counts; + Assert.Equal((1, 1, 1), (counts.Processing, counts.Success, counts.Failed)); + } + + [Fact] + public async Task Each_yes_or_no_filter_turns_round() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Flags Doc"); + var sub = await NewSubscription(db, "Operators Flags", doc.Id); + var failed = await xs.CreateXchange(sub, new XchangeFile("{}")); + var aggregated = await xs.CreateXchange(sub, new XchangeFile("{}")); + var scheduled = await xs.CreateXchange(sub, new XchangeFile("{}")); + await db.SaveChangesAsync(); + db.Add(new XchangeResult(failed.Id, null, null, exception: "boom")); + db.Add(new XchangeAggregation(aggregated.Id, failed.Id)); + // A day out, so the retry job cannot pick it up and delete the row mid-test. + db.Add(new DelayedRetry { Id = scheduled.Id, On = DateTime.UtcNow.AddDays(1) }); + await db.SaveChangesAsync(); + await new Retry(db, scope.Superuser(), xs).Handle(failed.Id, new XchangeRetry { Reset = false }); + await db.SaveChangesAsync(); + var retry = await db.Set().SingleAsync(x => x.RetryFor == failed.Id); + + var inDoc = $"DocumentId:1:{doc.Id}"; + Assert.Equal(Ids(failed, aggregated, scheduled), await Search(scope, inDoc, "IsRetry:2:true")); + Assert.Equal(Ids(failed, aggregated, retry.Id), await Search(scope, inDoc, "HasScheduledRetry:2:true")); + Assert.Equal(Ids(failed, scheduled, retry.Id), await Select(scope, inDoc, "Aggregated:2:true")); + } + + [Fact] + public async Task A_property_filter_turns_round_whole_and_keeps_exchanges_that_promoted_nothing() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Property Doc"); + + async Task Exchange(params (string Key, string Value)[] properties) + { + var xchange = await xs.CreateXchange(doc, null, new XchangeFile("{}")); + // Creating it reads its properties too, once the information type is in the cache - + // which depends on what ran before. Replaced, so "promoted nothing" means no row at all. + foreach (var read in db.ChangeTracker.Entries().Where(e => e.Entity.Id == xchange.Id).ToList()) + read.State = EntityState.Detached; + if (properties.Length > 0) + { + var filtered = new FilterResult(); + foreach (var (key, value) in properties) + filtered.Properties[key] = value; + db.Add(new XchangePromotedProperties(xchange.Id, filtered)); + } + await db.SaveChangesAsync(); + return xchange.Id; + } + + var a = await Exchange(("orderNo", "SO-1001-X"), ("customer", "Acme")); + // "1001" sits in another key's value here. + var b = await Exchange(("orderNo", "PO-7"), ("customer", "Zed-1001")); + var none = await Exchange(); + + var inDoc = $"DocumentId:1:{doc.Id}"; + // The key's own value only, turned round: b's 1001 is under customer, so b is kept. + Assert.Equal(Ids(b, none), await Search(scope, inDoc, "PromotedPropertyKey:2:orderNo", "PromotedPropertiesRaw:2:1001")); + Assert.Equal(Ids(b, none), await Select(scope, inDoc, "PromotedPropertyKey:2:orderNo", "PromotedPropertiesRaw:2:1001")); + // The same question the right way round still finds a alone. + Assert.Equal(Ids(a), await Search(scope, inDoc, "PromotedPropertyKey:1:orderNo", "PromotedPropertiesRaw:4:1001")); + // A key on its own: "is missing". + Assert.Equal(Ids(none), await Search(scope, inDoc, "PromotedPropertyKey:2:customer")); + // No key: in no property at all. + Assert.Equal(Ids(none), await Search(scope, inDoc, "PromotedPropertiesRaw:2:1001")); + Assert.Equal(Ids(b, none), await Search(scope, inDoc, "PromotedPropertiesRaw:2:acme")); + + // One part turned round and the other not has no reading worth guessing at. + await Assert.ThrowsAsync(() => + Search(scope, inDoc, "PromotedPropertyKey:1:orderNo", "PromotedPropertiesRaw:2:1001")); + } + + [Fact] + public async Task Several_properties_at_once_each_look_inside_their_own_key() + { + await using var scope = fixture.CreateScope(); + var (db, xs) = Services(scope); + var doc = await NewDocument(db, "Operators Pairs Doc"); + + async Task Exchange(params (string Key, string Value)[] properties) + { + var xchange = await xs.CreateXchange(doc, null, new XchangeFile("{}")); + foreach (var read in db.ChangeTracker.Entries().Where(e => e.Entity.Id == xchange.Id).ToList()) + read.State = EntityState.Detached; + if (properties.Length > 0) + { + var filtered = new FilterResult(); + foreach (var (key, value) in properties) + filtered.Properties[key] = value; + db.Add(new XchangePromotedProperties(xchange.Id, filtered)); + } + await db.SaveChangesAsync(); + return xchange.Id; + } + + var a = await Exchange(("orderNo", "SO-1001-X"), ("customer", "Acme")); + var b = await Exchange(("orderNo", "PO-7"), ("customer", "Zed-1001")); + var c = await Exchange(("orderNo", "SO-1002"), ("customer", "ACME Europe")); + var none = await Exchange(); + + var inDoc = $"DocumentId:1:{doc.Id}"; + Assert.Equal(Ids(a, c), await Search(scope, inDoc, "PromotedProperty:4:orderNo:SO", "PromotedProperty:4:customer:acme")); + Assert.Equal(Ids(a), await Search(scope, inDoc, "PromotedProperty:4:orderNo:1001", "PromotedProperty:4:customer:acme")); + Assert.Equal(Ids(a), await Select(scope, inDoc, "PromotedProperty:4:orderNo:1001", "PromotedProperty:4:customer:acme")); + // Inside its own key only: a's 1001 is under orderNo, not customer. + Assert.Equal(Ids(b), await Search(scope, inDoc, "PromotedProperty:4:customer:1001")); + // Each turns round on its own. + Assert.Equal(Ids(c), await Search(scope, inDoc, "PromotedProperty:2:orderNo:1001", "PromotedProperty:4:customer:acme")); + Assert.Equal(Ids(a, c), await Search(scope, inDoc, "PromotedProperty:1:orderNo", "PromotedProperty:2:customer:zed")); + Assert.Equal(Ids(none), await Search(scope, inDoc, "PromotedProperty:2:orderNo")); + // A value with a colon in it is still one value: only the first colon ends the key. + Assert.Empty(await Search(scope, inDoc, "PromotedProperty:4:orderNo:SO:1")); + + await Assert.ThrowsAsync(() => Search(scope, inDoc, "PromotedProperty:4::acme")); + } + + // ------------------------------------------------------------------- helpers + + private static (BitweenDbContext Db, XchangeService Xs) Services(AsyncServiceScope scope) => + (scope.ServiceProvider.GetRequiredService(), scope.ServiceProvider.GetRequiredService()); + + private static async Task NewDocument(BitweenDbContext db, string name) + { + var doc = new Document(null, $"{name} {Guid.NewGuid():N}"[..40], DocumentFormat.Json); + db.Add(doc); + await db.SaveChangesAsync(); + return doc; + } + + private static async Task NewSubscription(BitweenDbContext db, string name, int documentId, int? partnerId = null) + { + var sub = partnerId is { } partner + ? new Subscription(name, documentId, SubscriptionType.Internal, partner) { Inactive = false } + : new Subscription(name, documentId) { Inactive = false }; + db.Add(sub); + await db.SaveChangesAsync(); + return sub; + } + + private static string[] Ids(params object[] exchanges) => + exchanges.Select(x => x is Xchange xchange ? xchange.Id : (string)x).Order().ToArray(); + + private static async Task Search(AsyncServiceScope scope, params string[] filters) + { + var (db, xs) = Services(scope); + var response = (SearchyResponse)await new Resources.Xchanges.Search(db, xs, scope.Superuser()) + .Handle(new SearchyRequest(filters) { PageSize = 50 }); + Assert.Equal(response.Result.Count(), response.TotalCount); + return response.Result.Select(r => r.Id).Order().ToArray(); + } + + /// What "select all matching" acts on for the same filters, as bulk retry and export resolve it. + private static async Task Select(AsyncServiceScope scope, params string[] filters) + { + var db = scope.ServiceProvider.GetRequiredService(); + var filter = string.Join("&", filters.Select(f => "filter=" + Uri.EscapeDataString(f))); + return (await new XchangeSelection(db).Resolve(new XchangeFilesExport { Filter = filter }, 100)).Order().ToArray(); + } + + private static async Task Summarise(AsyncServiceScope scope, params string[] filters) + { + var db = scope.ServiceProvider.GetRequiredService(); + return (XchangeSummary)await new Summary(db, scope.Superuser()).Handle(new SearchyRequest(filters)); + } +} diff --git a/SW.Bitween.Web/ClientApp/src/api/http/documents.ts b/SW.Bitween.Web/ClientApp/src/api/http/documents.ts index 2fdecc65..240c7858 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/documents.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/documents.ts @@ -104,7 +104,7 @@ async function fetchDetail(id: number): Promise { get(`/documents/${id}`), enrichment(fetchSubscriptionsByDocument(id), []), enrichment(gatewayMethods.listBusGateways(), []), - enrichment(exchangeMethods.searchExchanges({ informationTypeId: id, offset: 0, limit: 8 }), { + enrichment(exchangeMethods.searchExchanges({ informationTypeIds: [id], offset: 0, limit: 8 }), { result: [], total: 0, }), diff --git a/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts b/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts index 5b06859e..16b82ffc 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/exchanges.ts @@ -4,6 +4,7 @@ import type { BulkRetrySelection, ExchangeBreakdown, ExchangeQuery, + NegatableExchangeFilter, ExchangeRow, ExchangeStatus, ExportFilesCheck, @@ -186,10 +187,19 @@ const toScheduledRetryRow = (raw: RawDelayedRetryRow): ScheduledRetryRow => ({ */ function buildExchangeFilters(query: ExchangeQuery): URLSearchParams { const params = new URLSearchParams(); + const not = (filter: NegatableExchangeFilter) => query.not?.includes(filter) ?? false; + const oneOf = (field: string, values: number[] | undefined, negated: boolean) => { + if (!values?.length) return; + // "is none of" is one "is not" (rule 2) per value, all of which have to hold. + if (negated) for (const v of values) params.append("filter", `${field}:2:${v}`); + else if (values.length === 1) params.append("filter", `${field}:1:${values[0]}`); + else params.append("filter", `${field}:9:numeric|${values.join("|")}`); + }; + if (query.status) params.append("filter", `StatusFilter:1:${STATUS_FILTER[query.status]}`); - if (query.subscriptionId !== undefined) params.append("filter", `SubscriptionId:1:${query.subscriptionId}`); - if (query.partnerId !== undefined) params.append("filter", `PartnerId:1:${query.partnerId}`); - if (query.informationTypeId !== undefined) params.append("filter", `DocumentId:1:${query.informationTypeId}`); + oneOf("SubscriptionId", query.subscriptionIds, not("subscription")); + oneOf("PartnerId", query.partnerIds, not("partner")); + oneOf("DocumentId", query.informationTypeIds, not("informationType")); if (query.ids?.trim()) { const ids = query.ids.split(/[\s,|]+/).filter(Boolean); params.append("filter", `Id:4:text|${ids.join("|")}`); @@ -200,15 +210,19 @@ function buildExchangeFilters(query: ExchangeQuery): URLSearchParams { params.append("filter", `ReceiveAttemptId:1:${query.receiveAttemptId}`); if (query.correlationId?.trim()) params.append("filter", `CorrelationId:1:${query.correlationId.trim()}`); if (query.latest) params.append("filter", "LatestOnly:1:true"); - // The key goes apart from the value so the server can look anywhere inside that key's value. - // Sent as one "key:value" term it only matched values starting with what was typed. - if (query.propertyKey) params.append("filter", `PromotedPropertyKey:1:${query.propertyKey}`); - const propertyValue = query.property?.trim(); - if (propertyValue) params.append("filter", `PromotedPropertiesRaw:4:${propertyValue}`); - if (query.errorContains?.trim()) params.append("filter", `ErrorContains:4:${query.errorContains.trim()}`); - if (query.isRetry) params.append("filter", "IsRetry:1:true"); - if (query.scheduledRetry) params.append("filter", "HasScheduledRetry:1:true"); - if (query.aggregated) params.append("filter", "Aggregated:1:true"); + // Each named property carries its own key and text, so the server looks for the text inside that + // key's value, and every one has to hold. Text to find in any property has no key to pair with. + // Rule 2 turns each of the server's own filters round. + for (const p of query.properties ?? []) { + const text = p.text?.trim(); + if (p.key) params.append("filter", `PromotedProperty:${p.negated ? 2 : text ? 4 : 1}:${p.key}${text ? `:${text}` : ""}`); + else if (text) params.append("filter", `PromotedPropertiesRaw:${p.negated ? 2 : 4}:${text}`); + } + if (query.errorContains?.trim()) + params.append("filter", `ErrorContains:${not("error") ? 2 : 4}:${query.errorContains.trim()}`); + if (query.isRetry) params.append("filter", `IsRetry:${not("retry") ? 2 : 1}:true`); + if (query.scheduledRetry) params.append("filter", `HasScheduledRetry:${not("scheduled") ? 2 : 1}:true`); + if (query.aggregated) params.append("filter", `Aggregated:${not("aggregated") ? 2 : 1}:true`); if (query.from) params.append("filter", `StartedOn:6:${query.from}`); if (query.to) params.append("filter", `StartedOn:8:${query.to}`); return params; diff --git a/SW.Bitween.Web/ClientApp/src/api/http/partners.ts b/SW.Bitween.Web/ClientApp/src/api/http/partners.ts index 42b068c5..81bede17 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/partners.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/partners.ts @@ -139,7 +139,7 @@ export const partnerMethods = { requireDetail(id), enrichment(gatewayMethods.listApiGateways(), []), enrichment(gatewayMethods.listBusGateways(), []), - enrichment(exchangeMethods.searchExchanges({ partnerId: id, offset: 0, limit: 8 }), { result: [], total: 0 }), + enrichment(exchangeMethods.searchExchanges({ partnerIds: [id], offset: 0, limit: 8 }), { result: [], total: 0 }), ]); return { id, diff --git a/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts b/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts index 790ab7f7..452449f4 100644 --- a/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts +++ b/SW.Bitween.Web/ClientApp/src/api/http/subscriptions.ts @@ -455,7 +455,7 @@ export const subscriptionMethods = { fetchRaw(id), gatewayMethods.listApiGateways(), gatewayMethods.listBusGateways(), - exchangeMethods.searchExchanges({ subscriptionId: id, offset: 0, limit: 8 }), + exchangeMethods.searchExchanges({ subscriptionIds: [id], offset: 0, limit: 8 }), ]); const infoType = await documentMethods.getInformationType(raw.documentId).catch(() => null); return { diff --git a/SW.Bitween.Web/ClientApp/src/api/types.ts b/SW.Bitween.Web/ClientApp/src/api/types.ts index 881839b0..25dc82f1 100644 --- a/SW.Bitween.Web/ClientApp/src/api/types.ts +++ b/SW.Bitween.Web/ClientApp/src/api/types.ts @@ -1561,11 +1561,36 @@ export interface BulkRetryPlan { properties: Record | undefined>; } +/** The exchange filters that can be asked the other way round: "is not", "does not contain", "is missing". */ +export type NegatableExchangeFilter = + | "subscription" + | "partner" + | "informationType" + | "property" + | "error" + | "retry" + | "scheduled" + | "aggregated"; + +/** One promoted-property filter. With a key, the text is looked for in that key's value; without one, in any. */ +export interface ExchangePropertyFilter { + key: string | null; + /** Null with a key asks only whether the exchange promoted it. */ + text: string | null; + negated: boolean; +} + export interface ExchangeQuery { status?: ExchangeStatus; - subscriptionId?: number; - partnerId?: number; - informationTypeId?: number; + /** One is "is", several are "is any of" - or, turned round by `not`, "is not" and "is none of". */ + subscriptionIds?: number[]; + partnerIds?: number[]; + informationTypeIds?: number[]; + /** + * The filters turned round. A turned-round filter keeps the exchanges with no value at all: + * "Subscription is not A" keeps the ones with no subscription. + */ + not?: NegatableExchangeFilter[]; /** Comma/pipe/newline separated; matches id, retryFor OR aggregationXchangeId. */ ids?: string; /** @@ -1580,13 +1605,8 @@ export interface ExchangeQuery { * attempts it took, so this turns a list of failures into a list of things still to deal with. */ latest?: boolean; - /** Substring match against promoted property keys and values. */ - property?: string; - /** - * Narrows `property` to one promoted key. Set on its own it asks "has this key at - * all", which is worth being able to ask. - */ - propertyKey?: string; + /** Promoted properties, any number of them, all of which have to hold. */ + properties?: ExchangePropertyFilter[]; /** Case-insensitive substring of the exception text. */ errorContains?: string; /** Only exchanges that are a retry of another one. */ diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx b/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx index 504dbfc0..fc35bf49 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/ExchangesPage.tsx @@ -36,6 +36,7 @@ import { toLocalInput, viewSearch, withParams, + type ParamChanges, } from "./exchangeFilters"; /** The most exchanges one files export takes: `XchangeExportController.Limit` on the server. */ @@ -88,7 +89,7 @@ export function ExchangesPage() { const queryClient = useQueryClient(); const apply = useCallback( - (changes: Record, opts?: { resetOffset?: boolean }) => + (changes: ParamChanges, opts?: { resetOffset?: boolean }) => setSearchParams((prev) => withParams(prev, changes, opts), { replace: true }), [setSearchParams], ); @@ -151,11 +152,14 @@ export function ExchangesPage() { * Read off the list already fetched for the information-type filter, so offering the * keys costs nothing - and picking from real names beats remembering how one was spelled. * - * Narrowed to the picked information type when there is one: a short list of its own keys - * beats the whole catalogue with a disambiguating line on every row. + * Narrowed to the picked information types when there are some: a short list of their own keys + * beats the whole catalogue with a disambiguating line on every row. Not when they are turned + * round, since the list is then everything but them. */ + const pickedTypes = query.not?.includes("informationType") ? undefined : query.informationTypeIds; + const onePickedType = pickedTypes?.length === 1; const propertyKeyOptions = useMemo(() => { - const scoped = query.informationTypeId ? infoTypes.filter((t) => t.id === query.informationTypeId) : infoTypes; + const scoped = pickedTypes ? infoTypes.filter((t) => pickedTypes.includes(t.id)) : infoTypes; const owners = new Map(); for (const t of scoped) for (const p of t.promotedProperties ?? []) { @@ -172,12 +176,12 @@ export function ExchangesPage() { // A second line rather than text beside the name: these run long enough // ("InventoryTransactionPosted") that sharing one line left the name with no room at // all. Dropped once a type is picked above - naming it again on every row says nothing. - sublabel: query.informationTypeId ? undefined : carriers.map((c) => c.type).join(", "), + sublabel: onePickedType ? undefined : carriers.map((c) => c.type).join(", "), // What the key actually reads out of the payload. The thing you want when a filter // comes back empty and you can't tell whether the key or the value is wrong. title: carriers.map((c) => `${c.type}: ${c.path}`).join("\n"), })); - }, [infoTypes, query.informationTypeId]); + }, [infoTypes, pickedTypes, onePickedType]); const filterOptions: FilterOptions = { subscriptions: subscriptions.map((i) => ({ value: String(i.id), label: i.name })), diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/QueryBar.tsx b/SW.Bitween.Web/ClientApp/src/pages/exchanges/QueryBar.tsx index 720151e5..fdc4e943 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/QueryBar.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/QueryBar.tsx @@ -8,7 +8,10 @@ import { RANGE_PRESETS, filterChips, hasTimeFilter, + idList, parseBound, + propertyFilters, + withPropertyFilters, parseRelativeRange, searchSuggestions, timeLabel, @@ -17,10 +20,11 @@ import { type ChipLookups, type FilterChip, type FilterKind, + type ParamChanges, type SavedView, } from "./exchangeFilters"; -type Changes = Record; +type Changes = ParamChanges; export interface FilterOptions { subscriptions: SearchSelectOption[]; @@ -53,10 +57,10 @@ export function QueryBar({ {chips.map((chip) => ( - + ))} - +
@@ -183,20 +187,22 @@ function ChipEditor({ apply: (c: Changes) => void; options: FilterOptions; }) { - const remove = () => apply(Object.fromEntries(chip.keys.map((k) => [k, null]))); + const remove = () => apply(chip.remove); const editable = !["retry", "scheduled", "aggregated"].includes(chip.kind); - const text = ( - <> - {chip.field} - {chip.op && {chip.op}} - {chip.value && ( - {chip.value} - )} - + const value = chip.value && ( + {chip.value} ); + // Laid out as Linear lays a filter out: what it is, how it matches (a menu of its own when it + // can be turned round), and what was picked, which opens the editor. return ( + {chip.field} + {chip.turn ? ( + apply(chip.turn!.set(negated))} /> + ) : ( + chip.op && {chip.op} + )} {editable ? ( - {text} + {value} )} > {(close) => ( { @@ -225,12 +233,13 @@ function ChipEditor({ )} ) : ( - {text} + {value} )} + )} + > + {(close) => ( +
+ {turn.ops.map((op, i) => { + const negated = i === 1; + const current = turn.negated === negated; + return ( + + ); + })} +
+ )} + + ); +} + +/** + * Several of a kind picked at once - "is any of" - with what is picked listed first, so a long + * list never hides it. Applying with nothing picked removes the filter. + */ +function MultiPicker({ + label, + options, + initial, + onApply, +}: { + label: string; + options: SearchSelectOption[]; + initial: string[]; + onApply: (values: string[]) => void; +}) { + const [chosen, setChosen] = useState(initial); + const [find, setFind] = useState(""); + const needle = find.trim().toLowerCase(); + const known = new Map(options.map((o) => [o.value, o])); + const matches = (o: SearchSelectOption) => + !needle || [o.label, o.code, o.sublabel].some((text) => text?.toLowerCase().includes(needle)); + // One picked earlier but gone since still shows, by its id, so it can be unpicked. + const picked = chosen.map((v) => known.get(v) ?? { value: v, label: `#${v}` }).filter(matches); + const rest = options.filter((o) => !chosen.includes(o.value) && matches(o)); + const shown = rest.slice(0, MULTI_PICK_ROWS); + const toggle = (v: string) => setChosen((c) => (c.includes(v) ? c.filter((x) => x !== v) : [...c, v])); + + return ( +
{ + e.preventDefault(); + onApply(chosen); + }} + > + setFind(e.target.value)} + /> +
+ {[...picked, ...shown].map((o) => ( + + ))} + {picked.length + shown.length === 0 &&

Nothing matches.

} + {rest.length > shown.length && ( +

{rest.length - shown.length} more - search to narrow the list.

+ )} +
+
+ {chosen.length === 0 ? "None picked" : `${chosen.length} picked`} + +
+
+ ); +} + +/** How many unpicked rows the picker lists before asking for a search. */ +const MULTI_PICK_ROWS = 100; + /** Where a picker inside a filter panel goes, so each editor reads the same. */ function EditorShell({ title, children }: { title: string; children: ReactNode }) { return ( @@ -309,37 +443,38 @@ function TextFilterForm({ function FilterEditor({ kind, + index, searchParams, options, onApply, }: { kind: FilterKind; + /** Which promoted property a property chip edits; none adds one. */ + index?: number; searchParams: URLSearchParams; options: FilterOptions; onApply: (changes: Changes) => void; }) { - const pick = (label: string, key: string, opts: SearchSelectOption[], clearLabel: string) => ( + const pick = (label: string, key: string, opts: SearchSelectOption[]) => ( - onApply({ [key]: v || null })} + onApply({ [key]: values.join(",") || null })} /> ); switch (kind) { case "subscription": - return pick("Subscription", "subscriptionId", options.subscriptions, "Any subscription"); + return pick("Subscriptions", "subscriptionId", options.subscriptions); case "partner": - return pick("Partner", "partnerId", options.partners, "Any partner"); + return pick("Partners", "partnerId", options.partners); case "informationType": - return pick("Information type", "informationTypeId", options.informationTypes, "Any information type"); + return pick("Information types", "informationTypeId", options.informationTypes); case "property": - return ; + return ; case "ids": return ( @@ -395,16 +530,20 @@ function FilterEditor({ } function PropertyEditor({ + index, searchParams, options, onApply, }: { + index?: number; searchParams: URLSearchParams; options: FilterOptions; onApply: (changes: Changes) => void; }) { - const [key, setKey] = useState(searchParams.get("propertyKey") ?? ""); - const [value, setValue] = useState(searchParams.get("property") ?? ""); + const list = propertyFilters(searchParams); + const editing = index === undefined ? undefined : list[index]; + const [key, setKey] = useState(editing?.key ?? ""); + const [value, setValue] = useState(editing?.text ?? ""); const valueId = useId(); const source = options.propertyKeys.find((o) => o.value === key); @@ -414,7 +553,9 @@ function PropertyEditor({ className="flex flex-col gap-3" onSubmit={(e) => { e.preventDefault(); - onApply({ propertyKey: key || null, property: value.trim() || null }); + // Kept the way round it was; a new one is added beside the others. + const edited = { key: key || null, text: value.trim() || null, negated: editing?.negated ?? false }; + onApply(withPropertyFilters(searchParams, editing ? list.map((p, i) => (i === index ? edited : p)) : [...list, edited])); }} >
@@ -605,7 +746,8 @@ function AddFilterMenu({ title={o.title} className={itemClass} onClick={() => { - apply({ propertyKey: o.value, property: null }); + const list = propertyFilters(searchParams); + apply(withPropertyFilters(searchParams, [...list, { key: o.value, text: null, negated: false }])); close(); }} > @@ -632,11 +774,11 @@ function AddFilterMenu({ // --- search --- -function SearchField({ apply }: { apply: (c: Changes) => void }) { +function SearchField({ searchParams, apply }: { searchParams: URLSearchParams; apply: (c: Changes) => void }) { const [text, setText] = useState(""); const [active, setActive] = useState(0); const [focused, setFocused] = useState(false); - const suggestions = useMemo(() => searchSuggestions(text), [text]); + const suggestions = useMemo(() => searchSuggestions(text, searchParams), [text, searchParams]); const inputRef = useRef(null); const listId = useId(); const open = focused && suggestions.length > 0; @@ -769,7 +911,7 @@ function ViewsMenu({ searchParams, apply }: { searchParams: URLSearchParams; app const go = (view: SavedView) => { const target = new URLSearchParams(view.search); const changes: Changes = {}; - for (const k of new Set([...searchParams.keys(), ...target.keys()])) if (k !== "open") changes[k] = target.get(k); + for (const k of new Set([...searchParams.keys(), ...target.keys()])) if (k !== "open") changes[k] = target.getAll(k); apply(changes); }; diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/ExchangesQueryBar.test.tsx b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/ExchangesQueryBar.test.tsx index 3a662e00..e07995c3 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/ExchangesQueryBar.test.tsx +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/ExchangesQueryBar.test.tsx @@ -323,13 +323,88 @@ describe("refresh", () => { }); describe("promoted property", () => { - it("sends the picked key apart from the value, so any part of that key's value matches", async () => { + it("sends each property with its own key, so any part of that key's value matches", async () => { const api = backend(); + // How a link named its one property before there could be several. renderApp("/exchanges?propertyKey=orderNo&property=1001", { handlers: api.handlers }); await waitFor(() => expect(api.listed.length).toBeGreaterThan(0)); - const sent = filters(api.listed.at(-1)!); - expect(sent).toContain("PromotedPropertyKey:1:orderNo"); - expect(sent).toContain("PromotedPropertiesRaw:4:1001"); + expect(filters(api.listed.at(-1)!)).toEqual(["PromotedProperty:4:orderNo:1001"]); + }); + + it("adds a second property beside the first from the search box", async () => { + const api = backend(); + const { user, router } = renderApp("/exchanges?prop=orderNo:1001", { handlers: api.handlers }); + + await user.type(await screen.findByRole("combobox", { name: "Search exchanges" }), "city:paris{Enter}"); + + expect(new URLSearchParams(router.state.location.search).getAll("prop")).toEqual(["orderNo:1001", "city:paris"]); + await waitFor(() => + expect(filters(api.listed.at(-1))).toEqual(["PromotedProperty:4:orderNo:1001", "PromotedProperty:4:city:paris"]), + ); + expect(screen.getByRole("button", { name: "Remove filter orderNo" })).toBeVisible(); + expect(screen.getByRole("button", { name: "Remove filter city" })).toBeVisible(); + }); +}); + +/** + * Filters that can be turned round - "is not", "does not contain" - from the operator on their + * chip, and Subscription, Partner and Information type picking several at once. + */ +describe("turning a filter round, and picking several", () => { + it("turns a filter round from its chip, and the list and its counts ask for everything else", async () => { + const api = backend(); + const { user, router } = renderApp("/exchanges?subscriptionId=16", { handlers: api.handlers }); + + await user.click(await screen.findByRole("button", { name: "How Subscription matches: is" })); + await user.click(screen.getByRole("menuitemradio", { name: /is not/ })); + + expect(new URLSearchParams(router.state.location.search).get("not")).toBe("subscription"); + await waitFor(() => expect(filters(api.listed.at(-1))).toEqual(["SubscriptionId:2:16"])); + expect(filters(api.summarised.at(-1))).toContain("SubscriptionId:2:16"); + expect(screen.getByRole("button", { name: "How Subscription matches: is not" })).toBeVisible(); + }); + + it("picks several subscriptions, then none of them", async () => { + const api = backend(); + const two = http.get(apiPath("/subscriptions"), () => + HttpResponse.json({ + result: [ + { id: 16, name: "Push orders", type: "Internal", partnerId: null }, + { id: 17, name: "Pull returns", type: "Internal", partnerId: null }, + ], + totalCount: 2, + }), + ); + const { user, router } = renderApp("/exchanges", { handlers: [two, ...api.handlers] }); + + await user.click(await screen.findByRole("button", { name: "Filter" })); + await user.click(within(screen.getByRole("menu", { name: "Add a filter" })).getByRole("menuitem", { name: "Subscription" })); + const list = screen.getByRole("group", { name: "Subscriptions" }); + await user.click(await within(list).findByRole("checkbox", { name: "Push orders" })); + await user.click(within(list).getByRole("checkbox", { name: "Pull returns" })); + await user.click(screen.getByRole("button", { name: "Apply" })); + + expect(new URLSearchParams(router.state.location.search).get("subscriptionId")).toBe("16,17"); + await waitFor(() => expect(filters(api.listed.at(-1))).toEqual(["SubscriptionId:9:numeric|16|17"])); + expect(screen.getByRole("button", { name: "Edit filter: Subscription is any of 2 subscriptions" })).toHaveAttribute( + "title", + "Push orders, Pull returns", + ); + + await user.click(screen.getByRole("button", { name: "How Subscription matches: is any of" })); + await user.click(screen.getByRole("menuitemradio", { name: /is none of/ })); + await waitFor(() => expect(filters(api.listed.at(-1))).toEqual(["SubscriptionId:2:16", "SubscriptionId:2:17"])); + }); + + it("sends the server's own filters turned round", async () => { + const api = backend(); + renderApp("/exchanges?error=timeout&prop=!orderNo:1001&retry=1¬=error,retry", { handlers: api.handlers }); + + await waitFor(() => expect(api.listed.length).toBeGreaterThan(0)); + expect(filters(api.listed.at(-1))).toEqual( + expect.arrayContaining(["ErrorContains:2:timeout", "PromotedProperty:2:orderNo:1001", "IsRetry:2:true"]), + ); + expect(await screen.findByRole("button", { name: "How Error matches: does not contain" })).toBeVisible(); }); }); diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts index ce11e5ae..024ef1ae 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts @@ -3,7 +3,9 @@ import { filterChips, filterSignature, idsIn, + notWith, parseBound, + propertyFilters, parseRelativeRange, rangeLabel, readQuery, @@ -12,6 +14,7 @@ import { timeLabel, viewSearch, withParams, + withPropertyFilters, } from "../exchangeFilters"; /** @@ -36,14 +39,14 @@ describe("reading an older link", () => { expect(q).toMatchObject({ status: "failed", latest: true, - subscriptionId: 16, - partnerId: 3, - informationTypeId: 5, + subscriptionIds: [16], + partnerIds: [3], + informationTypeIds: [5], ids: "a,b", receiveAttemptId: 9, correlationId: "c-1", - propertyKey: "orderNo", - property: "SO-1", + // How a link named its one property before there could be several. + properties: [{ key: "orderNo", text: "SO-1", negated: false }], offset: 25, limit: 25, }); @@ -117,7 +120,7 @@ describe("chips", () => { ["Partner", "is", "#99"], ["orderNo", "contains", "SO-1"], ["Error", "contains", "timeout"], - ["Auto-retry scheduled", "", ""], + ["Auto-retry", "is", "scheduled"], ]); }); @@ -128,7 +131,7 @@ describe("chips", () => { it("says how many ids rather than listing them", () => { const [chip] = filterChips(sp("ids=a,b,c"), lookups); expect(chip.value).toBe("3 ids"); - expect(chip.keys).toEqual(["ids"]); + expect(chip.remove).toEqual({ ids: null }); }); it("asks for a key on its own as 'has this property'", () => { @@ -137,6 +140,92 @@ describe("chips", () => { }); }); +/** + * A filter can be turned round - "is not", "does not contain" - and Subscription, Partner and + * Information type can pick several. Each keeps its own parameter, a list meaning "any of", and + * `not` names the filters turned round, so a link made before either still means what it did. + */ +describe("several values, and filters turned round", () => { + it("reads a list as any of, and not as the filters turned round", () => { + const q = readQuery(sp("subscriptionId=16,17&partnerId=3&error=timeout¬=subscription,error"), NOW); + expect(q).toMatchObject({ subscriptionIds: [16, 17], partnerIds: [3], errorContains: "timeout", not: ["subscription", "error"] }); + }); + + it("counts a not only for a filter that is set", () => { + expect(readQuery(sp("not=subscription,bogus"), NOW).not).toBeUndefined(); + }); + + it("drops a filter's not with the filter, so adding it again starts the right way round", () => { + const removed = withParams(sp("subscriptionId=16&error=x¬=error,subscription"), { error: null }); + expect(removed.get("not")).toBe("subscription"); + expect(withParams(removed, { subscriptionId: null }).has("not")).toBe(false); + }); + + it("turns one filter round and back without touching the others", () => { + const q = sp("subscriptionId=16&retry=1¬=retry"); + expect(notWith(q, "subscription", true)).toBe("retry,subscription"); + expect(notWith(q, "retry", false)).toBeNull(); + }); + + it("reads each kind of filter both ways round", () => { + const read = (q: string) => filterChips(sp(q), lookups).map((c) => [c.field, c.op, c.value]); + expect(read("subscriptionId=16,99¬=subscription")).toEqual([["Subscription", "is none of", "2 subscriptions"]]); + expect(read("subscriptionId=16,99")).toEqual([["Subscription", "is any of", "2 subscriptions"]]); + expect(read("partnerId=3¬=partner")).toEqual([["Partner", "is not", "Northwind"]]); + expect(read("propertyKey=orderNo&property=SO-1¬=property")).toEqual([["orderNo", "does not contain", "SO-1"]]); + expect(read("propertyKey=orderNo¬=property")).toEqual([["orderNo", "is not", "present"]]); + expect(read("property=SO-1¬=property")).toEqual([["Properties", "do not contain", "SO-1"]]); + expect(read("error=timeout¬=error")).toEqual([["Error", "does not contain", "timeout"]]); + expect(read("retry=1&scheduled=1&aggregated=1¬=retry,scheduled,aggregated")).toEqual([ + ["Exchange", "is not", "a retry"], + ["Auto-retry", "is not", "scheduled"], + ["Exchange", "is not", "part of an aggregation"], + ]); + }); + + it("names every pick behind a count on hover", () => { + const [chip] = filterChips(sp("subscriptionId=16,99"), lookups); + expect(chip.title).toBe("Push orders, #99"); + }); + + it("keeps several promoted properties, each with its own key and its own way round", () => { + const q = sp("prop=orderNo:1001&prop=!customer:zed&prop=city&property=acme¬=property"); + expect(readQuery(q, NOW).properties).toEqual([ + { key: "orderNo", text: "1001", negated: false }, + { key: "customer", text: "zed", negated: true }, + { key: "city", text: null, negated: false }, + { key: null, text: "acme", negated: true }, + ]); + expect(filterChips(q, lookups).map((c) => [c.field, c.op, c.value])).toEqual([ + ["orderNo", "contains", "1001"], + ["customer", "does not contain", "zed"], + ["city", "is", "present"], + ["Properties", "do not contain", "acme"], + ]); + }); + + it("removes or turns round one property and leaves the others", () => { + const q = sp("prop=orderNo:1001&prop=customer:zed"); + const [orderNo, customer] = filterChips(q, lookups); + expect(withParams(q, orderNo.remove).getAll("prop")).toEqual(["customer:zed"]); + expect(withParams(q, customer.turn!.set(true)).getAll("prop")).toEqual(["orderNo:1001", "!customer:zed"]); + // The text ends at nothing but the end: a colon in it is part of it. + expect(readQuery(sp("prop=orderNo:SO:1"), NOW).properties).toEqual([{ key: "orderNo", text: "SO:1", negated: false }]); + }); + + it("writes an older link's one property in the new form the first time the properties change", () => { + const q = sp("propertyKey=orderNo&property=1001¬=property,error&error=x"); + const changes = withPropertyFilters(q, [...propertyFilters(q), { key: "city", text: null, negated: false }]); + const next = withParams(q, changes); + expect(next.getAll("prop")).toEqual(["!orderNo:1001", "city"]); + expect([next.has("propertyKey"), next.has("property"), next.get("not")]).toEqual([false, false, "error"]); + }); + + it("keeps not in a saved view, as one of the filters", () => { + expect(viewSearch(sp("subscriptionId=16¬=subscription&open=x"))).toBe("subscriptionId=16¬=subscription"); + }); +}); + describe("the search box", () => { const id = "3f2a9c1e-0000-4000-8000-000000000001"; @@ -157,8 +246,8 @@ describe("the search box", () => { expect(searchSuggestions(" ")).toEqual([]); }); - it("reads key:value as a promoted property by name", () => { - const [first] = searchSuggestions("orderNo: SO-1"); - expect(first.changes).toEqual({ propertyKey: "orderNo", property: "SO-1" }); + it("reads key:value as a promoted property by name, added beside the ones already set", () => { + const [first] = searchSuggestions("orderNo: SO-1", sp("prop=city:paris")); + expect(first.changes).toEqual({ prop: ["city:paris", "orderNo:SO-1"], propertyKey: null, property: null, not: null }); }); }); diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts b/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts index badbefa4..38bc96ae 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts @@ -1,4 +1,7 @@ -import type { ExchangeQuery, ExchangeStatus } from "../../api"; +import type { ExchangePropertyFilter, ExchangeQuery, ExchangeStatus, NegatableExchangeFilter } from "../../api"; + +/** Parameters to set, drop (null) or, for one that repeats, set to several values. */ +export type ParamChanges = Record; /** * Everything the exchanges list can be narrowed by lives in the URL, so a view is a link: it @@ -33,6 +36,7 @@ export const FILTER_KEYS = [ "correlationId", "propertyKey", "property", + "prop", "error", "retry", "scheduled", @@ -41,10 +45,95 @@ export const FILTER_KEYS = [ "from", "to", "latest", + "not", ] as const; export type FilterKey = (typeof FILTER_KEYS)[number]; +// --- turned round --- + +/** + * The parameters each filter that can be turned round is set by. `not` lists the filters turned + * round - `not=subscription,error` - while each keeps its own parameter, so every link made before + * there was a `not` still means what it did. + */ +const NEGATABLE: Record = { + subscription: ["subscriptionId"], + partner: ["partnerId"], + informationType: ["informationTypeId"], + property: ["propertyKey", "property"], + error: ["error"], + retry: ["retry"], + scheduled: ["scheduled"], + aggregated: ["aggregated"], +}; + +const isNegatable = (kind: string): kind is NegatableExchangeFilter => kind in NEGATABLE; + +/** The filters turned round, counting only those that are set. */ +export function negatedFilters(sp: URLSearchParams): NegatableExchangeFilter[] { + return (sp.get("not") ?? "") + .split(",") + .filter(isNegatable) + .filter((kind) => NEGATABLE[kind].some((p) => sp.has(p))); +} + +/** The `not` parameter with one filter turned round, or back. */ +export function notWith(sp: URLSearchParams, kind: NegatableExchangeFilter, negated: boolean): string | null { + const kinds = new Set(negatedFilters(sp)); + if (negated) kinds.add(kind); + else kinds.delete(kind); + return [...kinds].sort().join(",") || null; +} + +// --- promoted properties --- + +/** + * Promoted properties, any number at once, all of which have to hold. Each named one is a `prop` + * parameter of its own - `prop=orderNo:1001`, `prop=city`, `prop=!customer:zed` - its key, then + * the text to find in that key's value, and a leading "!" to turn it round. Text to find in any + * property has no key to pair with, so it keeps `property`, turned round by `not=property`. + * + * `propertyKey` is how a link named its one property before there could be several. It is still + * read, with `property` as its text, and written over the first time the properties change. + */ +const parseProp = (raw: string): ExchangePropertyFilter => { + const negated = raw.startsWith("!"); + const body = negated ? raw.slice(1) : raw; + // The first colon ends the key, as it does where the values are stored; the text may hold more. + const colon = body.indexOf(":"); + return colon < 0 + ? { key: body, text: null, negated } + : { key: body.slice(0, colon), text: body.slice(colon + 1) || null, negated }; +}; + +const formatProp = (p: ExchangePropertyFilter) => `${p.negated ? "!" : ""}${p.key}${p.text ? `:${p.text}` : ""}`; + +/** The promoted-property filters, named ones first and text in any property last. */ +export function propertyFilters(sp: URLSearchParams): ExchangePropertyFilter[] { + const named = sp.getAll("prop").map(parseProp).filter((p) => p.key); + const legacyKey = sp.get("propertyKey"); + const text = sp.get("property"); + const negated = (sp.get("not") ?? "").split(",").includes("property"); + if (legacyKey) return [{ key: legacyKey, text: text || null, negated }, ...named]; + return text ? [...named, { key: null, text, negated }] : named; +} + +/** The parameters that say `list`, replacing whatever said the properties before. */ +export function withPropertyFilters(sp: URLSearchParams, list: ExchangePropertyFilter[]): ParamChanges { + const named = list.filter((p) => p.key); + // Text in any property is one filter: a second replaces the first. + const any = list.filter((p) => !p.key).at(-1); + const not = new Set(negatedFilters(sp).filter((kind) => kind !== "property")); + if (any?.negated) not.add("property"); + return { + prop: named.length > 0 ? named.map(formatProp) : null, + propertyKey: null, + property: any?.text ?? null, + not: [...not].sort().join(",") || null, + }; +} + /** Shown as "10,000+" past the cap. */ export const countLabel = (n: number) => (n > COUNT_CAP ? `${COUNT_CAP.toLocaleString()}+` : n.toLocaleString()); @@ -146,6 +235,17 @@ export const hasTimeFilter = (sp: URLSearchParams) => sp.has("range") || sp.has( const num = (v: string | null) => (v && !Number.isNaN(Number(v)) ? Number(v) : undefined); const flag = (v: string | null) => v === "1" || undefined; +/** A parameter holding one id or several, comma-separated: "16" or "16,17". */ +export const idList = (v: string | null): number[] => + (v ?? "") + .split(",") + .map(Number) + .filter((n) => Number.isInteger(n) && n > 0); +const ids = (v: string | null) => { + const list = idList(v); + return list.length > 0 ? list : undefined; +}; + /** The list's query, as of `now` - which only matters for a relative `range`. */ export function readQuery(sp: URLSearchParams, now: Date = new Date()): ExchangeQuery { const range = sp.get("range"); @@ -153,18 +253,20 @@ export function readQuery(sp: URLSearchParams, now: Date = new Date()): Exchange const from = resolved ? resolved.from : parseBound(sp.get("from"), "from"); const to = resolved ? resolved.to : parseBound(sp.get("to"), "to"); const status = sp.get("status"); + const not = negatedFilters(sp); + const properties = propertyFilters(sp); return { status: STATUSES.includes(status as ExchangeStatus) ? (status as ExchangeStatus) : undefined, - subscriptionId: num(sp.get("subscriptionId")), - partnerId: num(sp.get("partnerId")), - informationTypeId: num(sp.get("informationTypeId")), + subscriptionIds: ids(sp.get("subscriptionId")), + partnerIds: ids(sp.get("partnerId")), + informationTypeIds: ids(sp.get("informationTypeId")), + not: not.length > 0 ? not : undefined, ids: sp.get("ids") ?? undefined, receiveAttemptId: num(sp.get("receiveAttemptId")), correlationId: sp.get("correlationId") ?? undefined, latest: flag(sp.get("latest")), - propertyKey: sp.get("propertyKey") ?? undefined, - property: sp.get("property") ?? undefined, + properties: properties.length > 0 ? properties : undefined, errorContains: sp.get("error") ?? undefined, isRetry: flag(sp.get("retry")), scheduledRetry: flag(sp.get("scheduled")), @@ -182,20 +284,29 @@ export function readQuery(sp: URLSearchParams, now: Date = new Date()): Exchange */ export function withParams( sp: URLSearchParams, - changes: Record, + changes: ParamChanges, { resetOffset = true }: { resetOffset?: boolean } = {}, ): URLSearchParams { const next = new URLSearchParams(sp); for (const [key, value] of Object.entries(changes)) { - if (value === null || value === undefined || value === "") next.delete(key); - else next.set(key, value); + next.delete(key); + // A list is a parameter that repeats, one value each: `prop=orderNo:1&prop=city`. + for (const v of Array.isArray(value) ? value : [value]) if (v) next.append(key, v); } if (resetOffset) next.delete("offset"); + + // A filter that is gone takes its "not" with it, so the same filter added again later starts + // the right way round rather than silently turned. + if (next.has("not")) { + const not = [...new Set(negatedFilters(next))].sort().join(","); + if (not) next.set("not", not); + else next.delete("not"); + } return next; } /** The filters alone, in a fixed order, so two URLs that select the same rows compare equal. */ -export const filterSignature = (sp: URLSearchParams) => FILTER_KEYS.map((k) => sp.get(k) ?? "").join("\u0000"); +export const filterSignature = (sp: URLSearchParams) => FILTER_KEYS.map((k) => sp.getAll(k).join("\u0001")).join("\u0000"); export const activeFilterCount = (sp: URLSearchParams) => FILTER_KEYS.filter((k) => sp.has(k)).length; @@ -216,13 +327,29 @@ export type FilterKind = | "aggregated"; export interface FilterChip { + /** Tells two chips of one kind apart: there can be several promoted properties. */ + id: string; kind: FilterKind; + /** Which promoted property it is, for its editor. */ + index?: number; + /** What it is called whichever way round it reads, for its remove button: "Subscription". */ + name: string; field: string; op: string; value: string; mono?: boolean; - /** The parameters removing it drops. */ - keys: string[]; + /** The names behind a count such as "3 subscriptions", shown on hover. */ + title?: string; + /** What removing it changes in the URL. */ + remove: ParamChanges; + /** For a filter that can be turned round: its two readings, which one it is now, and how to change it. */ + turn?: { + negated: boolean; + ops: [string, string]; + /** What the turned-round reading also keeps, which is easy to forget. */ + hint?: string; + set: (negated: boolean) => ParamChanges; + }; } export interface ChipLookups { @@ -231,53 +358,93 @@ export interface ChipLookups { informationTypeName: (id: number) => string | undefined; } +const drop = (...keys: string[]): ParamChanges => Object.fromEntries(keys.map((k) => [k, null])); + /** * The active filters as chips, except the two with controls of their own: status (the tabs) and * time (its own chip), and "latest", which is a switch beside the tabs. */ export function filterChips(sp: URLSearchParams, lookups: ChipLookups): FilterChip[] { const chips: FilterChip[] = []; - const id = (key: string) => num(sp.get(key)); - - const sub = id("subscriptionId"); - if (sub !== undefined) - chips.push({ kind: "subscription", field: "Subscription", op: "is", value: lookups.subscriptionName(sub) ?? `#${sub}`, keys: ["subscriptionId"] }); - const partner = id("partnerId"); - if (partner !== undefined) - chips.push({ kind: "partner", field: "Partner", op: "is", value: lookups.partnerName(partner) ?? `#${partner}`, keys: ["partnerId"] }); - const type = id("informationTypeId"); - if (type !== undefined) - chips.push({ kind: "informationType", field: "Information type", op: "is", value: lookups.informationTypeName(type) ?? `#${type}`, keys: ["informationTypeId"] }); - - const key = sp.get("propertyKey"); - const property = sp.get("property"); - if (key || property) + const negated = negatedFilters(sp); + // The two readings of a filter turned round by `not`, and which it is now. + const turn = (kind: NegatableExchangeFilter, ops: [string, string], hint?: string) => { + const now = negated.includes(kind); + return { + op: ops[now ? 1 : 0], + turn: { negated: now, ops, hint, set: (to: boolean): ParamChanges => ({ not: notWith(sp, kind, to) }) }, + }; + }; + + // One picked reads by its name; several, by how many, with the names on hover. + const picked = ( + kind: "subscription" | "partner" | "informationType", + key: string, + name: string, + plural: string, + nameOf: (id: number) => string | undefined, + hint?: string, + ) => { + const list = idList(sp.get(key)); + if (list.length === 0) return; + const names = list.map((id) => nameOf(id) ?? `#${id}`); + chips.push({ + id: kind, + kind, + name, + field: name, + value: list.length === 1 ? names[0] : `${list.length} ${plural}`, + title: list.length === 1 ? undefined : names.join(", "), + remove: drop(key), + ...turn(kind, list.length === 1 ? ["is", "is not"] : ["is any of", "is none of"], hint), + }); + }; + picked("subscription", "subscriptionId", "Subscription", "subscriptions", lookups.subscriptionName, "Also keeps exchanges with no subscription."); + picked("partner", "partnerId", "Partner", "partners", lookups.partnerName, "Also keeps exchanges with no partner."); + picked("informationType", "informationTypeId", "Information type", "information types", lookups.informationTypeName); + + // One chip per promoted property, each removed and turned round on its own. + const properties = propertyFilters(sp); + properties.forEach((p, index) => { + const replaced = (by: ExchangePropertyFilter | null) => + withPropertyFilters(sp, properties.flatMap((q, i) => (i !== index ? [q] : by ? [by] : []))); + const reads = (ops: [string, string], hint?: string) => ({ + op: ops[p.negated ? 1 : 0], + turn: { negated: p.negated, ops, hint, set: (to: boolean) => replaced({ ...p, negated: to }) }, + }); + const base = { id: `property-${index}`, kind: "property" as const, index, remove: replaced(null) }; chips.push( - key && property - ? { kind: "property", field: key, op: "contains", value: property, mono: true, keys: ["propertyKey", "property"] } - : key - ? { kind: "property", field: key, op: "is", value: "present", keys: ["propertyKey", "property"] } - : { kind: "property", field: "Any property", op: "contains", value: property!, mono: true, keys: ["propertyKey", "property"] }, + p.key && p.text + ? { ...base, name: p.key, field: p.key, value: p.text, mono: true, ...reads(["contains", "does not contain"], `Also keeps exchanges without ${p.key}.`) } + : p.key + ? { ...base, name: p.key, field: p.key, value: "present", ...reads(["is", "is not"]) } + : { ...base, name: "Properties", field: "Properties", value: p.text!, mono: true, ...reads(["contain", "do not contain"], "Also keeps exchanges that promoted nothing.") }, ); + }); const ids = sp.get("ids"); if (ids) { const list = ids.split(/[\s,|]+/).filter(Boolean); chips.push( list.length === 1 - ? { kind: "ids", field: "Exchange", op: "is", value: list[0], mono: true, keys: ["ids"] } - : { kind: "ids", field: "Exchange", op: "is any of", value: `${list.length} ids`, keys: ["ids"] }, + ? { id: "ids", kind: "ids", name: "Exchange ids", field: "Exchange", op: "is", value: list[0], mono: true, remove: drop("ids") } + : { id: "ids", kind: "ids", name: "Exchange ids", field: "Exchange", op: "is any of", value: `${list.length} ids`, remove: drop("ids") }, ); } const correlation = sp.get("correlationId"); - if (correlation) chips.push({ kind: "correlationId", field: "Correlation id", op: "is", value: correlation, mono: true, keys: ["correlationId"] }); + if (correlation) + chips.push({ id: "correlationId", kind: "correlationId", name: "Correlation id", field: "Correlation id", op: "is", value: correlation, mono: true, remove: drop("correlationId") }); const run = sp.get("receiveAttemptId"); - if (run) chips.push({ kind: "receiveAttempt", field: "Created by run", op: "", value: `#${run}`, keys: ["receiveAttemptId"] }); + if (run) chips.push({ id: "receiveAttempt", kind: "receiveAttempt", name: "Created by run", field: "Created by run", op: "", value: `#${run}`, remove: drop("receiveAttemptId") }); const error = sp.get("error"); - if (error) chips.push({ kind: "error", field: "Error", op: "contains", value: error, mono: true, keys: ["error"] }); - if (sp.get("retry") === "1") chips.push({ kind: "retry", field: "Is a retry", op: "", value: "", keys: ["retry"] }); - if (sp.get("scheduled") === "1") chips.push({ kind: "scheduled", field: "Auto-retry scheduled", op: "", value: "", keys: ["scheduled"] }); - if (sp.get("aggregated") === "1") chips.push({ kind: "aggregated", field: "Part of an aggregation", op: "", value: "", keys: ["aggregated"] }); + if (error) + chips.push({ id: "error", kind: "error", name: "Error", field: "Error", value: error, mono: true, remove: drop("error"), ...turn("error", ["contains", "does not contain"], "Also keeps exchanges with no error.") }); + if (sp.get("retry") === "1") + chips.push({ id: "retry", kind: "retry", name: "Is a retry", field: "Exchange", value: "a retry", remove: drop("retry"), ...turn("retry", ["is", "is not"]) }); + if (sp.get("scheduled") === "1") + chips.push({ id: "scheduled", kind: "scheduled", name: "Auto-retry scheduled", field: "Auto-retry", value: "scheduled", remove: drop("scheduled"), ...turn("scheduled", ["is", "is not"]) }); + if (sp.get("aggregated") === "1") + chips.push({ id: "aggregated", kind: "aggregated", name: "Part of an aggregation", field: "Exchange", value: "part of an aggregation", remove: drop("aggregated"), ...turn("aggregated", ["is", "is not"]) }); return chips; } @@ -296,7 +463,7 @@ export interface SearchSuggestion { label: string; value: string; hint: string; - changes: Record; + changes: ParamChanges; } /** @@ -304,7 +471,7 @@ export interface SearchSuggestion { * value: someone holding an order number rarely knows whether Bitween calls it a promoted * property, a correlation id or the text of an error. */ -export function searchSuggestions(text: string): SearchSuggestion[] { +export function searchSuggestions(text: string, sp: URLSearchParams = new URLSearchParams()): SearchSuggestion[] { const value = text.trim(); if (!value) return []; @@ -330,10 +497,17 @@ export function searchSuggestions(text: string): SearchSuggestion[] { label: `Property ${keyed[1]} contains`, value: keyed[2].trim(), hint: "Promoted property, by name", - changes: { propertyKey: keyed[1].trim(), property: keyed[2].trim() || null }, + // Added beside any properties already filtered on, not instead of them. + changes: withPropertyFilters(sp, [...propertyFilters(sp), { key: keyed[1].trim(), text: keyed[2].trim() || null, negated: false }]), }); suggestions.push( - { key: "property", label: "Any property contains", value, hint: "Across every promoted property", changes: { property: value, propertyKey: null } }, + { + key: "property", + label: "Any property contains", + value, + hint: "Across every promoted property", + changes: withPropertyFilters(sp, [...propertyFilters(sp).filter((p) => p.key), { key: null, text: value, negated: false }]), + }, { key: "correlationId", label: "Correlation id is", value, hint: "Every exchange in the same flow", changes: { correlationId: value } }, { key: "error", label: "Error message contains", value, hint: "Searches the exception text of failed exchanges", changes: { error: value } }, ); @@ -362,9 +536,6 @@ export const BUILT_IN_VIEWS: SavedView[] = [ /** Just the filters, in the order FILTER_KEYS lists them, for saving and comparing views. */ export function viewSearch(sp: URLSearchParams): string { const out = new URLSearchParams(); - for (const k of FILTER_KEYS) { - const v = sp.get(k); - if (v !== null) out.set(k, v); - } + for (const k of FILTER_KEYS) for (const v of sp.getAll(k)) out.append(k, v); return out.toString(); } diff --git a/docs/admin-ui.md b/docs/admin-ui.md index 6582c7fc..cc0f6389 100644 --- a/docs/admin-ui.md +++ b/docs/admin-ui.md @@ -12,7 +12,7 @@ The sign-in page offers email and password, Microsoft, an [OpenID Connect provid | Page | What it does | |---|---| -| **Exchanges** | Search and filter every exchange with live refresh, optionally showing only the latest attempt of each retry chain. Expand a row to see each stage's file, formatted or raw, plus the exception, promoted properties and the retry chain. Retry one exchange, a selection, or every match of the current filters after previewing the plan. Create an exchange by hand. | +| **Exchanges** | Search and filter every exchange with live refresh, optionally showing only the latest attempt of each retry chain. A filter's operator is picked on its chip: *is* or *is not*, *contains* or *does not contain*; subscription, partner and information type can pick several (*is any of*, *is none of*), and any number of promoted properties can be filtered on at once. Expand a row to see each stage's file, formatted or raw, plus the exception, promoted properties and the retry chain. Retry one exchange, a selection, or every match of the current filters after previewing the plan. Create an exchange by hand. | | **Scheduled retries** | Automatic retries waiting to run, with the policy that scheduled each one. Run one now. | | **Queue health** | Live RabbitMQ lanes, backlogs, dead letters, alerts and orphaned queues. | diff --git a/docs/api-reference.md b/docs/api-reference.md index d903673f..1ebc4b35 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -93,12 +93,19 @@ All need the global values permissions: `global-values.view` to read, `.create`, | `GET /api/delayedretries` | `exchanges.view` or `dashboard.view` | Searchy. Waiting automatic retries. | | `POST /api/delayedretries/{id}/runnow` | `exchanges.operate` | Run a waiting retry now | -Exchange search filters, besides ordinary fields: +Exchange search filters, besides ordinary fields such as `SubscriptionId`, `PartnerId` and `DocumentId`. Those take rule 1 (is), 2 (is not) and 9 (is any of, as `SubscriptionId:9:numeric|4|7`). "Is none of" is one rule-2 filter per value. "Is not" keeps the exchanges with no value at all, for example no subscription. | Filter | Meaning | |---|---| | `StatusFilter:1:{n}` | 0 processing, 1 success, 2 bad response, 3 failed. Other values are refused. | | `LatestOnly:1:true` | Only the newest attempt of each retry chain | +| `ErrorContains:4:{text}` | The error contains the text, ignoring case | +| `PromotedProperty:4:{key}:{text}` | That key's value contains the text, ignoring case. Without `:{text}` (rule 1), the exchange promoted the key. Send one per property; every one has to hold. | +| `PromotedPropertiesRaw:4:{text}` | A promoted value contains the text, ignoring case | +| `PromotedPropertyKey:1:{key}` | The older form for one property. With `PromotedPropertiesRaw`, the text is looked for in that key's value only. | +| `IsRetry:1:true`, `HasScheduledRetry:1:true`, `Aggregated:1:true` | A retry of another exchange; an automatic retry is booked; rolled up into an aggregation | + +Rule 2 turns each of these round except `StatusFilter` and `LatestOnly`: `ErrorContains:2:timeout` is "the error does not contain timeout", and it keeps the exchanges with no error. Each `PromotedProperty` turns round on its own. In the older form, send the key and the text with the same rule, since they turn round together. Bulk retry takes `{ ids, filter, excludeIds, reason, reset }`. `filter` is the same query-string fragment the search takes, such as `filter=StatusFilter:1:3&filter=LatestOnly:1:true`, and it replaces `ids` when present. The plan returned by both endpoints lists how many were selected and will be retried, attempts substituted with the end of their chain, and skipped exchanges with reasons, plus `targets`, the attempts that will run. The retry carries out at most `limit` (500) per request; the preview plans up to `maxSelection` (5,000), so a larger selection is retried by sending `targets` back as `ids`, `limit` at a time. From 751647538c5430e4ad8b68f2f1224341e8026e45 Mon Sep 17 00:00:00 2001 From: Hamza Alqurneh Date: Wed, 21 Oct 2026 12:16:00 +0300 Subject: [PATCH 4/4] fix: ignore empty property text and unknown filter names in links --- SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs | 6 +++++- .../Tests/ExchangeFilterOperatorTests.cs | 3 +++ .../src/pages/exchanges/__tests__/exchangeFilters.test.ts | 2 ++ .../ClientApp/src/pages/exchanges/exchangeFilters.ts | 3 ++- 4 files changed, 12 insertions(+), 2 deletions(-) diff --git a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs index cc247000..5abd3f1e 100644 --- a/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs +++ b/SW.Bitween.Api/Resources/Xchanges/XchangeFilters.cs @@ -290,7 +290,11 @@ internal static IQueryable ApplySpecialFilters(this IQueryable f.Value.ToString()!.ToLower())); + // An empty value is no filter, as with the error text above. + var match = PropertyMatch(key, propertiesFilters + .Select(f => f.Value?.ToString()) + .Where(v => !string.IsNullOrEmpty(v)) + .Select(v => v.ToLower())); if (match != null) query = query.Where(negate ? NoneOrNot(match) : match); diff --git a/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs b/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs index 4c75fb3e..9383b41d 100644 --- a/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs +++ b/SW.Bitween.IntegrationTests/Tests/ExchangeFilterOperatorTests.cs @@ -177,6 +177,9 @@ async Task Exchange(params (string Key, string Value)[] properties) // No key: in no property at all. Assert.Equal(Ids(none), await Search(scope, inDoc, "PromotedPropertiesRaw:2:1001")); Assert.Equal(Ids(b, none), await Search(scope, inDoc, "PromotedPropertiesRaw:2:acme")); + // Empty text is no filter, either way round, rather than "promoted anything". + Assert.Equal(Ids(a, b, none), await Search(scope, inDoc, "PromotedPropertiesRaw:4:")); + Assert.Equal(Ids(a, b, none), await Search(scope, inDoc, "PromotedPropertiesRaw:2:")); // One part turned round and the other not has no reading worth guessing at. await Assert.ThrowsAsync(() => diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts index 024ef1ae..f0570ac3 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/__tests__/exchangeFilters.test.ts @@ -153,6 +153,8 @@ describe("several values, and filters turned round", () => { it("counts a not only for a filter that is set", () => { expect(readQuery(sp("not=subscription,bogus"), NOW).not).toBeUndefined(); + // A name every object has is not a filter either. + expect(readQuery(sp("subscriptionId=16¬=constructor,toString"), NOW).not).toBeUndefined(); }); it("drops a filter's not with the filter, so adding it again starts the right way round", () => { diff --git a/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts b/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts index 38bc96ae..c84aefd9 100644 --- a/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts +++ b/SW.Bitween.Web/ClientApp/src/pages/exchanges/exchangeFilters.ts @@ -68,7 +68,8 @@ const NEGATABLE: Record = { aggregated: ["aggregated"], }; -const isNegatable = (kind: string): kind is NegatableExchangeFilter => kind in NEGATABLE; +// Own keys only: the URL is anyone's, and `in` would also take "constructor". +const isNegatable = (kind: string): kind is NegatableExchangeFilter => Object.hasOwn(NEGATABLE, kind); /** The filters turned round, counting only those that are set. */ export function negatedFilters(sp: URLSearchParams): NegatableExchangeFilter[] {