Skip to content

Commit

Permalink
capabilities: avoid NRE when ValueOrBinding<int>? is set to null
Browse files Browse the repository at this point in the history
Nullable ValueOrBinding, for example ValueOrBinding<int>?
semantially means that the property is not set.
If the property is set to null, we would like to return null inside
of the ValueOrBinding.
However, that is not possible if T is non-nullable value type, so
either have to throw, or return an "outer" null. This patch
changes the behavior to return null instead of failing. Users
are unlikely to care about "not set" and "set to null", and
throwing the exception creates unnecessary friction.
  • Loading branch information
exyi committed Sep 20, 2024
1 parent 2bbf954 commit c94f5bc
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,14 @@ internal static class Helpers
public static ValueOrBinding<T>? GetOptionalValueOrBinding<T>(DotvvmBindableObject c, DotvvmProperty p)
{
if (c.properties.TryGet(p, out var x))
return ValueOrBinding<T>.FromBoxedValue(x);
{
// we want to return ValueOrBinding(null) -- "property set to null"
// but if that isn't possible, it's better to return null ("property missing") than crash
if (x is null && default(T) != null)
return null;
else
return ValueOrBinding<T>.FromBoxedValue(x);
}
else return null;
}
public static ValueOrBinding<T> GetValueOrBinding<T>(DotvvmBindableObject c, DotvvmProperty p)
Expand All @@ -27,7 +34,15 @@ public static ValueOrBinding<T> GetValueOrBinding<T>(DotvvmBindableObject c, Dot
public static ValueOrBinding<T>? GetOptionalValueOrBindingSlow<T>(DotvvmBindableObject c, DotvvmProperty p)
{
if (c.IsPropertySet(p))
return ValueOrBinding<T>.FromBoxedValue(c.GetValue(p));
{
var x = c.GetValue(p);
// we want to return ValueOrBinding(null) -- "property set to null"
// but if that isn't possible, it's better to return null ("property missing") than crash
if (x is null && default(T) != null)
return null;
else
return ValueOrBinding<T>.FromBoxedValue(x);
}
else return null;
}
public static ValueOrBinding<T> GetValueOrBindingSlow<T>(DotvvmBindableObject c, DotvvmProperty p)
Expand Down
3 changes: 2 additions & 1 deletion src/Framework/Framework/Controls/CompositeControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,8 @@ Func<IDotvvmRequestContext, CompositeControl, object> compileGetter(IControlAttr
}

static internal ControlInfo GetControlInfo(Type controlType) =>
controlInfoCache[controlType];
controlInfoCache.TryGetValue(controlType, out var result) ? result :
throw new Exception($"CompositeControl {controlType.FullName} wasn't initialized and GetContents cannot be executed. This is probably caused by a failure during property registration.");

internal IEnumerable<DotvvmControl> ExecuteGetContents(IDotvvmRequestContext context)
{
Expand Down
52 changes: 52 additions & 0 deletions src/Tests/Runtime/CapabilityPropertyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,33 @@ public void BitMoreComplexCapability_WeirdProperties_GetterAndSetter()
}


[DataTestMethod]
[DataRow(typeof(TestControl6))]
[DataRow(typeof(TestControlFallbackProps))]
[DataRow(typeof(TestControlInheritedProps))]
public void BitMoreComplexCapability_NullableValue(Type controlType)
{
var control1 = (DotvvmBindableObject)Activator.CreateInstance(controlType);
var capProp = DotvvmCapabilityProperty.Find(controlType, typeof(BitMoreComplexCapability));

Assert.IsFalse(control1.GetCapability<BitMoreComplexCapability>().Nullable.HasValue);

control1.SetProperty("Nullable", null);
XAssert.Single(control1.Properties);
Assert.IsFalse(control1.GetCapability<BitMoreComplexCapability>().Nullable.HasValue);

control1.SetProperty("Nullable", 1);
Assert.AreEqual(1, control1.GetCapability<BitMoreComplexCapability>().Nullable);

control1.SetValue(capProp, new BitMoreComplexCapability { Nullable = null });
Assert.AreEqual(null, control1.GetCapability<BitMoreComplexCapability>().Nullable);
Assert.IsTrue(control1.IsPropertySet(control1.GetDotvvmProperty("Nullable")));

control1.SetValue(capProp, new BitMoreComplexCapability { Nullable = 2 });
Assert.AreEqual(2, control1.GetCapability<BitMoreComplexCapability>().Nullable);
}


public class TestControl1:
HtmlGenericControl,
IObjectWithCapability<HtmlCapability>,
Expand Down Expand Up @@ -301,6 +328,7 @@ public int ValueOrBindingNullable2
}
public static readonly DotvvmProperty ValueOrBindingNullable2Property =
DotvvmProperty.Register<int, TestControlFallbackProps>(nameof(ValueOrBindingNullable2), defaultValue: 10);

[PropertyAlias("ValueOrBindingNullable2")]
public static readonly DotvvmProperty ValueOrBindingNullableProperty =
DotvvmPropertyAlias.RegisterAlias<TestControlFallbackProps>("ValueOrBindingNullable");
Expand All @@ -309,6 +337,30 @@ public int ValueOrBindingNullable2
DotvvmCapabilityProperty.RegisterCapability<BitMoreComplexCapability, TestControlFallbackProps>();
}

public class TestControlInheritedProps:
HtmlGenericControl,
IObjectWithCapability<BitMoreComplexCapability>
{
public int NotNullable
{
get { return (int)GetValue(NotNullableProperty); }
set { SetValue(NotNullableProperty, value); }
}
public static readonly DotvvmProperty NotNullableProperty =
DotvvmProperty.Register<int, TestControlInheritedProps>(nameof(NotNullable), isValueInherited: true);

public int? Nullable
{
get { return (int?)GetValue(NullableProperty); }
set { SetValue(NullableProperty, value); }
}
public static readonly DotvvmProperty NullableProperty =
DotvvmProperty.Register<int?, TestControlInheritedProps>(nameof(Nullable), isValueInherited: true);

public static readonly DotvvmCapabilityProperty BitMoreComplexCapabilityProperty =
DotvvmCapabilityProperty.RegisterCapability<BitMoreComplexCapability, TestControlInheritedProps>();
}

[DotvvmControlCapability]
public sealed record TestCapability
{
Expand Down

0 comments on commit c94f5bc

Please sign in to comment.