Skip to content

Commit

Permalink
Refactor async checks to avoid CA2012 & other cleanup (#63)
Browse files Browse the repository at this point in the history 
* Refactor async checks to avoid CA2012

Fixes #57

* Update package dependencies

* Rework invalid async handling & skip unreliable test
  • Loading branch information
@DamianEdwards
DamianEdwards authored Apr 21, 2024
1 parent 612ed4a commit 206ee70
Show file tree
Hide file tree
Showing 7 changed files with 199 additions and 137 deletions.
18 changes: 9 additions & 9 deletions Directory.Packages.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="PolySharp" Version="1.13.2" />
<PackageVersion Include="PolySharp" Version="1.14.1" />
<PackageVersion Include="System.ComponentModel.Annotations" Version="5.0.0" />
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="1.1.1" />
<PackageVersion Include="Swashbuckle.AspNetCore" Version="6.4.0" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.4.0" />
<PackageVersion Include="xunit" Version="2.4.2" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.4.5" />
<PackageVersion Include="coverlet.collector" Version="3.2.0" />
<PackageVersion Include="BenchmarkDotNet" Version="0.13.2" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="7.0.0" />
<PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
<PackageVersion Include="Swashbuckle.AspNetCore" Version="6.5.0" />
<PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.9.0" />
<PackageVersion Include="xunit" Version="2.7.1" />
<PackageVersion Include="xunit.runner.visualstudio" Version="2.5.8" />
<PackageVersion Include="coverlet.collector" Version="6.0.2" />
<PackageVersion Include="BenchmarkDotNet" Version="0.13.12" />
<PackageVersion Include="Microsoft.Extensions.DependencyInjection" Version="8.0.0" />
</ItemGroup>
</Project>
125 changes: 84 additions & 41 deletions src/MiniValidation/MiniValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,22 +183,34 @@ private static bool TryValidateImpl<TTarget>(TTarget target, IServiceProvider? s

bool isValid;

if (!validateTask.IsCompleted)
if (validateTask.IsCompleted)
{
isValid = validateTask.GetAwaiter().GetResult();
}
else
{
// This is a backstop check as TryValidateImpl and the methods it calls should all be doing this check as the object
// graph is walked during validation.
ThrowIfAsyncNotAllowed(validateTask, allowAsync);
try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
#if NET6_0_OR_GREATER
_ = validateTask.AsTask().GetAwaiter().GetResult();
#else
_ = validateTask.GetAwaiter().GetResult();
#endif
throw;
}

#if NET6_0_OR_GREATER
isValid = validateTask.AsTask().GetAwaiter().GetResult();
#else
isValid = validateTask.GetAwaiter().GetResult();
#endif
}
else
{
isValid = validateTask.GetAwaiter().GetResult();
}

errors = MapToFinalErrorsResult(workingErrors);

Expand Down Expand Up @@ -414,9 +426,20 @@ private static async Task<bool> TryValidateImpl(
if (target is IEnumerable)
{
RuntimeHelpers.EnsureSufficientExecutionStack();
var task = TryValidateEnumerable(target, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, prefix, currentDepth);
ThrowIfAsyncNotAllowed(task, allowAsync);
isValid = await task.ConfigureAwait(false) && isValid;

var validateTask = TryValidateEnumerable(target, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, prefix, currentDepth);

try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
_ = await validateTask.ConfigureAwait(false);
throw;
}

isValid = await validateTask.ConfigureAwait(false) && isValid;
}

// Validate complex properties
Expand All @@ -434,16 +457,36 @@ private static async Task<bool> TryValidateImpl(
if (propertyDetails.IsEnumerable)
{
var thePrefix = $"{prefix}{propertyDetails.Name}";
var task = TryValidateEnumerable(propertyValue, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, thePrefix, currentDepth);
ThrowIfAsyncNotAllowed(task, allowAsync);
isValid = await task.ConfigureAwait(false) && isValid;

var validateTask = TryValidateEnumerable(propertyValue, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, thePrefix, currentDepth);
try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
_ = await validateTask.ConfigureAwait(false);
throw;
}

isValid = await validateTask.ConfigureAwait(false) && isValid;
}
else
{
var thePrefix = $"{prefix}{propertyDetails.Name}."; // <-- Note trailing '.' here
var task = TryValidateImpl(propertyValue, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, thePrefix, currentDepth + 1);
ThrowIfAsyncNotAllowed(task, allowAsync);
isValid = await task.ConfigureAwait(false) && isValid;

var validateTask = TryValidateImpl(propertyValue, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, thePrefix, currentDepth + 1);
try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
await validateTask.ConfigureAwait(false);
throw;
}

isValid = await validateTask.ConfigureAwait(false) && isValid;
}
}
}
Expand Down Expand Up @@ -474,9 +517,18 @@ private static async Task<bool> TryValidateImpl(
validationContext.MemberName = null;
validationContext.DisplayName = validationContext.ObjectType.Name;

var task = validatable.ValidateAsync(validationContext);
ThrowIfAsyncNotAllowed(task, allowAsync);
var validatableResults = await task.ConfigureAwait(false);
var validateTask = validatable.ValidateAsync(validationContext);
try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
_ = await validateTask.ConfigureAwait(false);
throw;
}

var validatableResults = await validateTask.ConfigureAwait(false);
if (validatableResults is not null)
{
ProcessValidationResults(validatableResults, workingErrors, prefix);
Expand All @@ -495,27 +547,9 @@ static string GetDisplayName(PropertyDetails property)
}
}

#if NET6_0_OR_GREATER
private static void ThrowIfAsyncNotAllowed(ValueTask<bool> validateTask, bool allowAsync)
private static void ThrowIfAsyncNotAllowed(bool taskCompleted, bool allowAsync)
{
if (!validateTask.IsCompleted)
{
ThrowIfAsyncNotAllowed(allowAsync);
}
}
#endif

private static void ThrowIfAsyncNotAllowed(Task validateTask, bool allowAsync)
{
if (!validateTask.IsCompleted)
{
ThrowIfAsyncNotAllowed(allowAsync);
}
}

private static void ThrowIfAsyncNotAllowed(bool allowAsync)
{
if (!allowAsync)
if (!allowAsync & !taskCompleted)
{
throw new InvalidOperationException($"An object in the validation graph requires async validation. Call the '{nameof(TryValidateAsync)}' method instead.");
}
Expand Down Expand Up @@ -550,9 +584,18 @@ private static async Task<bool> TryValidateEnumerable(

var itemPrefix = $"{prefix}[{index}].";

var task = TryValidateImpl(item, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, itemPrefix, currentDepth + 1);
ThrowIfAsyncNotAllowed(task, allowAsync);
isValid = await task.ConfigureAwait(false);
var validateTask = TryValidateImpl(item, serviceProvider, recurse, allowAsync, workingErrors, validatedObjects, validationResults, itemPrefix, currentDepth + 1);
try
{
ThrowIfAsyncNotAllowed(validateTask.IsCompleted, allowAsync);
}
catch (Exception)
{
_ = await validateTask.ConfigureAwait(false);
throw;
}

isValid = await validateTask.ConfigureAwait(false);

if (!isValid)
{
Expand Down
25 changes: 25 additions & 0 deletions src/MiniValidation/NotNullWhenAttribute.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#if NETSTANDARD2_0
namespace System.Diagnostics.CodeAnalysis;

/// <summary>
/// Specifies that when a method returns <see cref="System.Diagnostics.CodeAnalysis.NotNullWhenAttribute.ReturnValue"/>,
/// the parameter will not be null even if the corresponding type allows it.
/// </summary>
[AttributeUsage(AttributeTargets.Parameter, Inherited = false)]
internal sealed class NotNullWhenAttribute : Attribute
{
/// <summary>
/// Initializes the attribute with the specified return value condition.
/// </summary>
/// <param name="returnValue">The return value condition. If the method returns this value, the associated parameter will not be null.</param>
public NotNullWhenAttribute(bool returnValue)
{
ReturnValue = returnValue;
}

/// <summary>
/// Gets the return value condition.
/// </summary>
public bool ReturnValue { get; }
}
#endif
16 changes: 2 additions & 14 deletions src/MiniValidation/TypeDetailsCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private void Visit(Type type, HashSet<Type> visited, ref bool requiresAsync)
return;
}

if (DoNotValidatePropertiesOf(type))
if (DoNotRecurseIntoPropertiesOf(type))
{
_cache[type] = (_emptyPropertyDetails, false);
return;
Expand Down Expand Up @@ -146,25 +146,13 @@ private void Visit(Type type, HashSet<Type> visited, ref bool requiresAsync)
_cache[type] = (propertiesToValidate?.ToArray() ?? _emptyPropertyDetails, requiresAsync);
}

private static bool DoNotValidatePropertiesOf(Type type) =>
private static bool DoNotRecurseIntoPropertiesOf(Type type) =>
type == typeof(object)
|| type.IsPrimitive
|| type.IsArray
|| type.IsPointer
|| type.IsEnum
|| type == typeof(string)
|| type == typeof(char)
|| type == typeof(bool)
|| type == typeof(byte)
|| type == typeof(sbyte)
|| type == typeof(short)
|| type == typeof(ushort)
|| type == typeof(int)
|| type == typeof(uint)
|| type == typeof(long)
|| type == typeof(ulong)
|| type == typeof(float)
|| type == typeof(double)
|| type == typeof(decimal)
|| type == typeof(DateTime)
|| type == typeof(DateTimeOffset)
Expand Down
Loading

0 comments on commit 206ee70

Please sign in to comment.