Skip to content

Commit 19b4e81

Browse files
authored
chore: allow why-comments, sweep narration comments, and align file layout with namespaces (#353)
1 parent 9fb66f1 commit 19b4e81

13 files changed

Lines changed: 284 additions & 270 deletions

File tree

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
- When modifying a public member, make sure to update the XML Documentation and the corresponding docs in the docs folder and the README file if applicable.
1111
- When modifying a public member, make sure to check the Public.API files for the affected assembly and update them if necessary.
1212
- Do not ignore CS1591 warnings; analyze and add missing XML comments instead.
13-
- Do not add comments inside methods; for complex logic, consider extracting it into a separate method with a descriptive name instead of adding comments.
13+
- Comments inside method bodies must explain *why*, not *what*: a why-comment states a constraint, invariant, or rationale the code cannot express on its own (e.g. concurrency/ordering requirements, the reason for a workaround, a justification for otherwise-surprising deliberate behavior). Narration/what-comments — restating what the next line does, section-header comments, play-by-play commentary — are not allowed. For complex logic, prefer extracting it into a separate method with a descriptive name over adding a comment.
1414

1515
## Testing Guidelines
1616
- Prefer using the Vulthil.xUnit testing framework for tests.
File renamed without changes.
File renamed without changes.
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
namespace Vulthil.Messaging.Queues;
2+
3+
/// <summary>
4+
/// Describes the dead letter queue and exchange configuration.
5+
/// </summary>
6+
public sealed record DeadLetterDefinition
7+
{
8+
/// <summary>
9+
/// Gets or sets the dead letter queue name, or <see langword="null"/> to use the default.
10+
/// </summary>
11+
public string? QueueName { get; set; }
12+
/// <summary>
13+
/// Gets or sets the dead letter exchange name, or <see langword="null"/> to use the default.
14+
/// </summary>
15+
public string? ExchangeName { get; set; }
16+
/// <summary>
17+
/// Gets or sets a value indicating whether dead letter routing is enabled.
18+
/// </summary>
19+
public bool Enabled { get; set; }
20+
}
Lines changed: 0 additions & 261 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
using System.Reflection;
2-
using System.Security.Cryptography;
32
using Vulthil.Messaging.Abstractions.Consumers;
43

54
namespace Vulthil.Messaging.Queues;
@@ -79,263 +78,3 @@ public interface IQueueConfigurator
7978
/// </summary>
8079
IQueueConfigurator UseSingleActiveConsumer();
8180
}
82-
83-
/// <summary>
84-
/// Defines a retry policy including intervals, jitter, and exception filtering.
85-
/// </summary>
86-
public sealed record RetryPolicyDefinition
87-
{
88-
/// <summary>
89-
/// Gets or sets the maximum number of retry attempts.
90-
/// </summary>
91-
public int MaxRetryCount { get; set; }
92-
/// <summary>
93-
/// Gets or sets the jitter factor (0.0–1.0) applied to retry intervals.
94-
/// </summary>
95-
public double JitterFactor { get; set; }
96-
/// <summary>
97-
/// Gets or sets a value indicating whether retries run in-memory — the consumer is re-invoked in-process
98-
/// while the delivery is held — rather than via delayed re-delivery through the retry queue. In-memory
99-
/// retries preserve message order (a later message cannot overtake the one being retried), so they are
100-
/// used automatically for partitioned queues. Defaults to <see langword="false"/> (delayed re-delivery).
101-
/// </summary>
102-
public bool InMemory { get; set; }
103-
/// <summary>
104-
/// Gets the delay intervals between successive retry attempts.
105-
/// </summary>
106-
public ICollection<TimeSpan> Intervals { get; } = [];
107-
/// <summary>
108-
/// Gets the fully-qualified type names of exceptions that should be excluded from retry attempts.
109-
/// Names outside the core library must be assembly-qualified to resolve; a name that cannot be
110-
/// resolved is skipped (the RabbitMQ transport logs a startup warning for each such name).
111-
/// </summary>
112-
public ICollection<string> IgnoreExceptions { get; } = [];
113-
114-
private readonly object _ignoredTypesGate = new();
115-
private readonly HashSet<Type> _ignoredTypes = [];
116-
private HashSet<Type>? _resolvedIgnoredTypes;
117-
118-
internal void AddIgnoredType(Type type) => _ignoredTypes.Add(type);
119-
/// <summary>
120-
/// Gets the resolved CLR exception types that should be excluded from retry attempts. The set is built
121-
/// once, on first access, from the fluently ignored types and the resolvable names in
122-
/// <see cref="IgnoreExceptions"/>; the resolution is thread-safe and later mutations of either
123-
/// collection do not change the result.
124-
/// </summary>
125-
public HashSet<Type> GetIgnoredExceptionTypes()
126-
{
127-
lock (_ignoredTypesGate)
128-
{
129-
return _resolvedIgnoredTypes ??= ResolveIgnoredExceptionTypes();
130-
}
131-
}
132-
133-
private HashSet<Type> ResolveIgnoredExceptionTypes()
134-
{
135-
var resolved = new HashSet<Type>(_ignoredTypes);
136-
foreach (var name in IgnoreExceptions)
137-
{
138-
if (Type.GetType(name, false) is { } type)
139-
{
140-
resolved.Add(type);
141-
}
142-
}
143-
144-
return resolved;
145-
}
146-
147-
/// <summary>
148-
/// Calculates the delay for the specified retry attempt, applying jitter when configured.
149-
/// Attempts beyond the last configured interval reuse that interval (capped back-off).
150-
/// </summary>
151-
/// <param name="attempt">The zero-based attempt index.</param>
152-
/// <returns>The delay before the next retry.</returns>
153-
public TimeSpan GetDelay(int attempt)
154-
{
155-
if (Intervals.Count == 0)
156-
{
157-
return TimeSpan.Zero;
158-
}
159-
160-
var interval = GetIntervalForAttempt(attempt);
161-
if (JitterFactor <= 0.0)
162-
{
163-
return interval;
164-
}
165-
166-
var jitterMultiplier = RandomNumberGeneratorExtensions.GetDouble() * 2 * JitterFactor - JitterFactor;
167-
var finalDelay = interval.TotalMilliseconds * (1 + jitterMultiplier);
168-
return TimeSpan.FromMilliseconds(Math.Max(0, finalDelay));
169-
}
170-
171-
private TimeSpan GetIntervalForAttempt(int attempt)
172-
{
173-
var index = Math.Min(attempt, Intervals.Count - 1);
174-
return Intervals is IList<TimeSpan> list ? list[index] : Intervals.ElementAt(index);
175-
}
176-
}
177-
178-
internal static class RandomNumberGeneratorExtensions
179-
{
180-
/// <summary>
181-
/// Generates a cryptographically random <see langword="double"/> in the range [0, 1). Retry jitter does
182-
/// not need cryptographic strength, but the repository enforces CA5394 (no <see cref="Random"/>) as an
183-
/// error, so the secure generator is used here as well.
184-
/// </summary>
185-
public static double GetDouble()
186-
{
187-
var bytes = RandomNumberGenerator.GetBytes(8);
188-
var ul = BitConverter.ToUInt64(bytes, 0) / (1 << 11);
189-
return ul / (double)(1UL << 53);
190-
}
191-
}
192-
193-
/// <summary>
194-
/// Describes the dead letter queue and exchange configuration.
195-
/// </summary>
196-
public sealed record DeadLetterDefinition
197-
{
198-
/// <summary>
199-
/// Gets or sets the dead letter queue name, or <see langword="null"/> to use the default.
200-
/// </summary>
201-
public string? QueueName { get; set; }
202-
/// <summary>
203-
/// Gets or sets the dead letter exchange name, or <see langword="null"/> to use the default.
204-
/// </summary>
205-
public string? ExchangeName { get; set; }
206-
/// <summary>
207-
/// Gets or sets a value indicating whether dead letter routing is enabled.
208-
/// </summary>
209-
public bool Enabled { get; set; }
210-
}
211-
212-
/// <summary>
213-
/// Fluent builder for constructing a <see cref="RetryPolicyDefinition"/>.
214-
/// </summary>
215-
public sealed class RetryPolicyConfigurator
216-
{
217-
private readonly HashSet<Type> _ignoredExceptions = [];
218-
private List<TimeSpan> _intervals = [];
219-
/// <summary>
220-
/// Gets the maximum number of retry attempts configured via one of the interval methods.
221-
/// </summary>
222-
public int RetryLimit { get; private set; }
223-
private double _jitterFactor;
224-
private bool _inMemory;
225-
/// <summary>
226-
/// Gets the configured delay intervals between successive retry attempts.
227-
/// </summary>
228-
public IReadOnlyList<TimeSpan> Intervals => _intervals;
229-
/// <summary>
230-
/// Gets the exception types that should be excluded from retry attempts and immediately surfaced.
231-
/// </summary>
232-
public IReadOnlySet<Type> IgnoredExceptions => _ignoredExceptions;
233-
234-
/// <summary>
235-
/// Configures immediate retries with zero delay.
236-
/// </summary>
237-
/// <param name="retryCount">The number of retries.</param>
238-
public void Immediate(int retryCount)
239-
{
240-
RetryLimit = retryCount;
241-
_intervals = [.. Enumerable.Repeat(TimeSpan.Zero, retryCount)];
242-
}
243-
244-
/// <summary>
245-
/// Configures explicit retry intervals.
246-
/// </summary>
247-
/// <param name="intervals">The delay between each retry attempt.</param>
248-
public void SetIntervals(params TimeSpan[] intervals)
249-
{
250-
RetryLimit = intervals.Length;
251-
_intervals = [.. intervals];
252-
}
253-
/// <summary>
254-
/// Adds random jitter to retry intervals.
255-
/// </summary>
256-
/// <param name="factor">The jitter factor between 0.0 and 1.0.</param>
257-
/// <exception cref="ArgumentOutOfRangeException">Thrown when <paramref name="factor"/> is outside the valid range.</exception>
258-
public void UseJitter(double factor)
259-
{
260-
if (factor is < 0 or > 1.0)
261-
{
262-
throw new ArgumentOutOfRangeException(nameof(factor), "Jitter must be between 0.0 and 1.0");
263-
}
264-
265-
_jitterFactor = factor;
266-
}
267-
/// <summary>
268-
/// Configures exponential back-off retry intervals.
269-
/// </summary>
270-
/// <param name="retryCount">The number of retries.</param>
271-
/// <param name="initialInterval">The delay for the first retry.</param>
272-
/// <param name="maxInterval">The maximum delay cap.</param>
273-
/// <param name="multiplier">The multiplier applied to each successive interval. Default is 2.0.</param>
274-
public void Exponential(
275-
int retryCount,
276-
TimeSpan initialInterval,
277-
TimeSpan maxInterval,
278-
double multiplier = 2.0)
279-
{
280-
RetryLimit = retryCount;
281-
var intervals = new List<TimeSpan>();
282-
var currentInterval = initialInterval;
283-
284-
for (int i = 0; i < retryCount; i++)
285-
{
286-
intervals.Add(currentInterval);
287-
288-
// Calculate next interval
289-
double nextTicks = currentInterval.Ticks * multiplier;
290-
currentInterval = TimeSpan.FromTicks((long)nextTicks);
291-
292-
// Cap it at the max interval
293-
if (currentInterval > maxInterval)
294-
{
295-
currentInterval = maxInterval;
296-
}
297-
}
298-
299-
_intervals = [.. intervals];
300-
}
301-
302-
/// <summary>
303-
/// Adds an exception type to the ignore list; matching exceptions will not trigger retries.
304-
/// </summary>
305-
/// <typeparam name="TException">The exception type to ignore.</typeparam>
306-
public void Ignore<TException>() where TException : Exception => _ignoredExceptions.Add(typeof(TException));
307-
308-
/// <summary>
309-
/// Configures retries to run in-memory: the consumer is re-invoked in-process while the delivery is held,
310-
/// instead of being re-delivered later through the retry queue. This preserves message order, so it is the
311-
/// behavior used automatically for partitioned queues.
312-
/// </summary>
313-
public void InMemory() => _inMemory = true;
314-
315-
/// <summary>
316-
/// Builds the <see cref="RetryPolicyDefinition"/> from the current configuration.
317-
/// </summary>
318-
/// <returns>The constructed retry policy definition.</returns>
319-
public RetryPolicyDefinition Build()
320-
{
321-
var definition = new RetryPolicyDefinition
322-
{
323-
MaxRetryCount = RetryLimit,
324-
JitterFactor = _jitterFactor,
325-
InMemory = _inMemory
326-
};
327-
328-
foreach (var interval in _intervals)
329-
{
330-
definition.Intervals.Add(interval);
331-
}
332-
333-
foreach (var type in _ignoredExceptions)
334-
{
335-
definition.AddIgnoredType(type);
336-
definition.IgnoreExceptions.Add(type.AssemblyQualifiedName!);
337-
}
338-
339-
return definition;
340-
}
341-
}

src/Vulthil.Messaging/Queues/QueueConfigurator.cs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,6 @@ public IQueueConfigurator AddRequestConsumer<TConsumer>(Action<IRequestConfigura
7575
var reqType = new MessageType(args[0]);
7676
var resType = args[1];
7777

78-
// Preservation of your original validation
7978
if (!_messagingOptions.RegisterRequestType(reqType))
8079
{
8180
throw new InvalidOperationException($"Request '{reqType.Name}' is already handled elsewhere.");
@@ -168,10 +167,9 @@ public IQueueConfigurator UseSingleActiveConsumer()
168167
/// </exception>
169168
internal void Build()
170169
{
171-
// 1. Auto-subscribe any concrete TMessage from consumer registrations not yet subscribed.
172-
// Routing keys are a Subscription-level concern, so auto-subscribed
173-
// subscriptions get a null routing key (broker uses an empty pattern). For direct/topic exchanges that
174-
// require a specific pattern, the caller must explicitly q.Subscribe<TConcrete>("pattern") first.
170+
// Routing keys are a Subscription-level concern, so auto-subscribed subscriptions get a null routing
171+
// key (broker uses an empty pattern). For direct/topic exchanges that require a specific pattern, the
172+
// caller must explicitly q.Subscribe<TConcrete>("pattern") first.
175173
var concreteConsumerMessageTypes = _queueDefinition.Registrations
176174
.Select(r => r.MessageType)
177175
.Where(m => m.Type is { IsAbstract: false, IsInterface: false });
@@ -181,7 +179,7 @@ internal void Build()
181179
_queueDefinition.AddSubscription(new Subscription(messageType));
182180
}
183181

184-
// 2. Request consumers cannot be polymorphic — the response type is fixed and can't be selected
182+
// Request consumers cannot be polymorphic — the response type is fixed and can't be selected
185183
// by the incoming concrete type.
186184
foreach (var rpc in _queueDefinition.Registrations.OfType<RequestConsumerRegistration>())
187185
{
@@ -194,7 +192,6 @@ internal void Build()
194192
}
195193
}
196194

197-
// 3. Every consumer must have at least one matching concrete subscription.
198195
var orphanConsumer = _queueDefinition.Registrations.FirstOrDefault(
199196
r => !_queueDefinition.Subscriptions.Any(s => r.MessageType.Type.IsAssignableFrom(s.MessageType.Type)));
200197
if (orphanConsumer is not null)
@@ -205,7 +202,6 @@ internal void Build()
205202
"Call q.Subscribe<TConcrete>() or q.SubscribeAll<TInterface>(assembly) for at least one implementer.");
206203
}
207204

208-
// 4. Every subscription must have at least one matching consumer.
209205
var orphanSubscription = _queueDefinition.Subscriptions.FirstOrDefault(
210206
s => !_queueDefinition.Registrations.Any(r => r.MessageType.Type.IsAssignableFrom(s.MessageType.Type)));
211207
if (orphanSubscription is not null)
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using System.Security.Cryptography;
2+
3+
namespace Vulthil.Messaging.Queues;
4+
5+
internal static class RandomNumberGeneratorExtensions
6+
{
7+
/// <summary>
8+
/// Generates a cryptographically random <see langword="double"/> in the range [0, 1). Retry jitter does
9+
/// not need cryptographic strength, but the repository enforces CA5394 (no <see cref="Random"/>) as an
10+
/// error, so the secure generator is used here as well.
11+
/// </summary>
12+
public static double GetDouble()
13+
{
14+
var bytes = RandomNumberGenerator.GetBytes(8);
15+
var ul = BitConverter.ToUInt64(bytes, 0) / (1 << 11);
16+
return ul / (double)(1UL << 53);
17+
}
18+
}

0 commit comments

Comments
 (0)