From 1ad93029290e11e32d2e31458d79a9086f3bedca Mon Sep 17 00:00:00 2001 From: vivet Date: Wed, 11 Nov 2026 11:57:24 +0100 Subject: [PATCH] Fixed Entity events not being declared as DIRECT exchange, ignoring the routing key --- .../skills/nano-add-custom-endpoint/SKILL.md | 10 +++++-- .../nano-add-custom-endpoint.prompt.md | 10 +++++-- .github/workflows/build-and-deploy.yml | 2 +- AGENTS.md | 29 +++++++++---------- Directory.Build.props | 3 +- Nano.Eventing.RabbitMq/RabbitMqEventing.cs | 25 ++++++++++++---- 6 files changed, 48 insertions(+), 31 deletions(-) diff --git a/.claude/skills/nano-add-custom-endpoint/SKILL.md b/.claude/skills/nano-add-custom-endpoint/SKILL.md index 89333b6c..e06d3a53 100644 --- a/.claude/skills/nano-add-custom-endpoint/SKILL.md +++ b/.claude/skills/nano-add-custom-endpoint/SKILL.md @@ -238,9 +238,13 @@ Client call happens inside the same action. `` is always the **route name of the controller that hosts the action** (its class name without `Controller`, e.g. a controller named `FooBarsController` uses `FooBars`), for both `Requests/` and `Responses/`. One folder per controller; never group DTOs by another axis (a shared `Common/` folder, or a -folder named after a sub-entity the controller also serves). A DTO used by several controllers lives in the -folder of the controller that returns it, or is duplicated per controller if each needs its own shape. A -request/response in a folder that doesn't match its controller is a defect: move it and fix the namespace +folder named after a sub-entity the controller also serves). A DTO used by several controllers is never shared +across folders. Put the shared members in an `abstract` base class named `BaseRequest` or +`BaseResponse`, placed in the root of `Requests/` or `Responses/`, and give each controller its own +derived class, uniquely named (for example `FooResponse`), in its own folder. The base class +initializes its own properties in its (protected) constructors; the derived class only calls `base(...)` and +adds the properties that are specific to that controller. A DTO used by one controller needs no base class. +A request/response in a folder that doesn't match its controller is a defect: move it and fix the namespace and `using`s. ```csharp diff --git a/.github/prompts/nano-add-custom-endpoint.prompt.md b/.github/prompts/nano-add-custom-endpoint.prompt.md index 7bf05ff9..8a77f0fe 100644 --- a/.github/prompts/nano-add-custom-endpoint.prompt.md +++ b/.github/prompts/nano-add-custom-endpoint.prompt.md @@ -238,9 +238,13 @@ Client call happens inside the same action. `` is always the **route name of the controller that hosts the action** (its class name without `Controller`, e.g. a controller named `FooBarsController` uses `FooBars`), for both `Requests/` and `Responses/`. One folder per controller; never group DTOs by another axis (a shared `Common/` folder, or a -folder named after a sub-entity the controller also serves). A DTO used by several controllers lives in the -folder of the controller that returns it, or is duplicated per controller if each needs its own shape. A -request/response in a folder that doesn't match its controller is a defect: move it and fix the namespace +folder named after a sub-entity the controller also serves). A DTO used by several controllers is never shared +across folders. Put the shared members in an `abstract` base class named `BaseRequest` or +`BaseResponse`, placed in the root of `Requests/` or `Responses/`, and give each controller its own +derived class, uniquely named (for example `FooResponse`), in its own folder. The base class +initializes its own properties in its (protected) constructors; the derived class only calls `base(...)` and +adds the properties that are specific to that controller. A DTO used by one controller needs no base class. +A request/response in a folder that doesn't match its controller is a defect: move it and fix the namespace and `using`s. ```csharp diff --git a/.github/workflows/build-and-deploy.yml b/.github/workflows/build-and-deploy.yml index 9efedc6a..353c54d8 100644 --- a/.github/workflows/build-and-deploy.yml +++ b/.github/workflows/build-and-deploy.yml @@ -8,7 +8,7 @@ on: - master env: APP_NAME: Nano.Library - VERSION: 10.0.26 + VERSION: 10.0.27 jobs: build-and-deploy: runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index 34adbeba..708d4d7f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1970,16 +1970,10 @@ explicit about, since it changes what an action's body actually does: either as a custom method on an entity controller or a bare `BaseController` action. Only ever called by a Public API (or another internal service) via its Api Client — never exposed directly to untrusted clients. -⚠ **Complicated logic always belongs in the internal service, not in the Public API.** A Public API action stays -simple: fetch then act (a get, then an edit), or check then act (a lookup or an ownership check, then a create, -edit or delete). A check on the request, or on what the action just fetched, that rejects with a -`BadRequestException` before acting is part of that and stays in the Public API (for example rejecting a request that -allows no login method, or a duplicate found by a lookup). Complicated rules, a decision made from which other rows exist, or a combined result or flags built -from several calls are one custom method on the owning service, exposed through its own Api Client method, and the -Public API only maps what it returns. That is exactly the case the [Core Principle](#core-principle--built-in-before-custom) -allows a custom endpoint for. Combining calls to different services is the Public API's job. The service then -enforces and reports a rule from one place, so a response or a button can never say yes where the service would -say no. +⚠ **Where a rule lives depends on who it applies to.** A Public API action stays simple: fetch then act (a get, then an edit), or check then act (a lookup or an ownership check, then a create, edit or delete). +- **A rule that must hold for every caller belongs in the internal service**, because the service is the one place all callers pass through. That is any rule about the data itself (a name must be unique, a tenant must keep one domain, an inactive tenant cannot get a domain), however simple it is, and any rule that needs a decision from which other rows exist or a combined result built from several calls. It is one custom method (or an override of the generic action) on the owning service, exposed through its own Api Client method, and the Public API only maps what it returns. That is exactly the case the [Core Principle](#core-principle--built-in-before-custom) allows a custom endpoint for. The service then enforces and reports the rule from one place, so a response or a button can never say yes where the service would say no, and a second Public API gets the rule for free. +- **A rule that is specific to one Public API stays in that Public API**: a policy of that audience or that surface (the back office may not change a tenant's own roles, a request must allow at least one login method), checked with a `BadRequestException` before acting. Combining calls to different services is also the Public API's job. +- **Test:** if another Public API or service calling the same service would need the same rule, it is not specific to this Public API, so it moves to the service. A check shared by several of one Public API's controllers is an Api Client extension method, never a copy. Best practice: a service action that changes something returns the data the caller needs next (for example the finished overview), so the Public API never makes a second call just to fetch it after the action. @@ -3378,9 +3372,10 @@ type — there's no per-entity message contract. `AddNanoEventing()` was never called, the SaveChanges interceptor never attaches — no error, no event, ever. Easy to miss in local testing. - With RabbitMQ specifically: all entity events (across every entity type, every app) flow through **one shared - fanout exchange**, differentiated only by which **queue** a subscriber binds (named after the entity - `TypeName`) — the exchange itself doesn't filter by routing key. A subscriber whose local model doesn't - recognize an incoming `TypeName` throws inside the handler rather than silently ignoring it. + direct exchange** (a message published or subscribed with a routing key uses a direct exchange). A publisher sends + each event with the entity `TypeName` as the routing key, and an app binds one **queue** for each `[Subscribe]` entity (named after the app + and the entity, shared by its replicas) with that `TypeName` as the key. An app therefore gets each event of the + entities it subscribes to once, and never the events of other entities, including the ones it owns itself. - Deletes carry no `Data` — only `Id`/`TypeName`/`State`. - `Modified` events where nothing in the publish paths actually changed are suppressed — no event, no bus traffic, for unrelated field updates. @@ -3563,9 +3558,11 @@ and shouldn't be handed as many concurrent messages as the app-wide default. ⚠ `BaseEventHandler` implementations **must be non-generic** — a handler class that is itself an open generic type is silently skipped during discovery. -With RabbitMQ, the exchange name is derived from `TEvent`'s type name (a fanout exchange — every bound queue -receives every message published to it, regardless of routing key), and the queue name is derived from the -consuming app + `TEvent` + routing key. Share the message contract as a referenced project or NuGet between +With RabbitMQ, the exchange name is derived from `TEvent`'s type name (a fanout exchange when no routing key is +used, so every bound queue receives every message published to it, and a direct exchange when a routing key is used, +so a queue only receives the messages published with the key it is bound with), and the queue name is derived from the +consuming app + `TEvent` + routing key. A publisher and its handlers must use a routing key together or not at all, since +they declare the same exchange and it can only have one type. Share the message contract as a referenced project or NuGet between publisher and subscriber to keep the shape in sync — there's no schema negotiation. --- diff --git a/Directory.Build.props b/Directory.Build.props index b9b10585..ec56a873 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -24,8 +24,7 @@ Reusable libraries for building .NET microservice applications This package is part of the Nano Library, a set of reusable .NET libraries for building microservice applications. Nano addresses common non-business concerns such as logging, persistence, messaging, validation, and documentation, while remaining fully configurable and extensible, so applications can stay focused on business logic. See https://github.com/Nano-Core/Nano.Library for details. - - Updated NuGets - - Updated AGENTS.md + - Fixed Entity events not being declared as DIRECT exchange, ignoring the routing key git master diff --git a/Nano.Eventing.RabbitMq/RabbitMqEventing.cs b/Nano.Eventing.RabbitMq/RabbitMqEventing.cs index 451b4cb4..4f7d1e74 100644 --- a/Nano.Eventing.RabbitMq/RabbitMqEventing.cs +++ b/Nano.Eventing.RabbitMq/RabbitMqEventing.cs @@ -3,6 +3,7 @@ using System.Threading; using System.Threading.Tasks; using EasyNetQ; +using EasyNetQ.Topology; using Microsoft.Extensions.DependencyInjection; using Nano.Common.Extensions; using Nano.Eventing.Abstractions; @@ -12,8 +13,8 @@ namespace Nano.Eventing.RabbitMq; /// /// RabbitMQ implementation of using EasyNetQ. /// -/// Provides a publish/subscribe pattern using fanout exchanges, durable quorum queues, -/// optional routing keys, and message prefetching. Automatically binds exchanges to queues +/// Provides a publish/subscribe pattern using fanout exchanges, or direct exchanges when a routing key is used, +/// durable quorum queues, optional routing keys, and message prefetching. Automatically binds exchanges to queues /// and invokes callbacks when messages are received. /// /// @@ -41,8 +42,8 @@ public async Task PublishAsync(TMessage body, string? routing = null, var name = typeof(TMessage) .GetFriendlyName(); - var exchange = await this.bus.Advanced - .ExchangeDeclareAsync(name, ExchangeType.Fanout, cancellationToken: cancellationToken) + var exchange = await this + .DeclareExchangeAsync(name, routing, cancellationToken) .ConfigureAwait(false); var message = new Message(body); @@ -72,8 +73,8 @@ public async Task SubscribeAsync(IServiceProvider serviceProvider, str }, cancellationToken) .ConfigureAwait(false); - var exchange = await this.bus.Advanced - .ExchangeDeclareAsync(name, ExchangeType.Fanout, cancellationToken: cancellationToken) + var exchange = await this + .DeclareExchangeAsync(name, routing, cancellationToken) .ConfigureAwait(false); await this.bus.Advanced @@ -111,6 +112,18 @@ await this.bus.Advanced } + private async Task DeclareExchangeAsync(string name, string? routing, CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(name); + + var exchangeType = string.IsNullOrEmpty(routing) + ? ExchangeType.Fanout + : ExchangeType.Direct; + + return await this.bus.Advanced + .ExchangeDeclareAsync(name, exchangeType, cancellationToken: cancellationToken) + .ConfigureAwait(false); + } private static string GetQueueName(string name, string? routing = null) { ArgumentNullException.ThrowIfNull(name);