Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error checking around constructing and parsing generic grain types and clean up tests #8705

Merged
merged 1 commit into from Nov 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -86,7 +86,7 @@ public GrainType GetGrainType(GrainInterfaceType interfaceType, string prefix)

if (GenericGrainType.TryParse(result, out var genericGrainType) && !genericGrainType.IsConstructed)
{
result = genericGrainType.GrainType.GetConstructed(genericInterface.Value);
result = genericGrainType.GetConstructed(genericInterface);
}

return result;
Expand Down Expand Up @@ -149,7 +149,7 @@ public bool TryGetGrainType(GrainInterfaceType interfaceType, out GrainType resu
}
else
{
result = genericGrainType.GrainType.GetConstructed(genericInterface.Value);
result = genericGrainType.GetConstructed(genericInterface);
}
}
else
Expand Down
32 changes: 27 additions & 5 deletions src/Orleans.Core/IDs/GenericGrainInterfaceType.cs
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics.CodeAnalysis;
using Orleans.Serialization.TypeSystem;
using Orleans.Utilities;

Expand All @@ -14,16 +15,22 @@ namespace Orleans.Runtime
/// Initializes a new instance of the <see cref="GenericGrainInterfaceType"/> struct.
/// </summary>
/// <param name="value">The underlying grain interface type.</param>
private GenericGrainInterfaceType(GrainInterfaceType value)
private GenericGrainInterfaceType(GrainInterfaceType value, int arity)
{
Value = value;
Arity = arity;
}

/// <summary>
/// The underlying <see cref="GrainInterfaceType"/>
/// </summary>
public GrainInterfaceType Value { get; }

/// <summary>
/// The arity of the generic type.
/// </summary>
public int Arity { get; }

/// <summary>
/// Returns <see langword="true" /> if this instance contains concrete type parameters.
/// </summary>
Expand All @@ -34,9 +41,16 @@ private GenericGrainInterfaceType(GrainInterfaceType value)
/// </summary>
public static bool TryParse(GrainInterfaceType grainType, out GenericGrainInterfaceType result)
{
if (!grainType.IsDefault && TypeConverterExtensions.IsGenericType(grainType.Value))
if (grainType.IsDefault)
{
result = default;
return false;
}

var arity = TypeConverterExtensions.GetGenericTypeArity(grainType.Value);
if (arity > 0)
{
result = new GenericGrainInterfaceType(grainType);
result = new GenericGrainInterfaceType(grainType, arity);
return true;
}

Expand All @@ -50,16 +64,21 @@ public static bool TryParse(GrainInterfaceType grainType, out GenericGrainInterf
public GenericGrainInterfaceType GetGenericGrainType()
{
var generic = TypeConverterExtensions.GetDeconstructed(Value.Value);
return new GenericGrainInterfaceType(new GrainInterfaceType(generic));
return new GenericGrainInterfaceType(new GrainInterfaceType(generic), Arity);
}

/// <summary>
/// Returns a constructed version of this instance.
/// </summary>
public GenericGrainInterfaceType Construct(TypeConverter formatter, params Type[] typeArguments)
{
if (Arity != typeArguments.Length)
{
ThrowIncorrectArgumentLength(typeArguments);
}

var constructed = formatter.GetConstructed(this.Value.Value, typeArguments);
return new GenericGrainInterfaceType(new GrainInterfaceType(constructed));
return new GenericGrainInterfaceType(new GrainInterfaceType(constructed), Arity);
}

/// <summary>
Expand All @@ -71,5 +90,8 @@ public GenericGrainInterfaceType Construct(TypeConverter formatter, params Type[
/// Returns a UTF8 interpretation of the current instance.
/// </summary>
public override string ToString() => Value.ToString();

[DoesNotReturn]
private void ThrowIncorrectArgumentLength(Type[] typeArguments) => throw new ArgumentException($"Incorrect number of type arguments, {typeArguments.Length}, to construct a generic grain type with arity {Arity}.", nameof(typeArguments));
}
}
35 changes: 28 additions & 7 deletions src/Orleans.Core/IDs/GenericGrainType.cs
@@ -1,4 +1,6 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Orleans.Serialization.TypeSystem;
using Orleans.Utilities;

Expand All @@ -14,16 +16,23 @@ namespace Orleans.Runtime
/// Initializes a new instance of the <see cref="GenericGrainType"/> struct.
/// </summary>
/// <param name="grainType">The underlying grain type.</param>
private GenericGrainType(GrainType grainType)
/// <param name="arity">The generic arity of the grain type.</param>
private GenericGrainType(GrainType grainType, int arity)
{
GrainType = grainType;
Arity = arity;
}

/// <summary>
/// The underlying grain type.
/// </summary>
public GrainType GrainType { get; }

/// <summary>
/// The generic arity of the grain type.
/// </summary>
public int Arity { get; }

/// <summary>
/// Returns <see langword="true" /> if this instance contains concrete type parameters.
/// </summary>
Expand All @@ -34,9 +43,10 @@ private GenericGrainType(GrainType grainType)
/// </summary>
public static bool TryParse(GrainType grainType, out GenericGrainType result)
{
if (TypeConverterExtensions.IsGenericType(grainType.Value))
var arity = TypeConverterExtensions.GetGenericTypeArity(grainType.Value);
if (arity > 0)
{
result = new GenericGrainType(grainType);
result = new GenericGrainType(grainType, arity);
return true;
}

Expand All @@ -50,21 +60,29 @@ public static bool TryParse(GrainType grainType, out GenericGrainType result)
public GenericGrainType GetUnconstructedGrainType()
{
var generic = TypeConverterExtensions.GetDeconstructed(GrainType.Value);
return new GenericGrainType(new GrainType(generic));
return new GenericGrainType(new GrainType(generic), Arity);
}

/// <summary>
/// Returns a constructed version of this instance.
/// </summary>
public GenericGrainType Construct(TypeConverter formatter, params Type[] typeArguments)
{
if (Arity != typeArguments.Length)
{
ThrowIncorrectArgumentLength(typeArguments);
}

var constructed = formatter.GetConstructed(this.GrainType.Value, typeArguments);
return new GenericGrainType(new GrainType(constructed));
return new GenericGrainType(new GrainType(constructed), Arity);
}

/// <summary>
/// Returns the type arguments which this instance was constructed with.
/// Gets the type arguments using the provided type converter.
/// </summary>
public Type[] GetArguments(TypeConverter formatter) => formatter.GetArguments(this.GrainType.Value);
/// <param name="converter">The type converter</param>
/// <returns>The type arguments.</returns>
public Type[] GetArguments(TypeConverter converter) => converter.GetArguments(this.GrainType.Value);

/// <inheritdoc/>
public override string ToString() => this.GrainType.ToString();
Expand All @@ -77,5 +95,8 @@ public GenericGrainType Construct(TypeConverter formatter, params Type[] typeArg

/// <inheritdoc/>
public override int GetHashCode() => this.GrainType.GetHashCode();

[DoesNotReturn]
private void ThrowIncorrectArgumentLength(Type[] typeArguments) => throw new ArgumentException($"Incorrect number of type arguments, {typeArguments.Length}, to construct a generic grain type with arity {Arity}.", nameof(typeArguments));
}
}
3 changes: 2 additions & 1 deletion src/Orleans.Core/Manifest/GrainTypeResolver.cs
Expand Up @@ -95,7 +95,8 @@ private GrainType AddGenericParameters(GrainType grainType, Type type)
&& !type.ContainsGenericParameters
&& !genericGrainType.IsConstructed)
{
grainType = genericGrainType.Construct(_typeConverter, type.GetGenericArguments()).GrainType;
var typeArguments = type.GetGenericArguments();
grainType = genericGrainType.Construct(_typeConverter, typeArguments).GrainType;
}

return grainType;
Expand Down
44 changes: 43 additions & 1 deletion src/Orleans.Core/Utils/TypeConverterExtensions.cs
@@ -1,5 +1,6 @@
using System;
using System.Buffers.Text;
using System.Diagnostics.CodeAnalysis;
using System.Text;
using Orleans.Runtime;
using Orleans.Serialization.TypeSystem;
Expand All @@ -19,6 +20,36 @@ internal static class TypeConverterExtensions
/// </summary>
public static bool IsGenericType(IdSpan type) => type.AsSpan().IndexOf((byte)GenericTypeIndicator) >= 0;

/// <summary>
/// Returns the generic arity of the specified grain type.
/// </summary>
public static int GetGenericTypeArity(IdSpan type)
{
var typeSpan = type.AsSpan();
var startIndex = typeSpan.IndexOf((byte)GenericTypeIndicator) + 1;
if (startIndex <= 0 || startIndex >= typeSpan.Length)
{
return 0;
}

int endIndex;
for (endIndex = startIndex; endIndex < typeSpan.Length; endIndex++)
{
var c = typeSpan[endIndex];
if (c is < ((byte)'0') or > ((byte)'9'))
{
break;
}
}

if (endIndex > startIndex && Utf8Parser.TryParse(typeSpan[startIndex..endIndex], out int arity, out _))
{
return arity;
}

throw new InvalidOperationException($"Unable to parse arity from type \"{type}\"");
}

/// <summary>
/// Returns true if the provided type string is a constructed generic type.
/// </summary>
Expand Down Expand Up @@ -65,8 +96,15 @@ public static IdSpan GetConstructed(this TypeConverter formatter, IdSpan unconst
/// <summary>
/// Returns the constructed form of the provided generic grain type using the type arguments from the provided constructed interface type.
/// </summary>
public static GrainType GetConstructed(this GrainType grainType, GrainInterfaceType typeArguments)
public static GrainType GetConstructed(this GenericGrainType genericGrainType, GenericGrainInterfaceType genericGrainInterfaceType)
{
if (genericGrainType.Arity != genericGrainInterfaceType.Arity)
{
ThrowGenericArityMismatch(genericGrainType, genericGrainInterfaceType);
}

var grainType = genericGrainType.GrainType;
var typeArguments = genericGrainInterfaceType.Value;
var args = typeArguments.Value.AsSpan();
var index = args.IndexOf((byte)StartArgument);
if (index <= 0) return grainType; // if no type arguments are provided, then the current logic expects the unconstructed form (but the grain call is going to fail later anyway...)
Expand Down Expand Up @@ -112,5 +150,9 @@ public static Type[] GetArguments(this TypeConverter formatter, IdSpan construct

return result;
}

[DoesNotReturn]
private static void ThrowGenericArityMismatch(GenericGrainType genericGrainType, GenericGrainInterfaceType genericInterfaceType)
=> throw new ArgumentException($"Cannot construct generic grain \"{genericGrainType.GrainType}\" using arguments from generic interface \"{genericInterfaceType}\" because the generic arities are not equal: {genericGrainType.Arity} is not equal to {genericInterfaceType.Arity}.");
}
}