Tags: BrighterCommand/Brighter
Tags
style(#3828): restore lost indentation on EmptyObjectSchema Copilot review catch: the indentation on `EmptyObjectSchema` was lost when `GetUniqueOperationId` was removed in the parent commit. Re-indent to 8 spaces to match the other private methods in the class. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com>
fix(aws-v4-tests): add matching TopicAttributes to SnsPublication in … …V4 Standard SNS tests Same fix as ed15cfc but for the V4 test project. The Standard SNS tests created topics via SnsMessageProducer with no TopicAttributes, causing a tag mismatch when the subscription later tried to create the same topic with Environment=Test tags. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
fix: resolve producer lookup failure when posting Reply messages (#4070) * fix: resolve producer lookup failure when posting Reply messages When posting a Reply, the message mapper sets the header topic to the dynamic reply address (a GUID queue name). The outbox dispatcher then fails to find the producer because it's registered under the static publication topic (e.g. "Reply"), not the GUID. Store the publication topic in the message header Bag during the wrap pipeline when a topic mismatch is detected, then use it as the lookup key in all three dispatch methods (sync, async, bulk). Co-Authored-By: Claude (claude-opus-4-6) <noreply@anthropic.com> * test: cover bulk-dispatch + null-publication-topic for reply producer lookup - bulk dispatch reply messages via outbox sweep (firstMessage bag lookup) - wrap with null publication.Topic skips bag write (guard pin, sync + async) Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: address PR #4070 review feedback - deterministic poll-until in bulk dispatch test (drop Task.Delay) - document firstMessage invariant in BulkDispatchAsync - clarify ProducerTopicHeaderName xml doc (bag key, not header) - braces around single-line if in WrapPipeline + WrapPipelineAsync - inline note on Log.DecoupledInvocationOfMessage divergence from lookup Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: strip producer-topic bag entry + address second review pass - strip ProducerTopicHeaderName from Header.Bag after lookup in Dispatch, DispatchAsync, and BulkDispatchAsync so transports that serialise the bag (AMQP headers, SNS/SQS attributes) don't leak internal topology - namespace bag key as "paramore.brighter.ProducerTopic" to eliminate collision risk with user-defined bag entries - rename async wrap-reply test to end with _Async per suite convention - extend reply post + bulk dispatch tests to pin the strip behaviour Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: extract strip helper overload to keep BulkDispatchAsync complexity flat Moves the per-message strip loop into an IEnumerable<Message> overload of StripProducerLookupTopic so the added foreach no longer counts against BulkDispatchAsync's cyclomatic complexity (CodeScene delta warning). Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: tighten review feedback — batch invariant assert, doc trims, test cleanup - Debug.Assert in BulkDispatchAsync that every message in a topicBatch resolves to the same producer-lookup topic (makes the firstMessage assumption explicit and cheaply catchable) - drop the poll loop in the bulk dispatch test; ClearOutstandingFromOutboxAsync awaits the dispatch so it completes before return - shorten StripProducerLookupTopic xml doc to a one-line comment - add a brief note in GetProducerLookupTopic explaining the null-topic fallback Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * fix: remove uncommitted sample projects from Brighter.slnx The slnx was accidentally swept into an earlier commit with references to samples/TaskQueue/ASBRequestReply/ projects that were never committed to the branch, breaking CI build. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: group bulk dispatch by (wire, producer) topic pair Debug.Assert is stripped in Release, so it could not enforce the single- producer invariant at runtime. Group by the composite key instead — this guarantees every batch resolves to one producer without any assertion. Addresses Copilot review feedback on PR #4070. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: address claude-bot review — stale comment, span key collision, outbox note - rewrite the stale class-level comment in When_Bulk_Dispatching_Reply_Messages_Async to match the current composite-key grouping - disambiguate producerSpans key with the composite (WireTopic, LookupTopic) so two batches sharing a WireTopic but differing in LookupTopic can both end their spans cleanly - document on StripProducerLookupTopic that the mutation is harmless for DB-backed outboxes but affects InMemoryOutbox retries (primarily dev/test scope) Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * fix: restore producer topic on dispatch failure and use collision-free span key Addresses copilot review feedback on #4070: - StripProducerLookupTopic now returns the prior bag value so each dispatch path restores it in a finally block when dispatch did not succeed. InMemoryOutbox stores the Message by reference, so without this a post-strip send failure would leave the outbox entry missing ProducerTopic and retry would fall back to Header.Topic. - BulkDispatchAsync's producerSpans key is now Guid.NewGuid().ToString() rather than "{WireTopic}|{LookupTopic}" — routing keys may legally contain '|', which could collide and drop spans. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * docs: clarify GetProducerLookupTopic comment for reply vs publication paths Per review feedback: the original comment focused on the fallback being "pre-fix behaviour," which obscured *why* the bag entry exists. Rewrite to call out the Reply path (mapper rewrites Header.Topic to a dynamic reply address) and the normal-publication path (no bag entry, falls back to Header.Topic). Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: move producer-topic stripping from mediator to transports Per review feedback (iancooper): the OutboxProducerMediator was stripping the ProducerTopic bag entry around dispatch and restoring it on failure. That mutated the message reference held by InMemoryOutbox, so on retry the producer hint was gone. Move stripping responsibility to the transport's wire-conversion step: - MessageHeader.LocalHeaderNames: static set of bag keys that are internal to Brighter and must not be serialised onto the wire. Pre-populated with Message.ProducerTopicHeaderName; extensible from downstream code. - MessageHeader.StripLocalHeaders(): instance method removing those keys from the bag (provided as a transport convenience). - AzureServiceBusMessagePublisher: skip LocalHeaderNames when copying Header.Bag into ApplicationProperties. - OutboxProducerMediator: drop StripProducerLookupTopic / RestoreProducerLookupTopic and the dispatched/batchDispatched book- keeping that existed only to drive the restore. The bag entry now survives a successful dispatch — InMemoryOutbox-by-reference keeps the producer hint for retries. Tests updated: reply-message Post tests now assert the bag entry survives dispatch (it's the transport's job to omit it on the wire). New unit tests cover MessageHeader.StripLocalHeaders and the ASB publisher's local-header skip. Note: only the ASB transport has been migrated. Other transports that serialise Header.Bag (RMQ, SNS/SQS, Kafka, …) will now leak the paramore.brighter.ProducerTopic entry on the wire — benign for receivers but a behaviour change. Follow-up work to apply the same pattern to those transports is documented on LocalHeaderNames. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: skip MessageHeader.LocalHeaderNames in remaining transports ASB stripped the ProducerTopic local header on the wire already (prior commit). Apply the same pattern to every other transport that copies Header.Bag onto its wire format so paramore.brighter.ProducerTopic doesn't leak. Inline-filter transports (existing skip-set extended with LocalHeaderNames): - RMQ.Async / RMQ.Sync RmqMessagePublisher - Kafka KafkaDefaultMessageHeaderBuilder - GcpPubSub Parser - RocketMQ RocketMqMessageProducer - MessageScheduler.Azure AzureServiceBusScheduler Whole-bag-as-JSON transports (use new MessageHeader.BagWithoutLocalHeaders): - AWSSQS V3 + V4 SnsMessagePublisher - AWSSQS V3 + V4 SqsMessageSender - Redis RedisMessagePublisher MessageHeader gains BagWithoutLocalHeaders() — returns a new dictionary copy minus LocalHeaderNames — so transports that serialise the bag in one shot (SNS / SQS / Redis emit it as a single JSON property) can hand the filtered view to the serialiser without mutating the original header (preserves InMemoryOutbox-by-reference retries). Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: protect LocalHeaderNames behind IsLocalHeader / RegisterLocalHeader The first cut exposed LocalHeaderNames as a public mutable HashSet so transports could call .Contains directly. That let any caller remove the framework-essential ProducerTopicHeaderName entry, and read/write races were possible if a registration ran while a publisher was iterating. Hide the storage and expose two operations: - IsLocalHeader(name): lock-free read against a copy-on-write snapshot (Volatile.Read of the field). This is on the per-bag-entry hot path. - RegisterLocalHeader(name): CAS loop that swaps in a new HashSet containing the additional key. Idempotent. Expected to be called once at startup from extension code. The field itself becomes private; the snapshot is never mutated in place after publication, so readers can iterate it freely without locking. StripLocalHeaders and BagWithoutLocalHeaders snapshot once via Volatile.Read at the start. ImmutableHashSet was the natural fit but isn't in the netstandard2.0 BCL and Brighter doesn't reference the package — emulating copy-on-write with HashSet keeps the same semantics without adding a dependency. Call sites in the 9 transports + the local-header tests switch to MessageHeader.IsLocalHeader. New test covers RegisterLocalHeader idempotency and that custom keys are honoured by BagWithoutLocalHeaders. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * refactor: drop unused MessageHeader.StripLocalHeaders instance method No transport calls it — they all use IsLocalHeader for inline filtering or BagWithoutLocalHeaders for whole-bag JSON serialisation. Keeping the method around invites callers to mutate the message's bag in place, which would re-introduce the InMemoryOutbox-by-reference regression that moving stripping out of the mediator just fixed. Test class renamed MessageHeaderStripLocalHeadersTests → MessageHeaderLocalHeadersTests and the two strip-specific cases rewritten against BagWithoutLocalHeaders (also asserts the original header is untouched, pinning the InMemoryOutbox-by-reference invariant). Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> * test: address review feedback — drop Fragile trait, document converter contract, pin fallback Three issues from claude[bot]'s latest review: 1. The new ASB unit test carried [Trait("Fragile", "CI")], copied by mistake from the live-ASB integration tests. The CI pipeline uses "Fragile!=CI" as its filter, so the trait skipped the only transport-level pin for local-header filtering. Drop it. 2. GetProducerLookupTopic uses `producerTopic is string topic`. Concern was that persistent outboxes round-trip Header.Bag through JSON, so values would come back as JsonElement and the cast would silently fail — reproducing the bug the PR fixes. Investigation shows Brighter's bag round-trip uses JsonSerialisationOptions.Options, which composes DictionaryStringObjectJsonConverter with ObjectToInferredTypesConverter to preserve string runtime types. Verified RelationDatabaseOutbox (covers MsSql, PostgreSQL, MySql, Sqlite, Spanner), MongoDb, and DynamoDB all use those options on deserialise. Document the contract on GetProducerLookupTopic and add BagStringValueRoundTripTests to pin the converter behaviour. If a future change drops one of the converters, the round-trip test fails loudly instead of GetProducerLookupTopic regressing silently. 3. Add WrapMatchingPublicationTopicTests to pin the fallback path: when a mapper does NOT override Header.Topic, no ProducerTopic bag entry is written and producer lookup falls back to Header.Topic. The existing test trio (reply-mapper-overrides, null-publication-topic, matching-topic) now covers all three branches of WrapPipeline.Wrap. Co-Authored-By: Claude (claude-opus-4-7) <noreply@anthropic.com> --------- Co-authored-by: Jonny Olliff-Lee <jonnyolliff.lee@wearefreemarket.com> Co-authored-by: Claude (claude-opus-4-6) <noreply@anthropic.com>
chore: clean up csproj and Directory.Build.props files (#4060) * chore: remove redundant properties from tests/ and tools/ Directory.Build.props LangVersion, Nullable, and NoWarn are already inherited from the root Directory.Build.props. Removing the duplicates to reduce maintenance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: remove redundant LangVersion=latest from csproj files LangVersion is already set to 'latest' in the root Directory.Build.props and src/Directory.Build.props. Remove the duplicate declarations from 38 src projects and 1 test project. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: remove redundant Nullable=enable from test, sample, and tool csproj files Nullable is already set to 'enable' in the root Directory.Build.props. Remove duplicate declarations from 12 test projects, 17 sample projects, and 1 tool project. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: clean up Locking.MsSql and remove redundant TreatWarningsAsErrors Locking.MsSql had duplicate Nullable, duplicate Description, and redundant TreatWarningsAsErrors. AzureServiceBus also had redundant TreatWarningsAsErrors (inherited from src/Directory.Build.props). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: document why SignAssembly=false in 6 projects These projects reference unsigned third-party packages (Confluent.Kafka, MQTTnet, RocketMQ.Client, Hangfire.Core, TickerQ, ServiceStack.Redis.Core) and cannot be strong-named. Also normalizes TickerQ casing to lowercase. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: centralize RabbitMQ.Client legacy version in Directory.Packages.props Replace hardcoded VersionOverride="6.8.1" with $(RabbitMQClientLegacy) property, consistent with how AWS SDK V4 versions are managed. Also removes hardcoded LangVersion=8 from the RMQRequestReply sample. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: remove obsolete RestoreUseStaticGraphEvaluation This has been the default behavior since the .NET 9 SDK and no longer needs to be set explicitly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: enable GenerateDocumentationFile for src projects Generates XML documentation so NuGet consumers get IntelliSense. Suppresses XML doc warnings (CS0419, CS1570-CS1734) to avoid conflicts with TreatWarningsAsErrors until existing doc comments are fixed. Removes the per-project override from MySql.EntityFrameworkCore as it is now inherited. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: update sample projects from net8.0 to net9.0 Update 7 sample projects (Scheduler samples and mTLS TestHarness) that were still targeting net8.0 to net9.0, consistent with the rest of the samples. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: document Nullable=disable in migration sample projects These FluentMigrator projects intentionally override the root Nullable=enable setting. Add comments explaining why. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * perf: build only net9.0+net10.0 locally, full multi-target on CI Local builds now skip netstandard2.0, net462, and net8.0 — reducing TFMs from 4-5 down to 2 per project (~42% faster builds). CI and tagged builds still compile the full TFM matrix. Developers can opt in to all TFMs locally with: dotnet build /p:BuildAllTargetFrameworks=true Also introduces BrighterMongoTargetFrameworks for MongoDB projects that require net472 on CI (via netstandard2.0 compat) but not locally. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: make polyfill package references conditional on target framework System.Text.RegularExpressions and System.Reflection.TypeExtensions are built into net9.0+ and only needed for netstandard2.0 / .NET Framework. Making them conditional avoids NU1510 pruning warnings on modern TFMs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert "fix: make polyfill package references conditional on target framework" This reverts commit b19ccb6. * Revert "perf: build only net9.0+net10.0 locally, full multi-target on CI" This reverts commit 8323e04. * refactor: rename RabbitMQClientLegacy to RabbitMQClientV6 Clearer name reflecting the RabbitMQ.Client major version rather than implying the package is deprecated. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: remove empty lines in csproj PropertyGroup blocks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
FIX: synchronize offset commits during Kafka partition revoke (#4057) * fix: synchronize offset commits during Kafka partition revoke to prevent race condition CommitOffsetsFor (called during partition revoke) was not acquiring the _flushToken semaphore, allowing it to race with in-flight background batch/sweep commits. Since librdkafka is not thread-safe for concurrent commit operations, this caused Local_BadMsg errors. The fix acquires the semaphore with a 5-second blocking timeout before committing revoked offsets. Also fixes Close() to use a blocking wait instead of a non-blocking try that could skip the final offset commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: allow CooperativeSticky partition assignment strategy now that librdkafka bug is fixed The librdkafka bug (confluentinc/librdkafka#4059) that prevented CooperativeSticky with manual commits was fixed in 2.12.1. The Confluent .NET client now uses 2.14.0.RC1 which includes the fix. Remove the ArgumentOutOfRangeException guard and parameterize rebalance tests to verify both RoundRobin and CooperativeSticky. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: add warning log on close timeout, extract commit sync constant, and harden test cleanup Address PR review feedback: - Log a warning when Close() times out waiting for the semaphore - Extract the 5-second commit sync timeout to a named constant - Add [Trait("Fragile", "CI")] to KafkaMessageConsumerCommitOnRevoke - Use 'using var' for consumerA/consumerB to prevent resource leaks on test failure Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
[Fix] RMQ event handlers not removed (#4044) (#4045) * Fix RMQ event handlers not removed (#4044) * Revert "Fix RMQ event handlers not removed (#4044)" This reverts commit 1208c5b. * Add RMQ channel event handlers once on creation (#4044) * Remove channel events in Dispose (#4044) --------- Co-authored-by: Ian Cooper <ian_hammond_cooper@yahoo.co.uk>
chore(deps): bump aws-actions/configure-aws-credentials from 5 to 6 (#… …4008) Bumps [aws-actions/configure-aws-credentials](https://github.com/aws-actions/configure-aws-credentials) from 5 to 6. - [Release notes](https://github.com/aws-actions/configure-aws-credentials/releases) - [Changelog](https://github.com/aws-actions/configure-aws-credentials/blob/main/CHANGELOG.md) - [Commits](aws-actions/configure-aws-credentials@v5...v6) --- updated-dependencies: - dependency-name: aws-actions/configure-aws-credentials dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ian Cooper <ian_hammond_cooper@yahoo.co.uk>
PreviousNext