Code review pass: fix bugs and expand test coverage in CoreEx and CoreEx.RefData

[chullybun]

Summary

Thorough code review (security, perf, bugs, testing) of the CoreEx and CoreEx.RefData packages, covering ~21K lines across both. Found and fixed 14 confirmed bugs (each with a regression test verified via the fail-before/pass-after protocol: write test → confirm it fails against the pre-fix code via git stash → restore the fix → confirm it passes), and backfilled test coverage for previously-untested or thinly-tested areas. No behavior changes beyond the listed fixes.

CoreEx bug fixes

• HybridCacheEntryOptions.CreateFor<T> — used nameof(T) (always literal "T") instead of typeof(T).Name, breaking config lookup by type name.
• ExecutionContext.GetKeyedService(Type, object?) — didn't correctly resolve via IKeyedServiceProvider for interface-based DI registrations.
• ResultsExtensions.When (Result<T> overload) — the "otherwise" branch called the wrong callback.
• RuntimeMetadata.AreEqual — enumerable comparisons didn't fully enumerate/compare sequence lengths, and mismatched ICollection types could throw InvalidCastException.
• JsonSubstituteNamingPolicy — ignored the configured FallbackPolicy for unmapped names.
• JsonExceptionConverterFactory — inverted filter accidentally included [JsonIgnore]-decorated members and TargetSite.
• Extensions.HttpRequestMessage (query builder) — query values weren't URI-encoded, breaking special characters.
• DecimalRuleHelper.CalcIntegralPartLength — floating-point boundary error miscounted digits near powers of ten.
• Extensions.HttpResponseMessage.ToProblemDetailsAsync — silently swallowed OperationCanceledException.
• ExecutionContext.CreateCopy — didn't copy OperationType/IncludeRelatedText.
• CoreExExtensions.AddDynamicServicesUsing — crashed on ReflectionTypeLoadException when scanning assemblies with unloadable types.
• EncodedStringToUInt32Converter — silently truncated malformed input instead of throwing.
• IntoMapperT2.MapInto — stray [NotNullIfNotNull] attribute on a void method.
• WorkOrchestrator — Provider/JsonSerializerOptions were mutable public fields.
• HostedServiceBase.StopAsync — dangling if skipped the Stopping status transition; also missing ConfigureAwait(false).
• IReferenceDataCollection<TId,TRef>.GetById(object?) (critical) — missing cast caused guaranteed infinite recursion / StackOverflowException crash on any call through the non-generic interface.
• ETag.ParseETag — off-by-one on the weak-prefix (W/"...") case left a stray leading quote in the result.

CoreEx.RefData bug fixes

• ReferenceDataCollectionCore.GetById/GetByCode — threw KeyNotFoundException via the dictionary indexer instead of returning null on a miss, contradicting the documented/nullable contract.
• ReferenceDataCodeCollection.CopyTo — copied the internal List<string?> into the caller's TRef[] array, throwing ArgumentException on every real call.
• ReferenceDataCodeCollection.Contains — compared a TRef instance against the internal List<string?>, always returning false.
• _mappingsDict — converted to ConcurrentDictionary so mapping reads (ContainsMapping/TryGetByMapping/GetByMapping) are safe against concurrent Add() calls, consistent with the _rdcId/_rdcCode dictionaries.

Test coverage added

Backfilled previously-missing or thin coverage across both packages: HostedServiceBase/TimerHostedServiceBase/HostedServiceManager/SynchronizedTimerHostedServiceBase, Invokers, ReferenceDataOrchestrator gaps, Security (AuthenticationUser), Runtime, ExtendedException builders, ProblemDetailsException, Entities.ETag, IdentifierGenerator, BiDirectionMapper/IntoMapper, EntitiesExtensions/DataExtensions LINQ helpers, HybridCacheSynchronizer, ReferenceDataCodeCollection, ReferenceDataContext, ReferenceDataOrchestratorHealthCheck, AddReferenceDataOrchestrator DI extensions, and ReferenceDataHybridCache (including concurrent-create dedup).

One item was intentionally left as-is pending further discussion: ReferenceDataCodeCollection's ref List<string?>? constructor parameter never reassigns the caller's variable, which is misleading API surface — flagged for a follow-up, not fixed here.

Test plan

• dotnet test tests/CoreEx.Test.Unit — 972/972 passing
• dotnet test tests/CoreEx.RefData.Test.Unit — 134/134 passing
• Every bug fix has a dedicated regression test, verified to fail against the pre-fix code and pass against the fix (via git stash)

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR applies a broad bug-fix and regression-test sweep across the CoreEx and CoreEx.RefData libraries, tightening correctness for DI/service resolution, JSON serialization behaviors, hosted-service lifecycle handling, reference-data lookups, and several utility helpers.

Changes:

• Fixes multiple correctness issues in CoreEx/CoreEx.RefData (notably reference-data lookup paths, runtime metadata comparisons, DI keyed service resolution, query encoding, and ETag parsing).
• Hardens concurrency and error-handling behavior (e.g., mapping dictionaries made concurrent; cancellation propagated properly).
• Adds/expands unit-test coverage with targeted regression tests for the fixed defects.

Reviewed changes

Copilot reviewed 58 out of 58 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/CoreEx.Test.Unit/Validation/DecimalRuleHelperTests.cs	Adds regression coverage for decimal digit-counting boundary cases.
tests/CoreEx.Test.Unit/Security/AuthenticationUserTests.cs	Adds tests around AuthenticationUser defaults, statics, and record semantics.
tests/CoreEx.Test.Unit/RuntimeTests.cs	Adds coverage for Runtime ambient clock/ID helpers and ExecutionContext interaction.
tests/CoreEx.Test.Unit/Runtime/RuntimeMetadataTests.cs	Adds sequence-comparison regression tests (length mismatch and mixed collection types).
tests/CoreEx.Test.Unit/Results/ExtensionsWhenTests.cs	Adds regression tests for Result.When branch behavior.
tests/CoreEx.Test.Unit/Mapping/IntoMapperTests.cs	Adds tests for IntoMapper standard/custom mapping behaviors.
tests/CoreEx.Test.Unit/Mapping/Converters/EncodedStringToUInt32ConverterTests.cs	Adds tests asserting invalid base64 payload lengths throw.
tests/CoreEx.Test.Unit/Mapping/BiDirectionMapperTests.cs	Adds tests for BiDirectionMapper mapping + non-generic bridge behavior.
tests/CoreEx.Test.Unit/Json/JsonSubstituteNamingPolicyTests.cs	Adds tests verifying fallback policy is honored for unmapped names.
tests/CoreEx.Test.Unit/Json/JsonExceptionConverterFactoryTests.cs	Adds tests for JsonIgnore and TargetSite filtering in exception JSON conversion.
tests/CoreEx.Test.Unit/Invokers/InvokerTests.cs	Adds baseline coverage for Invoker sync runner behavior and error propagation.
tests/CoreEx.Test.Unit/Invokers/InvokerNameAttributeTests.cs	Adds tests for attribute naming and cache consistency.
tests/CoreEx.Test.Unit/Invokers/InvokerBaseTests.cs	Adds tests for InvokerBase defaults, activity hooks, and exception paths.
tests/CoreEx.Test.Unit/Http/ProblemDetailsExceptionTests.cs	Adds tests for ProblemDetailsException conversion and business-exception helpers.
tests/CoreEx.Test.Unit/Http/HttpResponseMessageExtensionsTests.cs	Adds regression tests around cancellation propagation in problem-details parsing.
tests/CoreEx.Test.Unit/Http/HttpRequestMessageExtensionsTests.cs	Adds tests ensuring query values are encoded correctly; idempotency header coverage.
tests/CoreEx.Test.Unit/Hosting/Work/WorkOrchestratorTests.cs	Updates test to use AuthenticationUser import consistently.
tests/CoreEx.Test.Unit/Hosting/TimerHostedServiceBaseTests.cs	Adds execution-loop and configuration-reading tests for TimerHostedServiceBase.
tests/CoreEx.Test.Unit/Hosting/SynchronizedTimerHostedServiceBaseTests.cs	Adds tests for synchronizer enter/exit and synchronizer name propagation.
tests/CoreEx.Test.Unit/Hosting/Synchronization/HybridCacheSynchronizerTests.cs	Adds tests for hybrid-cache synchronizer semantics and disposal cleanup.
tests/CoreEx.Test.Unit/Hosting/HostedServiceManagerTests.cs	Adds tests for HostedServiceManager operations, ambiguity, and pre-check behavior.
tests/CoreEx.Test.Unit/Hosting/HostedServiceBaseTests.cs	Adds lifecycle/status transition tests including stopping/pause/resume.
tests/CoreEx.Test.Unit/ExtendedExceptionExtensionsTests.cs	Adds tests for fluent ExtendedException builder extensions.
tests/CoreEx.Test.Unit/ExecutionContextTests.cs	Expands tests for CreateCopy and keyed-service resolution behavior.
tests/CoreEx.Test.Unit/Entities/IdentifierGeneratorTests.cs	Adds tests for identifier generation/assignment flows and ExecutionContext override.
tests/CoreEx.Test.Unit/Entities/ETagTests.cs	Adds tests for ETag parsing/formatting, hashing, and concurrency comparisons.
tests/CoreEx.Test.Unit/Entities/EntitiesExtensionsTests.cs	Adds tests for LINQ helpers, paging, and feature support helpers.
tests/CoreEx.Test.Unit/DependencyInjection/CoreExExtensionsDependencyInjectionTests.cs	Adds tests for AddDynamicServicesUsing lifetimes and overload parity.
tests/CoreEx.Test.Unit/Data/DataExtensionsTests.cs	Adds tests for IQueryable wildcard filtering and paging/total-count helpers.
tests/CoreEx.Test.Unit/Caching/HybridCacheEntryOptionsTests.cs	Adds tests ensuring type-name-based config resolution works for CreateFor.
tests/CoreEx.RefData.Test.Unit/ReferenceDataOrchestratorTests.cs	Adds regression tests for non-generic GetById and orchestrator prefetch/mapping helpers.
tests/CoreEx.RefData.Test.Unit/ReferenceDataOrchestratorHealthCheckTests.cs	Adds health-check registration/behavior coverage.
tests/CoreEx.RefData.Test.Unit/ReferenceDataHybridCacheTests.cs	Adds caching behavior tests including concurrent factory de-dup.
tests/CoreEx.RefData.Test.Unit/ReferenceDataContextTests.cs	Adds coverage for ReferenceDataContext date/indexer/reset behaviors.
tests/CoreEx.RefData.Test.Unit/ReferenceDataCollectionTests.cs	Adds tests for GetById/GetByCode miss semantics and mapping behaviors.
tests/CoreEx.RefData.Test.Unit/ReferenceDataCodeCollectionTests.cs	Adds tests for code collection operations, resolution, and constructors.
tests/CoreEx.RefData.Test.Unit/CoreExReferenceDataExtensionsTests.cs	Adds DI extension tests for orchestrator/cache/health-check registration paths.
src/CoreEx/Validation/DecimalRuleHelper.cs	Fixes digit-counting boundary error by correcting log10 estimate drift.
src/CoreEx/Results/ResultsExtensions.When.cs	Fixes Result.When “otherwise” branch to call the correct callback.
src/CoreEx/RefData/IReferenceDataCollectionT.cs	Fixes non-generic GetById implementation (previous recursion/stack overflow path).
src/CoreEx/Metadata/RuntimeMetadata.Internal.cs	Fixes enumerable comparison to ensure the right-hand sequence is fully exhausted.
src/CoreEx/Metadata/RuntimeMetadata.AreEqual.cs	Fixes ICollection mismatch casting and ensures enumerations compare full length.
src/CoreEx/Mapping/IntoMapperT3.cs	Removes invalid nullability attribute on void MapInto method.
src/CoreEx/Mapping/Converters/EncodedStringToUInt32Converter.cs	Enforces decoded byte length is exactly 4; throws FormatException otherwise.
src/CoreEx/Json/JsonSubstituteNamingPolicy.cs	Uses configured FallbackPolicy (instead of hardcoded camelCase) for unmapped names.
src/CoreEx/Json/JsonExceptionConverterFactory.cs	Corrects filtering to exclude JsonIgnore members and always omit TargetSite.
src/CoreEx/Hosting/Work/WorkOrchestrator.cs	Makes Provider/JsonSerializerOptions immutable properties (instead of mutable fields).
src/CoreEx/Hosting/HostedServiceBase.cs	Fixes StopAsync status transition and adds ConfigureAwait(false).
src/CoreEx/Extensions.HttpResponseMessage.cs	Ensures OperationCanceledException propagates rather than being swallowed.
src/CoreEx/Extensions.HttpRequestMessage.cs	Encodes query parameter values to prevent query injection/breakage.
src/CoreEx/ExecutionContext.Infra.cs	Fixes keyed-service resolution using IKeyedServiceProvider.
src/CoreEx/ExecutionContext.cs	Ensures CreateCopy copies OperationType and IncludeRelatedText.
src/CoreEx/Entities/ETag.cs	Fixes weak ETag parsing offset (W/"...") to avoid stray quote.
src/CoreEx/CoreExExtensions.DependencyInjection.cs	Prevents AddDynamicServicesUsing from crashing on ReflectionTypeLoadException.
src/CoreEx/Caching/HybridCacheEntryOptions.cs	Fixes CreateFor to use actual type name for config lookup.
src/CoreEx.RefData/ReferenceDataHybridCache.TypedInvoker.cs	Refines typed invoker caching and helper naming for TryGetByKeyAsync.
src/CoreEx.RefData/ReferenceDataCodeCollection.cs	Fixes Contains/CopyTo semantics to operate on codes and resolved ref-data items.
src/CoreEx.RefData/Abstractions/ReferenceDataCollectionCore.cs	Fixes not-found behavior (null vs KeyNotFoundException) and makes mapping index concurrent.

[Copilot]

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/CoreEx/CoreExExtensions.DependencyInjection.cs:138

• GetLoadableTypes returns IEnumerable<Type> but the ReflectionTypeLoadException path currently returns IEnumerable<Type?> (ex.Types.Where(...)). With warnings-as-errors this can fail compilation (CS8619) and also leaves nullability ambiguous. Prefer filtering and projecting to non-null explicitly (or OfType<Type>()).

    private static IEnumerable<Type> GetLoadableTypes(Assembly assembly)
    {
        try
        {
            return assembly.GetTypes();
        }
        catch (ReflectionTypeLoadException ex)
        {
            return ex.Types.Where(t => t is not null)!;
        }

src/CoreEx/RefData/IReferenceDataCollectionT.cs:49

• The explicit non-generic GetById(object?) implementation will throw InvalidCastException when id isn’t actually a TId, and it treats null as default(TId) which can accidentally match a real item (e.g., id 0/Guid.Empty). For a nullable/"not found" API it’s safer to return null for null or mismatched id types.

    /// <inheritdoc/>
    IReferenceData? IReferenceDataCollection.GetById(object? id) => GetById((TId)(id ?? default(TId)!));

tests/CoreEx.Test.Unit/Hosting/Synchronization/HybridCacheSynchronizerTests.cs:101

• This test doesn’t await the async exception assertion. As written, ThrowAsync returns a Task that isn’t awaited, so the assertion may never execute and the test can pass incorrectly. Make the test async Task and await the assertion.

[Copilot]

Pull request overview

Copilot reviewed 58 out of 58 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/CoreEx.RefData/ReferenceDataCodeCollection.cs:65

• CopyTo validates bounds using arrayIndex + Count > array.Length, which can overflow for large arrayIndex values and result in an unexpected IndexOutOfRangeException during the copy loop. It should validate using subtraction and also reject arrayIndex > array.Length explicitly.

        array.ThrowIfNull();
        if (arrayIndex < 0 || arrayIndex + Count > array.Length)
            throw new ArgumentOutOfRangeException(nameof(arrayIndex));

src/CoreEx/Metadata/RuntimeMetadata.Internal.cs:74

• The typed enumerable comparison creates enumerators without disposing them. Since IEnumerator<T> is IDisposable, use using var to ensure any underlying resources are released (matching foreach disposal semantics).

        // Ensure the right-hand sequence does not have additional trailing elements.
        return !er.MoveNext();

src/CoreEx/Metadata/RuntimeMetadata.AreEqual.cs:150

• The IEnumerable fallback comparison allocates enumerators without disposing them. Use using var for both enumerators so any disposable enumerators are cleaned up, matching foreach disposal behavior.

            // Ensure the right-hand sequence does not have additional trailing elements.
            return !er.MoveNext();