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

serializer: only inject properties to constructors of records #1525

Merged
merged 14 commits into from
Dec 10, 2022
Merged
Show file tree
Hide file tree
Changes from 13 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
4 changes: 4 additions & 0 deletions src/Framework/Framework/Utils/ReflectionUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -569,5 +569,9 @@ internal static void ClearCaches(Type[] types)
cache_GetTypeHash.TryRemove(t, out _);
}
}

internal static bool IsInitOnly(this PropertyInfo prop) =>
prop.SetMethod is { ReturnParameter: {} returnParameter } &&
returnParameter.GetRequiredCustomModifiers().Any(t => t == typeof(System.Runtime.CompilerServices.IsExternalInit));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void ResetFunctions()
/// <summary>
/// Creates the constructor for this object.
/// </summary>
private Expression CallConstructor(Expression services, Dictionary<PropertyInfo, ParameterExpression> propertyVariables)
private Expression CallConstructor(Expression services, Dictionary<PropertyInfo, ParameterExpression> propertyVariables, bool throwImmediately = false)
{
if (constructorFactory != null)
return Convert(Invoke(Constant(constructorFactory), services), Type);
Expand All @@ -82,10 +82,10 @@ private Expression CallConstructor(Expression services, Dictionary<PropertyInfo,
}

if (Constructor is null && (Type.IsInterface || Type.IsAbstract))
throw new Exception($"Can not deserialize {Type.ToCode()} because it's abstract. Please avoid using abstract types in view model. If you really mean it, you can add a static factory method and mark it with [JsonConstructor] attribute.");
return jitException($"Can not deserialize {Type.ToCode()} because it's abstract. Please avoid using abstract types in view model. If you really mean it, you can add a static factory method and mark it with [JsonConstructor] attribute.");

if (Constructor is null)
throw new Exception($"Can not deserialize {Type.ToCode()}, no constructor or multiple constructors found. Use the [JsonConstructor] attribute to specify the constructor used for deserialization.");
return jitException($"Can not deserialize {Type.ToCode()}, no parameterless constructor found. Use the [JsonConstructor] attribute to specify the constructor used for deserialization.");

var parameters = Constructor.GetParameters().Select(p => {
var prop = Properties.FirstOrDefault(pp => pp.ConstructorParameter == p);
Expand Down Expand Up @@ -116,6 +116,16 @@ private Expression CallConstructor(Expression services, Dictionary<PropertyInfo,
Call(m, parameters),
_ => throw new NotSupportedException()
};

// Since the old serializer didn't care about constructor problems until it was actually needed,
// we can't throw exception during compilation, so we wait until this code will run.
Expression jitException(string message) =>
throwImmediately
? throw new Exception(message)
: Expression.Throw(Expression.New(
typeof(Exception).GetConstructor(new [] { typeof(string) })!,
Expression.Constant(message)
), this.Type);
}

/// <summary>
Expand All @@ -141,14 +151,28 @@ public ReaderDelegate CreateReaderFactory()
p => Expression.Variable(p.Type, "prop_" + p.Name)
);

var hasConstructorProperties = Properties.Any(p => p.ConstructorParameter is {});
var constructorCall = CallConstructor(servicesParameter, propertyVars);
// If we have constructor property or if we have { get; init; } property, we always create new instance
var alwaysCallConstructor = Properties.Any(p => p.TransferToServer && (
p.ConstructorParameter is {} ||
p.PropertyInfo.IsInitOnly()));

// We don't want to clone IDotvvmViewModel automatically, because the user is likely to register this specific instance somewhere
if (alwaysCallConstructor && typeof(IDotvvmViewModel).IsAssignableFrom(Type) && Constructor is {} && !Constructor.IsDefined(typeof(JsonConstructorAttribute)))
{
var cloneReason =
Properties.FirstOrDefault(p => p.TransferToServer && p.PropertyInfo.IsInitOnly()) is {} initProperty
? $"init-only property {initProperty.Name} is transferred client → server" :
Properties.FirstOrDefault(p => p.TransferToServer && p.ConstructorParameter is {}) is {} ctorProperty
? $"property {ctorProperty.Name} must be injected into constructor parameter {ctorProperty.ConstructorParameter!.Name}" : "internal bug";
throw new Exception($"Deserialization of {Type.ToCode()} is not allowed, because it implements IDotvvmViewModel and {cloneReason}. To allow cloning the object on deserialization, mark a constructor with [JsonConstructor].");
}
var constructorCall = CallConstructor(servicesParameter, propertyVars, throwImmediately: alwaysCallConstructor);

// curly brackets are used for variables and methods from the context of this factory method
// value = ({Type})valueParam;
block.Add(Assign(value,
Condition(Equal(valueParam, Constant(null)),
hasConstructorProperties
alwaysCallConstructor
? Default(Type)
: constructorCall,
Convert(valueParam, Type)
Expand All @@ -170,7 +194,6 @@ public ReaderDelegate CreateReaderFactory()
// add current object to encrypted values, this is needed because one property can potentially contain more objects (is a collection)
block.Add(Expression.Call(encryptedValuesReader, nameof(EncryptedValuesReader.Nest), Type.EmptyTypes));


// if the reader is in an invalid state, throw an exception
// TODO: Change exception type, just Exception is not exactly descriptive
block.Add(ExpressionUtils.Replace((JsonReader rdr) => rdr.TokenType == JsonToken.StartObject ? rdr.Read() : ExpressionUtils.Stub.Throw<bool>(new Exception($"TokenType = StartObject was expected.")), reader));
Expand Down Expand Up @@ -323,7 +346,7 @@ public ReaderDelegate CreateReaderFactory()
block.Add(Expression.Call(encryptedValuesReader, nameof(EncryptedValuesReader.AssertEnd), Type.EmptyTypes));

// call the constructor
if (hasConstructorProperties)
if (alwaysCallConstructor)
{
block.Add(Assign(value, constructorCall));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,26 +41,60 @@ public ViewModelSerializationMapper(IValidationRuleTranslator validationRuleTran
/// </summary>
protected virtual ViewModelSerializationMap CreateMap(Type type)
{
var constructor = GetConstructor(type);
return new ViewModelSerializationMap(type, GetProperties(type, constructor), constructor, configuration);
// constructor which takes properties as parameters
// if it exists, we always need to recreate the viewmodel
var valueConstructor = GetConstructor(type);
return new ViewModelSerializationMap(type, GetProperties(type, valueConstructor), valueConstructor, configuration);
}

protected virtual MethodBase? GetConstructor(Type type)
{
if (ReflectionUtils.IsPrimitiveType(type) || ReflectionUtils.IsEnumerable(type))
return null;

if (type.GetConstructors().FirstOrDefault(c => c.IsDefined(typeof(JsonConstructorAttribute))) is {} ctor)
return ctor;
if (type.GetMethods(BindingFlags.Static | BindingFlags.Public).FirstOrDefault(c => c.IsDefined(typeof(JsonConstructorAttribute))) is {} factory)
return factory;

if (type.IsAbstract)
return null;

if (type.GetConstructors().FirstOrDefault(c => c.IsDefined(typeof(JsonConstructorAttribute))) is {} ctor)
return ctor;

if (type.GetConstructor(Type.EmptyTypes) is {} emptyCtor)
return emptyCtor;
var constructors = type.GetConstructors();
return GetRecordConstructor(type);
}

protected static MethodBase? GetRecordConstructor(Type t)
{
if (t.IsAbstract)
return null;
// https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AbEAzAzmgFxCgFcA7AHwgAcYyACAZQE9cCYBbAWACga6mrdhwB0AGQCWZAI68CzWvQDC9ALz0A3vQBCIelIIBuZXoPGwpsgXoBfOQpj0AqmvoANehXoBNehGz6Vry81FAG2AwARACCkcE8GDDW1urytP4APEoAfPGJ1gCGBARQrgQiAOJJSiRsEBzRxWHAJOy4ACJFBQAUAJQi0WTM3djk9AX0cNnjA00SLewAKg4iAGIkGBgAcgUcjuqRALISYFAQuP7lq4wAFgVQ1CJK0DBP9dQSGEUSEGSHBdQPmQAOaNErzVowSL0ABkMOUvwAbjAoOVFhAAJJWADMACZugU3mQ2KQwARoNEoMCSHsrLgANoABgAuiIAGoFDAkGC9Vy43ohMJWCL0SJFEp6ACksXGTSAA=
// F# record or single case discriminated union (multi case is abstract)
if (t.GetCustomAttributesData().Any(a => a.AttributeType.FullName == "Microsoft.FSharp.Core.CompilationMappingAttribute" && Convert.ToInt32(a.ConstructorArguments[0].Value) is 1 or 2))
{
return t.GetConstructors().Single();
}
// TODO: F# normal type, it's not possible AFAIK to add attribute to the default constructor

// find constructor which matches Deconstruct method
var deconstruct = t.GetMethods(BindingFlags.Instance | BindingFlags.Public).Where(m => m.Name == "Deconstruct").ToArray();
if (deconstruct.Length == 0)
return null;
var constructors =
(from c in t.GetConstructors()
from d in deconstruct
where c.GetParameters().Select(p => p.Name).SequenceEqual(d.GetParameters().Select(p => p.Name)) &&
c.GetParameters().Select(p => unwrapByRef(p.ParameterType)).SequenceEqual(d.GetParameters().Select(p => unwrapByRef(p.ParameterType)))
select c).ToArray();

if (constructors.Length == 1)
return constructors[0];

return null;

static Type unwrapByRef(Type t) => t.IsByRef ? t.GetElementType()! : t;
}

/// <summary>
Expand Down
2 changes: 2 additions & 0 deletions src/Samples/AspNetCore/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using DotVVM.Framework.Routing;
using DotVVM.Samples.BasicSamples.ViewModels.ComplexSamples.Auth;
using DotVVM.Samples.BasicSamples.ViewModels.FeatureSamples.StaticCommand;
using DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelDependencyInjection;
using DotVVM.Samples.Common.ViewModels.FeatureSamples.DependencyInjection;
using Microsoft.AspNetCore.Authentication.Cookies;
using Microsoft.AspNetCore.Builder;
Expand Down Expand Up @@ -66,6 +67,7 @@ public void ConfigureServices(IServiceCollection services)
services.AddSingleton<IGreetingComputationService, HelloGreetingComputationService>();

services.AddScoped<ViewModelScopedDependency>();
services.AddTransient<ChildViewModel>();
}

public void Configure(IApplicationBuilder app, IWebHostEnvironment env, ILoggerFactory loggerFactory)
Expand Down
3 changes: 3 additions & 0 deletions src/Samples/Common/CommonConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using DotVVM.Samples.Common.Api.Owin;
using DotVVM.Samples.Common.Resources;
using DotVVM.Samples.Common.Utilities;
using DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelDependencyInjection;
using DotVVM.Samples.Common.ViewModels.FeatureSamples.BindingVariables;
using DotVVM.Samples.Common.ViewModels.FeatureSamples.DependencyInjection;
using DotVVM.Samples.Common.ViewModels.FeatureSamples.AutoUI;
Expand Down Expand Up @@ -78,6 +79,8 @@ public static void ConfigureServices(IDotvvmServiceCollection dotvvmServices)
services.AddTransient<ISelectionProvider<ProductSelection>, ProductSelectionProvider>();
services.AddTransient<ISelectionProvider<CountrySelection>, CountrySelectionProvider>();
services.AddTransient<ISelectionProvider<StateSelection, AddressDTO>, StateSelectorDataProvider>();

services.AddTransient<ChildViewModel>();
}

private static void RegisterResources(DotvvmResourceRepository resources)
Expand Down
1 change: 1 addition & 0 deletions src/Samples/Common/DotVVM.Samples.Common.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
<None Remove="Views\ComplexSamples\SPAErrorReporting\default.dothtml" />
<None Remove="Views\ComplexSamples\SPAErrorReporting\site.dotmaster" />
<None Remove="Views\ComplexSamples\SPAErrorReporting\test.dothtml" />
<None Remove="Views\ComplexSamples\ViewModelPopulate\ViewModelPopulate.dothtml" />
<None Remove="Views\ControlSamples\CheckBox\CheckBox_Objects.dothtml" />
<None Remove="Views\ControlSamples\ComboBox\BindingCTValidation_StringToEnum.dothtml" />
<None Remove="Views\ControlSamples\ComboBox\HeavilyNested.dothtml" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System;
using System.Threading.Tasks;
using DotVVM.Framework.ViewModel;

namespace DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelDependencyInjection;

public class ChildViewModel : DotvvmViewModelBase
{
public ChildViewModel()
{
}

public override Task Init()
{
if (Context is null)
{
throw new Exception($"{nameof(Context)} is null in {nameof(Init)} method of {nameof(ParentViewModel)}.");
}

return base.Init();
}

public override Task Load()
{
if (Context is null)
{
throw new Exception($"{nameof(Context)} is null in {nameof(Load)} method of {nameof(ParentViewModel)}.");
}

return base.Load();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System;
using System.Threading.Tasks;
using DotVVM.Framework.ViewModel;

namespace DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelDependencyInjection;

public class ParentViewModel : DotvvmViewModelBase
{
public ChildViewModel ChildViewModel { get; set; }
public bool Result { get; set; }


public ParentViewModel(ChildViewModel childViewModel)
{
this.ChildViewModel = childViewModel;
}

public override Task Init()
{
if (Context is null)
{
throw new Exception($"{nameof(Context)} is null in {nameof(Init)} method of {nameof(ParentViewModel)}.");
}

return base.Init();
}

public override Task Load()
{
if (Context is null)
{
throw new Exception($"{nameof(Context)} is null in {nameof(Load)} method of {nameof(ParentViewModel)}.");
}

return base.Load();
}

public void DoSomething()
{
Result = true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using DotVVM.Framework.ViewModel;

namespace DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelPopulate
{
public class ViewModelPopulateViewModel : DotvvmViewModelBase
{

public NonDeserializableObject NonDeserializableObject { get; set; } = new(1, "");


public void DoSomething()
{
NonDeserializableObject.Test = NonDeserializableObject.Test + "success";
}
}

public class NonDeserializableObject
{

public string Test { get; set; }

public NonDeserializableObject(int nonPropertyField, string test)
{

}
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
@viewModel DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelDependencyInjection.ParentViewModel, DotVVM.Samples.Common

<!DOCTYPE html>

<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta charset="utf-8" />
<title>ViewModel Dependency Injection</title>
</head>
<body>
<p>
<dot:LinkButton Text="Call command"
Click="{command: DoSomething()}"
data-ui="command" />
</p>
<p class="result">{{value: Result}}</p>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
@viewModel DotVVM.Samples.Common.ViewModels.ComplexSamples.ViewModelPopulate.ViewModelPopulateViewModel, DotVVM.Samples.Common

<!DOCTYPE html>

<html lang="en" xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta charset="utf-8" />
<title></title>
</head>
<body>

<dot:TextBox Text="{value: NonDeserializableObject.Test}" />

<dot:Button Text="Test" Click="{command: DoSomething()}" />

</body>
</html>


2 changes: 2 additions & 0 deletions src/Samples/Tests/Abstractions/SamplesRouteUrls.designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using DotVVM.Samples.Tests.Base;
using DotVVM.Testing.Abstractions;
using Riganti.Selenium.Core;
using Riganti.Selenium.DotVVM;
using Xunit;

namespace DotVVM.Samples.Tests.Complex
{
public class ViewModelDependencyInjectionTests : AppSeleniumTest
{
public ViewModelDependencyInjectionTests(Xunit.Abstractions.ITestOutputHelper output) : base(output)
{
}

[Fact]
public void Complex_ViewModelDependencyInjection_Sample()
{
RunInAllBrowsers(browser => {
browser.NavigateToUrl(SamplesRouteUrls.ComplexSamples_ViewModelDependencyInjection_Sample);

browser.Single("a").Click();
AssertUI.TextEquals(browser.Single(".result"), "true");
});
}
}
}
Loading