Merge pull request #1525 from riganti/serializer-nerf-records

serializer: only inject properties to constructors of records

src/Framework/Framework/Utils/ReflectionUtils.cs

@@ -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));
     }
 }

src/Framework/Framework/ViewModel/Serialization/ViewModelSerializationMap.cs

@@ -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);
@@ -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);
@@ -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>
@@ -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)
@@ -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));
@@ -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));
             }

src/Framework/Framework/ViewModel/Serialization/ViewModelSerializationMapper.cs

@@ -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>

src/Samples/AspNetCore/Startup.cs

@@ -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;
@@ -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)

src/Samples/Common/CommonConfiguration.cs

@@ -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;
@@ -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)

src/Samples/Common/DotVVM.Samples.Common.csproj

@@ -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" />

src/Samples/Common/ViewModels/ComplexSamples/ViewModelDependencyInjection/ChildViewModel.cs

@@ -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();
+    }
+}

src/Samples/Common/ViewModels/ComplexSamples/ViewModelDependencyInjection/ParentViewModel.cs

@@ -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;
+    }
+}

src/Samples/Common/ViewModels/ComplexSamples/ViewModelPopulate/ViewModelPopulateViewModel.cs

@@ -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)
+        {
+
+        }
+    }
+}
+

src/Samples/Common/Views/ComplexSamples/ViewModelDependencyInjection/Sample.dothtml

@@ -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>

src/Samples/Common/Views/ComplexSamples/ViewModelPopulate/ViewModelPopulate.dothtml

@@ -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>
+
+

src/Samples/Tests/Tests/Complex/ViewModelDependencyInjectionTests.cs

@@ -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");
+            });
+        }
+    }
+}