Skip to content

Commit

Permalink
CompositeControl: better handling of ambiguous GetContents, test for …
Browse files Browse the repository at this point in the history
…inheritance

According to  #1613 CompositeControl do not work with
both base and derived types defining GetContents method.
I could not replicate that, the provided example works for me,
maybe it was fixed accidentaly.

* I added test for that
* In any case, multiple GetContents overloads were handled poorly,
   I added better error for this case.

In the issue, I mentioned that it would be complex to handle
potential redefinition or "removal" of properties in the derived type.
That isn't really the case, both cases don't need any special code
* The "removal" - omission of an property in the derived type is simply ignored.
   I think that it's quite intuitive that properties can't disapear in
   derived types, if the GetContents doesn't ask for it it won't get it.
* Redefinition is already handled reasonably, since we can already
   try to redefine IncludeInPage or some capability property.

Resolves #1613
  • Loading branch information
exyi committed Oct 18, 2023
1 parent 6d1a350 commit f21c2cf
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 1 deletion.
13 changes: 12 additions & 1 deletion src/Framework/Framework/Controls/CompositeControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,18 @@ Func<IDotvvmRequestContext, CompositeControl, object> compileGetter(IControlAttr

DefaultControlResolver.InitType(controlType.BaseType.NotNull());

var method = controlType.GetMethod("GetContents", BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
MethodInfo? method;
try
{
method = controlType.GetMethod("GetContents", BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic);
}
catch (AmbiguousMatchException ex)
{
var allMethods = controlType.GetMethods(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.FlattenHierarchy).Where(m => m.Name == "GetContents").ToArray();

var message = $"Could not initialize control {controlType.FullName}, multiple GetContents methods are present: {allMethods.Take(3).Select(m => ReflectionUtils.FormatMethodInfo(m, stripNamespace: true)).StringJoin(", ")}{(allMethods.Length > 3 ? ", ..." : "")}";
throw new Exception(message, ex);
}
if (method == null)
throw new Exception($"Could not initialize control {controlType.FullName}, could not find (single) GetContents method");
if (!(typeof(DotvvmControl).IsAssignableFrom(method.ReturnType) || typeof(IEnumerable<DotvvmControl>).IsAssignableFrom(method.ReturnType)))
Expand Down
44 changes: 44 additions & 0 deletions src/Tests/ControlTests/CompositeControlTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using DotVVM.Framework.ViewModel;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using DotVVM.Framework.Hosting;
using System.ComponentModel;

namespace DotVVM.Framework.Tests.ControlTests
{
Expand Down Expand Up @@ -322,6 +323,18 @@ public async Task MenuRepeater()
check.CheckString(r.FormattedHtml, fileExtension: "html");
}

[TestMethod]
public async Task CompositeControlInheritance()
{
var r = await cth.RunPage(typeof(BasicTestViewModel), """
<cc:TestBaseCompositeControl Text="Text" Number=1 />
<cc:TestDerivedCompositeControl Text="SecondText" AnotherNumber=1.1 />
"""
);

check.CheckString(r.FormattedHtml, fileExtension: "html");
}

public class BasicTestViewModel: DotvvmViewModelBase
{
[Bind(Name = "int")]
Expand Down Expand Up @@ -689,4 +702,35 @@ public static DotvvmControl GetContents(ValueOrBinding<TestStatusEnum> testStatu
return icon;
}
}

public class TestBaseCompositeControl: CompositeControl
{
public static DotvvmControl GetContents(
ValueOrBinding<string> text,
[DefaultValue(1)]
ValueOrBinding<int> number
)
{
return new HtmlGenericControl("div") {
Children = {
new Literal(text), new Literal(number)
}
};
}
}

public class TestDerivedCompositeControl: TestBaseCompositeControl
{
public static DotvvmControl GetContents(
ValueOrBinding<string> text,
ValueOrBinding<double> anotherNumber // declare another property, type of existing property cannot be changed
)
{
return new HtmlGenericControl("p") {
Children = {
new Literal(text), new Literal(anotherNumber)
}
};
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<html>
<head></head>
<body>
<div>Text1</div>
<p>SecondText1.1</p>
</body>
</html>

0 comments on commit f21c2cf

Please sign in to comment.