Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions src/Core/Infrastructure/DefaultValues.cs
Original file line number Diff line number Diff line change
Expand Up @@ -105,16 +105,48 @@ public class DefaultValues
return null;
}

// Try exact type first
if (!_componentCache.TryGetValue(componentType, out var properties))
var inheritanceChain = new List<Type>();

for (var currentType = componentType; currentType is not null && currentType != typeof(object); currentType = currentType.BaseType)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's going to be a performance issue here: you can't keep recalculating the BaseType parents all the time. You need to cache the type just once and then retrieve it without having to run through that loop again.

The logic must be different

{
inheritanceChain.Add(currentType);

if (currentType == typeof(FluentComponentBase))
{
break;
}
}

ConcurrentDictionary<string, object?>? mergedProperties = null;

for (var i = inheritanceChain.Count - 1; i >= 0; i--)
{
// Fallback: check open generic type definition
if (componentType.IsGenericType)
if (TryGetRegisteredProperties(inheritanceChain[i], out var properties))
{
_componentCache.TryGetValue(componentType.GetGenericTypeDefinition(), out properties);
mergedProperties ??= new ConcurrentDictionary<string, object?>(StringComparer.Ordinal);
foreach (var property in properties)
{
mergedProperties[property.Key] = property.Value;
}
}
}

return properties;
return mergedProperties;
}

private bool TryGetRegisteredProperties(Type componentType, [NotNullWhen(true)] out ConcurrentDictionary<string, object?>? properties)
{
if (_componentCache.TryGetValue(componentType, out properties))
{
return true;
}

if (componentType.IsGenericType)
{
return _componentCache.TryGetValue(componentType.GetGenericTypeDefinition(), out properties);
}

properties = null;
return false;
}
}
35 changes: 35 additions & 0 deletions src/Core/Infrastructure/DefaultValuesComponentBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ public void Set<TValue>(Expression<Func<TComponent, TValue>> parameterSelector,
var propertyName = propertyInfo?.Name
?? throw new ArgumentException($"The parameter selector '{parameterSelector}' does not resolve to a public property on the component '{typeof(TComponent)}'.", nameof(parameterSelector));

if (!propertyInfo.CanWrite)
{
throw new ArgumentException($"The property '{propertyName}' on component '{typeof(TComponent)}' is read-only and cannot be used for default values.", nameof(parameterSelector));
}

ValidateValueCompatibility(propertyInfo, value, propertyName);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to check the value's type? It takes time, especially when using reflection, and I don't really see the advantage: without this check, the code will crash anyway, right? Keep in mind that these methods are executed thousands of times since they're in the constructor of every component; they therefore need to be very efficie


// Add the value to the dictionary, using the property name as the key.
_values.AddOrUpdate(propertyName, AddFunction, UpdateFunction);

Expand All @@ -68,4 +75,32 @@ public void Set<TValue>(Expression<Func<TComponent, TValue>> parameterSelector,
return value;
}
}

private static void ValidateValueCompatibility(PropertyInfo propertyInfo, object? value, string propertyName)
{
var propertyType = propertyInfo.PropertyType;
var nullableUnderlyingType = Nullable.GetUnderlyingType(propertyType);
var targetType = nullableUnderlyingType ?? propertyType;

if (value is null)
{
if (propertyType.IsValueType && nullableUnderlyingType is null)
{
throw new ArgumentException($"Default value for '{propertyName}' on component '{typeof(TComponent)}' cannot be null because the property type '{propertyType}' is non-nullable.", nameof(value));
}

var nullabilityInfo = new NullabilityInfoContext().Create(propertyInfo);
if (nullabilityInfo.WriteState == NullabilityState.NotNull)
{
throw new ArgumentException($"Default value for '{propertyName}' on component '{typeof(TComponent)}' cannot be null because the property is non-nullable.", nameof(value));
}

return;
}
Comment thread
vnbaaij marked this conversation as resolved.

if (!targetType.IsInstanceOfType(value))
{
throw new ArgumentException($"Default value for '{propertyName}' on component '{typeof(TComponent)}' must be assignable to '{targetType}', but was '{value.GetType()}'.", nameof(value));
}
}
}
Loading