diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d071e4..240c251 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,19 +45,6 @@ To have a consistent experience across all packages, some public interfaces have ### Changed -- **request.result:** Document that `Result`/`Result` equality treats any two failures as equal, ignoring error content; branch on the `Error` directly for failure-specific logic (see PLAN.md Inconsistencies#E) - -### Fixed - -- **request.result:** Correct the namespace in the `Prelude` doc comment (`Geekeey.Extensions.Result` → `Geekeey.Request.Result`) -- **request.result:** Fold `IsSuccess` into `Result.GetHashCode` to avoid collisions between a failure and a success value whose hash is `0` -- **request.validation:** Guard null comparison bounds in `GreaterThan`/`LessThan`/`Between` etc. so a `null` bound no longer throws (`NullReferenceException`) -- **request.validation:** Return no problems when validating a `null` instance of a reference type instead of dereferencing it (`NullReferenceException`) -- **request.validation:** Make `Severity` meaningful — `Validation.IsValid` now only fails on `Error` problems; `Warning`/`Info` no longer invalidate. Added `Validation.IsValidFor(Severity)` to set the failing-severity threshold (see PLAN.md Inconsistencies#B) -- **request.dispatcher:** Throw `InvalidOperationException` listing candidates when multiple handlers match a request instead of silently resolving via `.First()` (matches MediatR). Also throws when no handler is registered (see PLAN.md Inconsistencies#C) -- **request.dispatcher:** Resolve handlers/behaviors in registration order across closed and open generics instead of the fixed closed-before-open reflection order (see PLAN.md Inconsistencies#D) -- **request.dispatcher:** Add `AddBehavior(order:)` registration-time API so pipeline behavior ordering is explicit and deterministic (lower order runs first, registration order as tie-break) instead of relying on reflection/discovery order (see PLAN.md Tips — Explicit pipeline ordering API) - ### Removed [1.0.0]: https://code.geekeey.de/geekeey/request/releases/tag/1.0.0 diff --git a/src/request.dispatcher.tests/ScalarBehaviourTests.cs b/src/request.dispatcher.tests/ScalarBehaviourTests.cs index d2a6dd6..9c10e4b 100644 --- a/src/request.dispatcher.tests/ScalarBehaviourTests.cs +++ b/src/request.dispatcher.tests/ScalarBehaviourTests.cs @@ -87,26 +87,6 @@ internal sealed class ScalarBehaviourTests await Assert.That(tracker.Executed).IsTrue(); } - [Test] - public async Task I_can_order_behaviours_explicitly_by_order_property() - { - var sc = new ServiceCollection(); - sc.AddSingleton(); - sc.AddRequestDispatcher(builder => builder - .Add(typeof(ScalarTestHandler)) - .AddBehavior(2) - .AddBehavior(1)); - var provider = sc.BuildServiceProvider(); - var dispatcher = provider.GetRequiredService(); - var tracker = provider.GetRequiredService(); - - var request = new ScalarTestRequest { Value = "Hello" }; - await dispatcher.DispatchAsync(request); - - // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. - await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); - } - [Test] public async Task I_can_maintain_the_ordering_between_open_and_closed_behaviours() { diff --git a/src/request.dispatcher.tests/ScalarDispatcherTests.cs b/src/request.dispatcher.tests/ScalarDispatcherTests.cs index f274da9..5c10998 100644 --- a/src/request.dispatcher.tests/ScalarDispatcherTests.cs +++ b/src/request.dispatcher.tests/ScalarDispatcherTests.cs @@ -116,7 +116,7 @@ internal sealed class ScalarDispatcherTests } [Test] - public async Task I_can_see_it_throw_on_ambiguous_concrete_and_constrained_scalar_handlers() + public async Task I_can_dispatch_a_request_async_with_an_interface_constrained_handler() { var sc = new ServiceCollection(); sc.AddRequestDispatcher(builder => builder @@ -126,18 +126,13 @@ internal sealed class ScalarDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; + var result = await dispatcher.DispatchAsync(request); // Both InterfaceInheritedScalarHandler and InterfaceConstrainedScalarHandler could match. // InterfaceInheritedScalarHandler is a concrete match for InterfaceInheritedScalarRequest. // InterfaceConstrainedScalarHandler is an open generic match. - // Ambiguity is no longer resolved silently with .First(); it throws instead. - var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); - - using (Assert.Multiple()) - { - await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedScalarHandler)); - await Assert.That(ex?.Message).Contains("InterfaceConstrainedScalarHandler"); - } + // Currently Dispatcher.SendAsync checks concrete handlers first. + await Assert.That(result).IsEquivalentTo("Constrained-InterfaceHandled"); } [Test] @@ -239,49 +234,6 @@ internal sealed class ScalarDispatcherTests } } - [Test] - public async Task I_can_see_it_throw_on_ambiguous_scalar_handlers() - { - var sc = new ServiceCollection(); - sc.AddRequestDispatcher(builder => builder - .Add(typeof(DuplicateScalarHandlerA)) - .Add(typeof(DuplicateScalarHandlerB))); - var provider = sc.BuildServiceProvider(); - var dispatcher = provider.GetRequiredService(); - - var request = new DuplicateScalarRequest(); - var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); - - using (Assert.Multiple()) - { - await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerA)); - await Assert.That(ex?.Message).Contains(nameof(DuplicateScalarHandlerB)); - } - } - - [Test] - public async Task I_can_see_candidates_ordered_by_registration_not_closed_first() - { - var sc = new ServiceCollection(); - sc.AddRequestDispatcher(builder => builder - .Add(typeof(InterfaceConstrainedScalarHandler<>)) - .Add(typeof(InterfaceInheritedScalarHandler))); - var provider = sc.BuildServiceProvider(); - var dispatcher = provider.GetRequiredService(); - - var request = new InterfaceInheritedScalarRequest { Name = "Constrained" }; - - // The open generic handler is registered before the closed handler, so candidates must be - // listed in registration order (open first), not the old fixed closed-before-open order. - var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request)).Throws(); - - var message = ex?.Message ?? string.Empty; - var openIndex = message.IndexOf("InterfaceConstrainedScalarHandler", StringComparison.Ordinal); - var closedIndex = message.IndexOf("InterfaceInheritedScalarHandler", StringComparison.Ordinal); - - await Assert.That(openIndex >= 0 && closedIndex >= 0 && openIndex < closedIndex).IsTrue(); - } - [Test] public async Task I_can_see_it_throw_if_dispatcher_options_are_modified_after_build() { diff --git a/src/request.dispatcher.tests/StreamBehaviourTests.cs b/src/request.dispatcher.tests/StreamBehaviourTests.cs index 7e9377f..c96b530 100644 --- a/src/request.dispatcher.tests/StreamBehaviourTests.cs +++ b/src/request.dispatcher.tests/StreamBehaviourTests.cs @@ -66,26 +66,6 @@ internal sealed class StreamBehaviourTests await Assert.That(tracker.Log[1]).IsEquivalentTo("Behaviour2"); } - [Test] - public async Task I_can_order_behaviours_explicitly_by_order_property() - { - var sc = new ServiceCollection(); - sc.AddSingleton(); - sc.AddRequestDispatcher(builder => builder - .Add(typeof(StreamTestHandler)) - .AddBehavior(2) - .AddBehavior(1)); - var provider = sc.BuildServiceProvider(); - var dispatcher = provider.GetRequiredService(); - var tracker = provider.GetRequiredService(); - - var request = new StreamTestRequest { Value = "Hello" }; - await dispatcher.DispatchAsync(request).ToListAsync(); - - // Registered A (order 2) then B (order 1) via AddBehavior; explicit order overrides discovery order. - await Assert.That(tracker.Log).IsEquivalentTo(["OrderedB", "OrderedA"]); - } - [Test] public async Task I_can_work_with_a_generic_wrapper_request_and_the_open_behaviour() { diff --git a/src/request.dispatcher.tests/StreamDispatcherTests.cs b/src/request.dispatcher.tests/StreamDispatcherTests.cs index 5fcc7b2..c06cb36 100644 --- a/src/request.dispatcher.tests/StreamDispatcherTests.cs +++ b/src/request.dispatcher.tests/StreamDispatcherTests.cs @@ -7,26 +7,6 @@ namespace Geekeey.Request.Dispatcher.Tests; public class StreamDispatcherTests { - [Test] - public async Task I_can_see_it_throw_on_ambiguous_stream_handlers() - { - var sc = new ServiceCollection(); - sc.AddRequestDispatcher(builder => builder - .Add(typeof(DuplicateStreamHandlerA)) - .Add(typeof(DuplicateStreamHandlerB))); - var provider = sc.BuildServiceProvider(); - var dispatcher = provider.GetRequiredService(); - - var request = new DuplicateStreamRequest(); - var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); - - using (Assert.Multiple()) - { - await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerA)); - await Assert.That(ex?.Message).Contains(nameof(DuplicateStreamHandlerB)); - } - } - [Test] public async Task I_can_dispatch_a_request_async_with_an_open_generic_handler() { @@ -141,18 +121,13 @@ public class StreamDispatcherTests var dispatcher = provider.GetRequiredService(); var request = new InterfaceInheritedStreamRequest { Name = "Constrained" }; + var results = await dispatcher.DispatchAsync(request).ToListAsync(); // Both InterfaceInheritedStreamHandler and InterfaceConstrainedStreamHandler could match. // InterfaceInheritedStreamHandler is a concrete match for InterfaceInheritedStreamRequest. // InterfaceConstrainedStreamHandler is an open generic match. - // Ambiguity is no longer resolved silently with .First(); it throws instead. - var ex = await Assert.That(async () => await dispatcher.DispatchAsync(request).ToListAsync()).Throws(); - - using (Assert.Multiple()) - { - await Assert.That(ex?.Message).Contains(nameof(InterfaceInheritedStreamHandler)); - await Assert.That(ex?.Message).Contains("InterfaceConstrainedStreamHandler"); - } + // Currently Dispatcher checks concrete handlers first. + await Assert.That(results).IsEquivalentTo(["Constrained-InterfaceHandled-0", "Constrained-InterfaceHandled-1"]); } [Test] diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs deleted file mode 100644 index 6a0504e..0000000 --- a/src/request.dispatcher.tests/_fixtures/DuplicateScalarHandler.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Dispatcher.Tests; - -public sealed class DuplicateScalarRequest : IScalarRequest -{ -} - -public sealed class DuplicateScalarHandlerA : IScalarRequestHandler -{ - public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) - { - return Task.FromResult("A"); - } -} - -public sealed class DuplicateScalarHandlerB : IScalarRequestHandler -{ - public Task HandleAsync(DuplicateScalarRequest request, CancellationToken cancellationToken) - { - return Task.FromResult("B"); - } -} diff --git a/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs b/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs deleted file mode 100644 index d3f0414..0000000 --- a/src/request.dispatcher.tests/_fixtures/DuplicateStreamHandler.cs +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Dispatcher.Tests; - -public sealed class DuplicateStreamRequest : IStreamRequest -{ -} - -public sealed class DuplicateStreamHandlerA : IStreamRequestHandler -{ - public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) - { - yield return "A"; - } -} - -public sealed class DuplicateStreamHandlerB : IStreamRequestHandler -{ - public async IAsyncEnumerable HandleAsync(DuplicateStreamRequest request, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) - { - yield return "B"; - } -} diff --git a/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs deleted file mode 100644 index 1eab048..0000000 --- a/src/request.dispatcher.tests/_fixtures/OrderedScalarBehavior.cs +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Dispatcher.Tests; - -public class OrderedScalarBehaviorA(ScalarTestTracker tracker) : IScalarRequestBehavior -{ - public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) - { - tracker.Log.Add("OrderedA"); - return await next(request, cancellationToken); - } -} - -public class OrderedScalarBehaviorB(ScalarTestTracker tracker) : IScalarRequestBehavior -{ - public async Task HandleAsync(ScalarTestRequest request, ScalarHandlerDelegate next, CancellationToken cancellationToken) - { - tracker.Log.Add("OrderedB"); - return await next(request, cancellationToken); - } -} diff --git a/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs b/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs deleted file mode 100644 index 0439f21..0000000 --- a/src/request.dispatcher.tests/_fixtures/OrderedStreamBehavior.cs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Dispatcher.Tests; - -public class OrderedStreamBehaviorA(StreamTestTracker tracker) : IStreamRequestBehavior -{ - public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) - { - tracker.Log.Add("OrderedA"); - await foreach (var item in next(request, cancellationToken)) - { - yield return item; - } - } -} - -public class OrderedStreamBehaviorB(StreamTestTracker tracker) : IStreamRequestBehavior -{ - public async IAsyncEnumerable HandleAsync(StreamTestRequest request, StreamHandlerDelegate next, [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken cancellationToken) - { - tracker.Log.Add("OrderedB"); - await foreach (var item in next(request, cancellationToken)) - { - yield return item; - } - } -} diff --git a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs index c2b6976..8d53e3b 100644 --- a/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs +++ b/src/request.dispatcher/RequestDispatcherBuilderExtensions.cs @@ -70,27 +70,6 @@ public static class RequestDispatcherBuilderExtensions return builder; } - /// - /// Adds the specified pipeline behavior to the request dispatcher configuration, with an explicit - /// pipeline used to determine its position in the pipeline. Lower values run - /// first (outermost); behaviors with equal order run in registration order. - /// - /// The behavior type to add. Must implement a request behavior interface. - /// The to configure. - /// The pipeline order for this behavior. Defaults to 0. - /// The instance for further configuration. - public static IRequestDispatcherBuilder AddBehavior(this IRequestDispatcherBuilder builder, int order = 0) - where TBehavior : class - { - ArgumentNullException.ThrowIfNull(builder); - ValidateNoNestedRequestHandlers([typeof(TBehavior)]); - - builder.Services.AddOptions() - .Configure(options => options.Inspect([typeof(TBehavior)], order)); - - return builder; - } - /// /// Adds the specified type to the request dispatcher configuration for inspection. /// diff --git a/src/request.dispatcher/RequestDispatcherOptions.cs b/src/request.dispatcher/RequestDispatcherOptions.cs index f7f8c68..d18def9 100644 --- a/src/request.dispatcher/RequestDispatcherOptions.cs +++ b/src/request.dispatcher/RequestDispatcherOptions.cs @@ -10,27 +10,24 @@ namespace Geekeey.Request.Dispatcher; internal sealed class RequestDispatcherOptions { - private readonly List<(Type Type, int? Order)> _search = []; + private readonly List _search = []; private readonly Lazy _behaviorsTypeIndex; private readonly Lazy _handlersTypeIndex; public RequestDispatcherOptions() { - _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.DistinctBy(x => x.Type))); - _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.DistinctBy(x => x.Type))); + _behaviorsTypeIndex = new Lazy(() => new BehaviorTypeIndex(_search.Distinct())); + _handlersTypeIndex = new Lazy(() => new HandlerTypeIndex(_search.Distinct())); } - public void Inspect(IEnumerable assembly, int? order = null) + public void Inspect(IEnumerable assembly) { if (_behaviorsTypeIndex.IsValueCreated || _handlersTypeIndex.IsValueCreated) { throw new InvalidOperationException("The type index has already been created. Cannot inspect new assemblies."); } - foreach (var type in assembly) - { - _search.Add((type, order)); - } + _search.AddRange(assembly); } public IEnumerable GetRequestBehaviors(IServiceProvider services) @@ -49,22 +46,11 @@ internal sealed class RequestDispatcherOptions protected readonly Dictionary> _closedTypeInfo = []; protected readonly List _openTypeInfo = []; - protected readonly Dictionary _order = []; - protected readonly Dictionary _explicitOrder = []; - protected TypeIndex(IEnumerable<(Type Type, int? Order)> collection, Func predicate) + protected TypeIndex(IEnumerable collection, Func predicate) { - var index = 0; - - foreach (var (type, order) in collection) + foreach (var type in collection) { - _order[type] = index++; - - if (order is { } explicitOrder) - { - _explicitOrder[type] = explicitOrder; - } - if (type.IsGenericTypeDefinition) { if (type.GetInterfaces().Any(predicate)) @@ -82,11 +68,6 @@ internal sealed class RequestDispatcherOptions } } - protected int EffectiveOrder(Type type) - { - return _explicitOrder.TryGetValue(type, out var order) ? order : _order[type]; - } - public IEnumerable Resolve(IServiceProvider services) { return (IEnumerable)_cache.GetOrAdd(typeof(T), CreateResolverFactory)(services); @@ -116,19 +97,16 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestHandler<,>)); } - private sealed class HandlerTypeIndex(IEnumerable<(Type Type, int? Order)> collection) + private sealed class HandlerTypeIndex(IEnumerable collection) : TypeIndex(collection, IsRequestHandlerType) { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List<(int Order, Type Type)>(); + var result = new List(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - foreach (var type in list) - { - result.Add((EffectiveOrder(type), type)); - } + result.AddRange(list); } var requestType = @interface.GetGenericArguments()[0]; @@ -143,7 +121,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType); if (impl.IsAssignableTo(@interface)) { - result.Add((EffectiveOrder(type), impl)); + result.Add(impl); } } catch (ArgumentException) @@ -159,7 +137,7 @@ internal sealed class RequestDispatcherOptions var impl = type.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((EffectiveOrder(type), impl)); + result.Add(impl); } } } @@ -168,7 +146,7 @@ internal sealed class RequestDispatcherOptions } } - return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; + return result; } } @@ -179,19 +157,16 @@ internal sealed class RequestDispatcherOptions type.GetGenericTypeDefinition() == typeof(IStreamRequestBehavior<,>)); } - private sealed class BehaviorTypeIndex(IEnumerable<(Type Type, int? Order)> collection) + private sealed class BehaviorTypeIndex(IEnumerable collection) : TypeIndex(collection, IsRequestBehaviorType) { protected override IReadOnlyList IsAssignableTo(Type @interface) { - var result = new List<(int Order, Type Type)>(); + var result = new List(); if (_closedTypeInfo.TryGetValue(@interface, out var list)) { - foreach (var type in list) - { - result.Add((EffectiveOrder(type), type)); - } + result.AddRange(list); } var requestType = @interface.GetGenericArguments()[0]; @@ -205,7 +180,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType, responseType); if (impl.IsAssignableTo(@interface)) { - result.Add((EffectiveOrder(behaviour), impl)); + result.Add(impl); } } catch (ArgumentException) @@ -218,7 +193,7 @@ internal sealed class RequestDispatcherOptions var impl = behaviour.MakeGenericType(requestType.GetGenericArguments()); if (impl.IsAssignableTo(@interface)) { - result.Add((EffectiveOrder(behaviour), impl)); + result.Add(impl); } } catch (ArgumentException) @@ -226,7 +201,7 @@ internal sealed class RequestDispatcherOptions } } - return [.. result.OrderBy(static item => item.Order).Select(static item => item.Type)]; + return result; } } } diff --git a/src/request.dispatcher/ScalarRequestInvoker.cs b/src/request.dispatcher/ScalarRequestInvoker.cs index 2c6d129..63de03e 100644 --- a/src/request.dispatcher/ScalarRequestInvoker.cs +++ b/src/request.dispatcher/ScalarRequestInvoker.cs @@ -43,20 +43,7 @@ internal sealed class ScalarRequestInvoker : ScalarRequestI Task Head(IScalarRequest r, CancellationToken ct) { - var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); - - if (handlers.Length is 0) - { - throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); - } - - if (handlers.Length > 1) - { - var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); - throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); - } - - return handlers[0].HandleAsync((TRequest)r, ct); + return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); } } } diff --git a/src/request.dispatcher/StreamRequestInvoker.cs b/src/request.dispatcher/StreamRequestInvoker.cs index 817b5b2..882d538 100644 --- a/src/request.dispatcher/StreamRequestInvoker.cs +++ b/src/request.dispatcher/StreamRequestInvoker.cs @@ -47,20 +47,7 @@ internal sealed class StreamRequestInvoker : StreamRequestI IAsyncEnumerable Head(IStreamRequest r, CancellationToken ct) { - var handlers = options.GetRequestHandlers>(serviceProvider).ToArray(); - - if (handlers.Length is 0) - { - throw new InvalidOperationException($"No handler is registered for request '{typeof(TRequest).FullName}'."); - } - - if (handlers.Length > 1) - { - var candidates = string.Join(", ", handlers.Select(handler => handler.GetType().FullName)); - throw new InvalidOperationException($"Multiple handlers are registered for request '{typeof(TRequest).FullName}'. Ambiguous handlers: {candidates}"); - } - - return handlers[0].HandleAsync((TRequest)r, ct); + return options.GetRequestHandlers>(serviceProvider).First().HandleAsync((TRequest)r, ct); } } } diff --git a/src/request.result.tests/ResultEqualityTests.cs b/src/request.result.tests/ResultEqualityTests.cs index 99d7094..67233dd 100644 --- a/src/request.result.tests/ResultEqualityTests.cs +++ b/src/request.result.tests/ResultEqualityTests.cs @@ -166,48 +166,11 @@ internal sealed class ResultEqualityTests } [Test] - public async Task I_can_get_hashcode_and_get_non_zero_for_failure() + public async Task I_can_get_hashcode_and_get_zero_for_failure() { var result = Prelude.Failure("error"); - await Assert.That(result.GetHashCode()).IsNotEqualTo(0); - } - - [Test] - public async Task I_can_get_hashcode_and_not_collide_non_generic_success_and_failure() - { - await Assert.That(Prelude.Success().GetHashCode()) - .IsNotEqualTo(Prelude.Failure("error").GetHashCode()); - } - - [Test] - public async Task I_can_get_hashcode_and_not_collide_success_value_zero_with_failure() - { - var success = Prelude.Success(0); - var failure = Prelude.Failure("error"); - - await Assert.That(success.Equals(failure)).IsFalse(); - await Assert.That(success.GetHashCode()).IsNotEqualTo(failure.GetHashCode()); - } - - [Test] - public async Task I_can_equal_failure_and_failure_ignoring_error_content() - { - var a = Prelude.Failure("error 1"); - var b = Prelude.Failure("error 2"); - - await Assert.That(a.Equals(b)).IsTrue(); - await Assert.That(a.Error).IsNotEqualTo(b.Error); - } - - [Test] - public async Task I_can_equal_non_generic_failure_and_failure_ignoring_error_content() - { - var a = Prelude.Failure("error 1"); - var b = Prelude.Failure("error 2"); - - await Assert.That(a.Equals(b)).IsTrue(); - await Assert.That(a.Error).IsNotEqualTo(b.Error); + await Assert.That(result.GetHashCode()).IsZero(); } [Test] @@ -236,53 +199,4 @@ internal sealed class ResultEqualityTests await Assert.That(a.Equals(b)).IsFalse(); } - - [Test] - public async Task I_can_use_equality_operator_and_get_false_for_success_and_failure() - { - var success = Prelude.Success(2); - var failure = Prelude.Failure("error"); - - await Assert.That(success == failure).IsFalse(); - await Assert.That(success != failure).IsTrue(); - } - - [Test] - public async Task I_can_use_equality_operator_and_get_true_for_successes_with_equal_value() - { - await Assert.That(Prelude.Success(2) == Prelude.Success(2)).IsTrue(); - await Assert.That(Prelude.Success(2) != Prelude.Success(3)).IsTrue(); - } - - [Test] - public async Task I_can_use_equality_operator_and_compare_result_to_value() - { - await Assert.That(Prelude.Success(2) == 2).IsTrue(); - await Assert.That(Prelude.Success(2) != 3).IsTrue(); - await Assert.That(Prelude.Failure("x") == 2).IsFalse(); - } - - [Test] - public async Task I_can_equal_object_and_get_false_for_null() - { - object result = Prelude.Success(2); - - await Assert.That(result.Equals(null)).IsFalse(); - } - - [Test] - public async Task I_can_equal_object_and_get_false_for_wrong_type() - { - object result = Prelude.Success(2); - - await Assert.That(result.Equals("not a result")).IsFalse(); - } - - [Test] - public async Task I_can_equal_object_and_get_true_for_boxed_equal_result() - { - object result = Prelude.Success(2); - - await Assert.That(result.Equals(Prelude.Success(2))).IsTrue(); - } } diff --git a/src/request.result/Prelude.cs b/src/request.result/Prelude.cs index e59349d..de48354 100644 --- a/src/request.result/Prelude.cs +++ b/src/request.result/Prelude.cs @@ -9,7 +9,7 @@ namespace Geekeey.Request.Result; /// A class containing various utility methods, a 'prelude' to the rest of the library. /// /// -/// This class is meant to be imported statically, e.g. using static Geekeey.Request.Result.Prelude;. +/// This class is meant to be imported statically, e.g. using static Geekeey.Extensions.Result.Prelude;. /// Recommended to be imported globally via a global using statement. /// public static class Prelude diff --git a/src/request.result/Result.Equality.cs b/src/request.result/Result.Equality.cs index da9372d..0253b88 100644 --- a/src/request.result/Result.Equality.cs +++ b/src/request.result/Result.Equality.cs @@ -9,15 +9,7 @@ namespace Geekeey.Request.Result; public partial class Result : IEquatable { - /// - /// Checks whether two results are equal. Results are equal if both are success or both are failure. - /// - /// - /// Two failures are considered equal regardless of their content. Equality therefore - /// ignores the error; if you need to branch on a specific failure, compare the values - /// directly instead of relying on equality. - /// - /// The result to check for equality with the current result. + /// [Pure] public bool Equals(Result? other) { @@ -57,8 +49,7 @@ public partial class Result : IEquatable public partial class Result : IEqualityOperators { /// - /// Checks whether two results are equal. Results are equal if they are both success or both failure. Two - /// failures are equal regardless of their error content. + /// Checks whether two results are equal. Results are equal if they are both success or both failure. /// /// The first result to compare. /// The second result to compare. @@ -88,11 +79,6 @@ public partial class Result : IEquatable>, IEquatable /// Checks whether the result is equal to another result. Results are equal if both results are success values and /// the success values are equal, or if both results are failures. /// - /// - /// Two failures are considered equal regardless of their content. Equality therefore - /// ignores the error; if you need to branch on a specific failure, compare the values - /// directly instead of relying on equality. - /// /// The result to check for equality with the current result. [Pure] public bool Equals(Result? other) @@ -195,12 +181,7 @@ public partial class Result : IEquatable>, IEquatable return comparer.GetHashCode(result.Value); } - if (result is { IsSuccess: true, Value: null }) - { - return 0; - } - - return result.Error?.GetHashCode() ?? 0; + return 0; } } @@ -208,7 +189,7 @@ public partial class Result : IEqualityOperators, Result, bool>, { /// /// Checks whether two results are equal. Results are equal if both results are success values and the success - /// values are equal, or if both results are failures. Two failures are equal regardless of their error content. + /// values are equal, or if both results are failures. /// /// The first result to compare. /// The second result to compare. diff --git a/src/request.validation.tests/RuleBuilderExtensionsTests.cs b/src/request.validation.tests/RuleBuilderExtensionsTests.cs index c0d2cae..5b5271a 100644 --- a/src/request.validation.tests/RuleBuilderExtensionsTests.cs +++ b/src/request.validation.tests/RuleBuilderExtensionsTests.cs @@ -314,71 +314,6 @@ internal sealed class RuleBuilderExtensionsTests .Throws(); } - [Test] - public async Task I_can_validate_greater_than_with_null_bound_without_throwing() - { - var validator = new PropertyValidator(model - => model.Value, rule => rule.GreaterThan(null)); - - var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); - var ignoredNull = validator.Validate(new ComparableModel()); - - await Assert.That(valid.IsValid).IsTrue(); - await Assert.That(ignoredNull.IsValid).IsTrue(); - } - - [Test] - public async Task I_can_validate_greater_than_or_equal_to_with_null_bound_without_throwing() - { - var validator = new PropertyValidator(model - => model.Value, rule => rule.GreaterThanOrEqualTo(null)); - - var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); - var ignoredNull = validator.Validate(new ComparableModel()); - - await Assert.That(valid.IsValid).IsTrue(); - await Assert.That(ignoredNull.IsValid).IsTrue(); - } - - [Test] - public async Task I_can_validate_less_than_with_null_bound_without_throwing() - { - var validator = new PropertyValidator(model - => model.Value, rule => rule.LessThan(null)); - - var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); - var ignoredNull = validator.Validate(new ComparableModel()); - - await Assert.That(valid.IsValid).IsTrue(); - await Assert.That(ignoredNull.IsValid).IsTrue(); - } - - [Test] - public async Task I_can_validate_less_than_or_equal_to_with_null_bound_without_throwing() - { - var validator = new PropertyValidator(model - => model.Value, rule => rule.LessThanOrEqualTo(null)); - - var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); - var ignoredNull = validator.Validate(new ComparableModel()); - - await Assert.That(valid.IsValid).IsTrue(); - await Assert.That(ignoredNull.IsValid).IsTrue(); - } - - [Test] - public async Task I_can_validate_between_with_null_bounds_without_throwing() - { - var validator = new PropertyValidator(model - => model.Value, rule => rule.Between(null, null)); - - var valid = validator.Validate(new ComparableModel { Value = new ComparableValue { Number = 1 } }); - var ignoredNull = validator.Validate(new ComparableModel()); - - await Assert.That(valid.IsValid).IsTrue(); - await Assert.That(ignoredNull.IsValid).IsTrue(); - } - [Test] public async Task I_can_see_it_throw_for_invalid_between_configuration() { diff --git a/src/request.validation.tests/ValidationSeverityTests.cs b/src/request.validation.tests/ValidationSeverityTests.cs deleted file mode 100644 index 6f24c8d..0000000 --- a/src/request.validation.tests/ValidationSeverityTests.cs +++ /dev/null @@ -1,49 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Validation.Tests; - -internal sealed class ValidationSeverityTests -{ - [Test] - public async Task I_can_have_warning_problems_not_invalidate_by_default() - { - var validator = new PropertyValidator(person - => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") - .WithSeverity(Severity.Warning)); - - var result = validator.Validate(new Person { Name = "" }); - - await Assert.That(result.Problems).Count().IsEqualTo(1); - await Assert.That(result.IsValid).IsTrue(); - await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); - await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); - } - - [Test] - public async Task I_can_have_info_problems_only_invalidate_at_info_threshold() - { - var validator = new PropertyValidator(person - => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.") - .WithSeverity(Severity.Info)); - - var result = validator.Validate(new Person { Name = "" }); - - await Assert.That(result.Problems).Count().IsEqualTo(1); - await Assert.That(result.IsValid).IsTrue(); - await Assert.That(result.IsValidFor(Severity.Warning)).IsTrue(); - await Assert.That(result.IsValidFor(Severity.Info)).IsFalse(); - } - - [Test] - public async Task I_can_have_error_problems_invalidate_by_default() - { - var validator = new PropertyValidator(person - => person.Name, rule => rule.Must(value => !string.IsNullOrWhiteSpace(value), "Name is required.")); - - var result = validator.Validate(new Person { Name = "" }); - - await Assert.That(result.IsValid).IsFalse(); - await Assert.That(result.IsValidFor(Severity.Error)).IsFalse(); - } -} diff --git a/src/request.validation.tests/ValidatorTests.cs b/src/request.validation.tests/ValidatorTests.cs index 820edc4..14239f7 100644 --- a/src/request.validation.tests/ValidatorTests.cs +++ b/src/request.validation.tests/ValidatorTests.cs @@ -14,8 +14,7 @@ internal sealed class ValidatorTests var result = validator.Validate(new Person { Name = "" }); - await Assert.That(result.IsValid).IsTrue(); - await Assert.That(result.IsValidFor(Severity.Warning)).IsFalse(); + await Assert.That(result.IsValid).IsFalse(); await Assert.That(result.Problems).Count().IsEqualTo(1); var problem = result.Problems.Single(); @@ -40,24 +39,12 @@ internal sealed class ValidatorTests using (Assert.Multiple()) { - await Assert.That(invalid.IsValid).IsTrue(); - await Assert.That(invalid.IsValidFor(Severity.Warning)).IsFalse(); + await Assert.That(invalid.IsValid).IsFalse(); await Assert.That(valid.IsValid).IsTrue(); await Assert.That(valid.Problems).IsEmpty(); } } - [Test] - public async Task I_can_validate_with_a_null_instance_without_throwing() - { - var validator = new PropertyValidator(person - => person!.Name, rule => rule.Must(value => value is not null, "Name is required.")); - - var result = validator.Validate((Person?)null); - - await Assert.That(result.IsValid).IsTrue(); - } - [Test] public async Task I_can_compose_nested_validators_and_aggregate_property_paths() { diff --git a/src/request.validation.tests/_fixtures/ComparableModel.cs b/src/request.validation.tests/_fixtures/ComparableModel.cs deleted file mode 100644 index 9178228..0000000 --- a/src/request.validation.tests/_fixtures/ComparableModel.cs +++ /dev/null @@ -1,19 +0,0 @@ -// Copyright (c) The Geekeey Authors -// SPDX-License-Identifier: EUPL-1.2 - -namespace Geekeey.Request.Validation.Tests; - -internal sealed class ComparableModel -{ - public ComparableValue? Value { get; init; } -} - -internal sealed class ComparableValue : IComparable -{ - public int Number { get; init; } - - public int CompareTo(ComparableValue? other) - { - return Number.CompareTo(other!.Number); - } -} diff --git a/src/request.validation/Rule.cs b/src/request.validation/Rule.cs index a6a397c..0c65cc0 100644 --- a/src/request.validation/Rule.cs +++ b/src/request.validation/Rule.cs @@ -46,7 +46,7 @@ internal abstract record Rule : Rule if (context.Instance is null && default(T) is null) { - return []; + return Validate((T)context.Instance!, context); } var actualType = context.Instance?.GetType().FullName ?? "null"; diff --git a/src/request.validation/RuleBuilderExtensions.cs b/src/request.validation/RuleBuilderExtensions.cs index 7e1a5db..528b387 100644 --- a/src/request.validation/RuleBuilderExtensions.cs +++ b/src/request.validation/RuleBuilderExtensions.cs @@ -170,7 +170,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) > 0, + return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) > 0, $"Value must be greater than {comparisonValue}."); } @@ -189,7 +189,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) >= 0, + return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) >= 0, $"Value must be greater than or equal to {comparisonValue}."); } @@ -208,7 +208,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) < 0, + return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) < 0, $"Value must be less than {comparisonValue}."); } @@ -227,7 +227,7 @@ public static class RuleBuilderExtensions { ArgumentNullException.ThrowIfNull(rule); - return rule.Must(value => IsNull(value) || IsNull(comparisonValue) || value.CompareTo(comparisonValue) <= 0, + return rule.Must(value => IsNull(value) || value.CompareTo(comparisonValue) <= 0, $"Value must be less than or equal to {comparisonValue}."); } @@ -254,9 +254,7 @@ public static class RuleBuilderExtensions "Maximum value must be greater than or equal to minimum value."); } - return rule.Must(value => IsNull(value) - || ((minValue is null || value.CompareTo(minValue) >= 0) - && (maxValue is null || value.CompareTo(maxValue) <= 0)), + return rule.Must(value => IsNull(value) || (value.CompareTo(minValue) >= 0 && value.CompareTo(maxValue) <= 0), $"Value must be between {minValue} and {maxValue}."); } diff --git a/src/request.validation/Validation.cs b/src/request.validation/Validation.cs index 8e9c7c7..fadadff 100644 --- a/src/request.validation/Validation.cs +++ b/src/request.validation/Validation.cs @@ -23,27 +23,7 @@ public sealed class Validation /// /// Whether the validation was successful. /// - /// - /// A validation is considered successful unless it contains at least one problem with - /// . Problems of lower severity ( or - /// ) do not invalidate the result. Use - /// to control which severities are treated as failures. - /// - public bool IsValid => IsValidFor(Severity.Error); - - /// - /// Whether the validation was successful, treating problems at or above the given severity as failures. - /// - /// The minimum severity that should be considered a failure. - /// - /// A problem invalidates the validation when its is at least as severe as - /// . For example, IsValidFor(Severity.Warning) fails on errors and - /// warnings but not on informational problems. - /// - public bool IsValidFor(Severity minimum) - { - return !Problems.Any(problem => problem.Severity <= minimum); - } + public bool IsValid => Problems.Count is 0; /// /// The problems that were found during validation.