From 0c5c7c690b88243e922deb27917508ca0ec90d5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Wed, 16 Oct 2024 12:56:40 +0200 Subject: [PATCH] Fix ko.observable handling in Linq translations * FirstOrDefault, LastOrDefault, ... should behave like indexers - return observable, if the array contains observables, which then needs to be unwrapped - this is a regression in 4.3 * Where, Order, Take, ... return the same structure which the original array has * Select is weird as it unwraps all observables in the array, but not recursively. That's why patch adds the JsObjectObservableMap object, which can represent this mid-state (and many other combinations) - Indexing after Select was apparently always broken in DotVVM --- .../Javascript/GenericMethodCompiler.cs | 8 + .../JavascriptTranslatableMethodCollection.cs | 61 +++-- .../Javascript/JavascriptTranslator.cs | 222 +++++++++++++++++- .../Javascript/JsViewModelPropertyAdjuster.cs | 17 +- .../Framework/Hosting/DotvvmPresenter.cs | 3 +- .../Binding/JavascriptCompilationTests.cs | 119 ++++++++-- 6 files changed, 378 insertions(+), 52 deletions(-) diff --git a/src/Framework/Framework/Compilation/Javascript/GenericMethodCompiler.cs b/src/Framework/Framework/Compilation/Javascript/GenericMethodCompiler.cs index 641c66e634..8140fcc5a6 100644 --- a/src/Framework/Framework/Compilation/Javascript/GenericMethodCompiler.cs +++ b/src/Framework/Framework/Compilation/Javascript/GenericMethodCompiler.cs @@ -29,6 +29,14 @@ public GenericMethodCompiler(Func bu : builder(new[] { t?.JsExpression()! }.Concat(arg.Select(a => a.JsExpression())).ToArray(), arg.Select(a => a.OriginalExpression).ToArray()); } + public GenericMethodCompiler(Func builder, Func? check = null) + { + TryTranslateDelegate = + (t, arg, m) => check?.Invoke(m, t?.OriginalExpression, arg.Select(a => a.OriginalExpression).ToArray()) == false + ? null + : builder(new[] { t?.JsExpression()! }.Concat(arg.Select(a => a.JsExpression())).ToArray(), arg.Select(a => a.OriginalExpression).ToArray(), m); + } + public GenericMethodCompiler(Func builder, Func? check = null) { TryTranslateDelegate = diff --git a/src/Framework/Framework/Compilation/Javascript/JavascriptTranslatableMethodCollection.cs b/src/Framework/Framework/Compilation/Javascript/JavascriptTranslatableMethodCollection.cs index c9516a745f..f5fe9a1347 100644 --- a/src/Framework/Framework/Compilation/Javascript/JavascriptTranslatableMethodCollection.cs +++ b/src/Framework/Framework/Compilation/Javascript/JavascriptTranslatableMethodCollection.cs @@ -167,7 +167,7 @@ public void AddPropertyGetterTranslator(Type declaringType, string propName, IJa } public static JsExpression BuildIndexer(JsExpression target, JsExpression index, [AllowNull] MemberInfo member) => - target.Indexer(index).WithAnnotation(new VMPropertyInfoAnnotation(member.NotNull())); + target.Indexer(index).WithAnnotation(new VMPropertyInfoAnnotation(member.NotNull(), objectPath: ImmutableArray.Create("Item")!)); public void AddDefaultMethodTranslators() { @@ -192,7 +192,7 @@ JsExpression listSetIndexer(JsExpression[] args, MethodInfo method) => JsExpression arrayElementSetter(JsExpression[] args, MethodInfo method) => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("setItem").Invoke(args[0].WithAnnotation(ShouldBeObservableAnnotation.Instance), args[2], args[1]); JsExpression dictionaryGetIndexer(JsExpression[] args, MethodInfo method) => - new JsIdentifierExpression("dotvvm").Member("translations").Member("dictionary").Member("getItem").Invoke(args[0], args[1]); + new JsIdentifierExpression("dotvvm").Member("translations").Member("dictionary").Member("getItem").Invoke(args[0], args[1]).WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.FirstArgumentMethodTargetPath, isObservable: false)); JsExpression dictionarySetIndexer(JsExpression[] args, MethodInfo method) => new JsIdentifierExpression("dotvvm").Member("translations").Member("dictionary").Member("setItem").Invoke(args[0].WithAnnotation(ShouldBeObservableAnnotation.Instance), args[1], args[2]); @@ -550,9 +550,13 @@ string GetDelegateReturnTypeHash(Type type) var anyPred = new GenericMethodCompiler(args => args[1].Member("some").Invoke(args[2])); AddMethodTranslator(() => Enumerable.Any(Enumerable.Empty(), _ => false), anyPred); AddMethodTranslator(() => ImmutableArrayExtensions.Any(default(ImmutableArray), _ => false), anyPred); - AddMethodTranslator(() => Enumerable.Concat(Enumerable.Empty(), Enumerable.Empty()), new GenericMethodCompiler(args => args[1].Member("concat").Invoke(args[2]))); + AddMethodTranslator(() => Enumerable.Concat(Enumerable.Empty(), Enumerable.Empty()), new GenericMethodCompiler((args, method) => + args[1].Member("concat").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty)))); AddMethodTranslator(() => Enumerable.Count(Enumerable.Empty()), new GenericMethodCompiler(args => args[1].Member("length"))); - AddMethodTranslator(() => Enumerable.Empty().Distinct(), new GenericMethodCompiler(args => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("distinct").Invoke(args[1]), + AddMethodTranslator(() => Enumerable.Empty().Distinct(), new GenericMethodCompiler((args, method) => + new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("distinct").Invoke(args[1]) + .WithAnnotation(new ViewModelInfoAnnotation(method.ReturnType, containsObservables: false)), // distinct unwraps all observables, and only supports primitives anyway check: (method, target, arguments) => EnsureIsComparableInJavascript(method, ReflectionUtils.GetEnumerableType(arguments.First().Type).NotNull()))); AddMethodTranslator(() => Enumerable.Empty().ElementAt(0), @@ -564,43 +568,59 @@ string GetDelegateReturnTypeHash(Type type) AddMethodTranslator(() => ImmutableArrayExtensions.ElementAtOrDefault(default(ImmutableArray), 0), new GenericMethodCompiler((args, method) => BuildIndexer(args[1], args[2], method))); - var firstOrDefault = new GenericMethodCompiler((args, m) => BuildIndexer(args[1], new JsLiteral(0), m).WithAnnotation(MayBeNullAnnotation.Instance)); + var firstOrDefault = new GenericMethodCompiler((args, m) => BuildIndexer(args[1], new JsLiteral(0), m)); AddMethodTranslator(() => Enumerable.Empty().FirstOrDefault(), firstOrDefault); AddMethodTranslator(() => Enumerable.Empty().First(), firstOrDefault); AddMethodTranslator(() => ImmutableArrayExtensions.FirstOrDefault(default(ImmutableArray)), firstOrDefault); AddMethodTranslator(() => ImmutableArrayExtensions.First(default(ImmutableArray)), firstOrDefault); - var firstOrDefaultPred = new GenericMethodCompiler(args => - args[1].Member("find").Invoke(args[2]).WithAnnotation(MayBeNullAnnotation.Instance)); + var firstOrDefaultPred = new GenericMethodCompiler((args, method) => + args[1].Member("find").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, objectPath: ImmutableArray.Create("Item")!))); AddMethodTranslator(() => Enumerable.Empty().FirstOrDefault(_ => true), firstOrDefaultPred); AddMethodTranslator(() => Enumerable.Empty().First(_ => true), firstOrDefaultPred); AddMethodTranslator(() => ImmutableArrayExtensions.FirstOrDefault(default(ImmutableArray), _ => true), firstOrDefaultPred); AddMethodTranslator(() => ImmutableArrayExtensions.First(default(ImmutableArray), _ => true), firstOrDefaultPred); - var lastOrDefault = new GenericMethodCompiler(args => args[1].Member("at").Invoke(new JsLiteral(-1)).WithAnnotation(MayBeNullAnnotation.Instance)); + var lastOrDefault = new GenericMethodCompiler((args, method) => + args[1].Member("at").Invoke(new JsLiteral(-1)) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, objectPath: ImmutableArray.Create("Item")!))); AddMethodTranslator(() => Enumerable.Empty().LastOrDefault(), lastOrDefault); AddMethodTranslator(() => ImmutableArrayExtensions.LastOrDefault(default(ImmutableArray)), lastOrDefault); - var lastOrDefaultPred = new GenericMethodCompiler(args => - args[1].Member("findLast").Invoke(args[2]).WithAnnotation(MayBeNullAnnotation.Instance)); + var lastOrDefaultPred = new GenericMethodCompiler((args, method) => + args[1].Member("findLast").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, objectPath: ImmutableArray.Create("Item")!))); AddMethodTranslator(() => Enumerable.Empty().LastOrDefault(_ => false), lastOrDefaultPred); AddMethodTranslator(() => ImmutableArrayExtensions.LastOrDefault(default(ImmutableArray), _ => false), lastOrDefaultPred); - AddMethodTranslator(() => Enumerable.Empty().OrderBy(_ => Generic.Enum.Something), new GenericMethodCompiler((jArgs, dArgs) => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("orderBy") - .Invoke(jArgs[1], jArgs[2], new JsLiteral((IsDelegateReturnTypeEnum(dArgs.Last().Type)) ? GetDelegateReturnTypeHash(dArgs.Last().Type) : null)), + AddMethodTranslator(() => Enumerable.Empty().OrderBy(_ => Generic.Enum.Something), new GenericMethodCompiler((jArgs, dArgs, method) => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("orderBy") + .Invoke(jArgs[1], jArgs[2], new JsLiteral((IsDelegateReturnTypeEnum(dArgs.Last().Type)) ? GetDelegateReturnTypeHash(dArgs.Last().Type) : null)) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.FirstArgumentMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty)), check: (method, _, arguments) => EnsureIsComparableInJavascript(method, arguments.Last().Type.GetGenericArguments().Last()))); - AddMethodTranslator(() => Enumerable.Empty().OrderByDescending(_ => Generic.Enum.Something), new GenericMethodCompiler((jArgs, dArgs) => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("orderByDesc") - .Invoke(jArgs[1], jArgs[2], new JsLiteral((IsDelegateReturnTypeEnum(dArgs.Last().Type)) ? GetDelegateReturnTypeHash(dArgs.Last().Type) : null)), + AddMethodTranslator(() => Enumerable.Empty().OrderByDescending(_ => Generic.Enum.Something), new GenericMethodCompiler((jArgs, dArgs, method) => new JsIdentifierExpression("dotvvm").Member("translations").Member("array").Member("orderByDesc") + .Invoke(jArgs[1], jArgs[2], new JsLiteral((IsDelegateReturnTypeEnum(dArgs.Last().Type)) ? GetDelegateReturnTypeHash(dArgs.Last().Type) : null)) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.FirstArgumentMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty)), check: (method, _, arguments) => EnsureIsComparableInJavascript(method, arguments.Last().Type.GetGenericArguments().Last()))); - var select = new GenericMethodCompiler(args => args[1].Member("map").Invoke(args[2])); + // the lambda function will not return observable, but nested properties will again be observable + var select = new GenericMethodCompiler((args, method) => + args[1].Member("map").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, isObservable: false)) + .WithAnnotation(new ViewModelInfoAnnotation(method.ReturnType, false, null, new JsObjectObservableMap { ContainsObservables = false, DefaultChild = JsObjectObservableMap.Default })) + ); AddMethodTranslator(() => Enumerable.Empty().Select(_ => Generic.Enum.Something), select); AddMethodTranslator(() => ImmutableArrayExtensions.Select(default(ImmutableArray), _ => Generic.Enum.Something), select); - AddMethodTranslator(() => Enumerable.Empty().Skip(0), new GenericMethodCompiler(args => args[1].Member("slice").Invoke(args[2]))); + AddMethodTranslator(() => Enumerable.Empty().Skip(0), new GenericMethodCompiler((args, method) => + args[1].Member("slice").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty)))); - AddMethodTranslator(() => Enumerable.Empty().Take(0), new GenericMethodCompiler(args => - args[1].Member("slice").Invoke(new JsLiteral(0), args[2]))); + AddMethodTranslator(() => Enumerable.Empty().Take(0), new GenericMethodCompiler((args, method) => + args[1].Member("slice").Invoke(new JsLiteral(0), args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty)))); - var where = new GenericMethodCompiler(args => args[1].Member("filter").Invoke(args[2])); + var where = new GenericMethodCompiler((args, method) => + args[1].Member("filter").Invoke(args[2]) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.InstanceMethodTargetPath, isObservable: false, objectPath: ImmutableArray.Empty))); AddMethodTranslator(() => Enumerable.Empty().Where(_ => true), where); AddMethodTranslator(() => ImmutableArrayExtensions.Where(default(ImmutableArray), _ => true), where); @@ -698,7 +718,8 @@ private void AddDefaultDictionaryTranslations() var defaultValue = args.Length > 3 ? args[3] : new JsLiteral(ReflectionUtils.GetDefaultValue(method.GetGenericArguments().Last())); - return new JsIdentifierExpression("dotvvm").Member("translations").Member("dictionary").Member("getItem").Invoke(args[1], args[2], defaultValue); + return new JsIdentifierExpression("dotvvm").Member("translations").Member("dictionary").Member("getItem").Invoke(args[1], args[2], defaultValue) + .WithAnnotation(new VMPropertyInfoAnnotation(method, targetPath: VMPropertyInfoAnnotation.FirstArgumentMethodTargetPath, isObservable: false)); }); #if DotNetCore AddMethodTranslator(() => default(IReadOnlyDictionary)!.GetValueOrDefault(null!), getValueOrDefault); diff --git a/src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs b/src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs index 83957e3fcf..d54a3445d6 100644 --- a/src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs +++ b/src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Linq.Expressions; @@ -193,7 +194,12 @@ public class ViewModelInfoAnnotation : IEquatable public BindingExtensionParameter? ExtensionParameter { get; set; } public ViewModelSerializationMap? SerializationMap { get; set; } - public bool? ContainsObservables { get; set; } + public JsObjectObservableMap ObservableMap { get; set; } = JsObjectObservableMap.Default; + public bool? ContainsObservables + { + get => ObservableMap.ContainsObservables; + set => ObservableMap = ObservableMap with { ContainsObservables = value }; + } public bool Equals(ViewModelInfoAnnotation? other) => object.ReferenceEquals(this, other) || @@ -201,7 +207,7 @@ other is not null && Type == other.Type && IsControl == other.IsControl && ExtensionParameter == other.ExtensionParameter && - ContainsObservables == other.ContainsObservables; + ObservableMap == other.ObservableMap; public override bool Equals(object? obj) => obj is ViewModelInfoAnnotation obj2 && this.Equals(obj2); @@ -212,32 +218,230 @@ public ViewModelInfoAnnotation(Type type, bool isControl = false, BindingExtensi this.Type = type; this.IsControl = isControl; this.ExtensionParameter = extensionParameter; - this.ContainsObservables = containsObservables; + this.ObservableMap = JsObjectObservableMap.FromBool(containsObservables); + } + + public ViewModelInfoAnnotation(Type type, bool isControl, BindingExtensionParameter? extensionParameter, JsObjectObservableMap? observableMap) + { + this.Type = type; + this.IsControl = isControl; + this.ExtensionParameter = extensionParameter; + this.ObservableMap = observableMap ?? JsObjectObservableMap.Default; + } + } + + public sealed record JsObjectObservableMap: IEquatable + { + /// The default value for all descendants, unless overriden using the other properties + public bool? ContainsObservables { get; set; } + + /// Default value for all properties. Null means that `ContainsObservables` is assumed to be the same as the parent's + public JsObjectObservableMap? DefaultChild { get; set; } + + public Dictionary? ChildObjects { get; set; } + + public Dictionary? PropertyIsObservable { get; set; } + + public bool? IsPropertyObservable(string? name) => name is {} && PropertyIsObservable?.TryGetValue(name, out var value) == true ? value : ContainsObservables; + public bool? IsPropertyObservable(ReadOnlySpan objectPath) + { + if (objectPath.Length == 0) return null; + var current = this.GetChildObject(objectPath.Slice(0, objectPath.Length - 1)); + return current.IsPropertyObservable(objectPath[objectPath.Length - 1]); + } + + public JsObjectObservableMap GetChildObject(string? name) + { + if (name is {} && ChildObjects?.TryGetValue(name, out var value) == true) + return value; + if (DefaultChild is {}) + return DefaultChild; + return FromBool(ContainsObservables); + } + public JsObjectObservableMap GetChildObject(ReadOnlySpan objectPath) + { + var current = this; + foreach (var name in objectPath) + current = current.GetChildObject(name); + return current; + } + + public JsObjectObservableMap OverrideWith(JsObjectObservableMap? @override) + { + if (@override is null || @override == Default || this == @override || this == Default) return this; + + var result = this with { ContainsObservables = @override.ContainsObservables ?? ContainsObservables }; + + if (@override.DefaultChild is not null) + { + result.DefaultChild = DefaultChild?.OverrideWith(@override.DefaultChild) ?? @override.DefaultChild; + } + + if (@override.ChildObjects is not null) + { + result.ChildObjects = result.ChildObjects is {} ? new(result.ChildObjects) : new(); + foreach (var (key, value) in @override.ChildObjects) + result.ChildObjects[key] = result.ChildObjects.TryGetValue(key, out var existing) ? existing.OverrideWith(value) : value; + } + if (@override.PropertyIsObservable is not null) + { + result.PropertyIsObservable = result.PropertyIsObservable is {} ? new(result.PropertyIsObservable) : new(); + foreach (var (key, value) in @override.PropertyIsObservable) + result.PropertyIsObservable[key] = value; + } + return result; } + + public override string ToString() + { + if (this == Default) return "Default"; + if (this == Observables) return "Observables"; + if (this == Plain) return "Plain"; + + var sb = new System.Text.StringBuilder().Append("{ "); + if (ContainsObservables is {}) + sb.Append("ContainsObservables = ").Append(ContainsObservables).Append(", "); + if (PropertyIsObservable is {Count: > 0}) + sb.Append("Properties = { ").Append(string.Join(", ", PropertyIsObservable.Select(k => $"{k.Key}: {(k.Value ? "observable" : "plain")}"))).Append(" }, "); + if (DefaultChild is {}) + sb.Append("DefaultChild = ").Append(DefaultChild).Append(", "); + if (ChildObjects is {Count: > 0}) + sb.Append("ChildObjects = { ").Append(string.Join(", ", ChildObjects.Select(k => $"{k.Key}: {k.Value}"))).Append(" }, "); + return sb.Append("}").ToString(); + } + + public bool Equals(JsObjectObservableMap? other) + { + if (other == (object)this) return true; + if (other is null) return false; + return ContainsObservables == other.ContainsObservables && + (this.ChildObjects?.Count ?? 0) == (other.ChildObjects?.Count ?? 0) && + (this.PropertyIsObservable?.Count ?? 0) == (other.PropertyIsObservable?.Count ?? 0) && + DefaultChild == other.DefaultChild && + dictEquals(ChildObjects, other.ChildObjects) && + dictEquals(PropertyIsObservable, other.PropertyIsObservable); + + static bool dictEquals(Dictionary? a, Dictionary? b) where V: IEquatable where K: notnull + { + if (a is null or { Count: 0 } && b is null or { Count: 0 }) + return true; + if (a is null || b is null || a.Count != b.Count) + return false; + + foreach (var (k, v) in a) + { + if (!b.TryGetValue(k, out var v2) || Equals(v, v2)) + return false; + } + return true; + } + } + + public override int GetHashCode() + { + var hash = (ContainsObservables, DefaultChild, ChildObjects?.Count, PropertyIsObservable?.Count).GetHashCode(); + if (ChildObjects is not null) + foreach (var (k, v) in ChildObjects) + hash += (k, v).GetHashCode(); // plus is commutative, because dict order should not matter + if (PropertyIsObservable is not null) + foreach (var (k, v) in PropertyIsObservable) + hash += (k, v).GetHashCode(); + return hash; + } + + /// Assume the current preference - i.e. observables in value bindings and plain objects in staticCommands + public static readonly JsObjectObservableMap Default = new(); + /// The object is wrapped in observables all the way down + public static readonly JsObjectObservableMap Observables = new() { ContainsObservables = true }; + /// The object contains plain JS values without knockout observables. + public static readonly JsObjectObservableMap Plain = new() { ContainsObservables = false }; + internal static JsObjectObservableMap FromBool(bool? containsObservables) => + containsObservables switch { + true => Observables, + false => Plain, + null => Default + }; } - /// Marks that the expression is essentially a member access on the target. We use this to keep track which objects have observables and which don't. + /// + /// Marks that the expression is essentially a member access on the target - i.e. an expression which accesses part of a viewmodel. We use this to keep track which objects have observables and which don't. + /// DotVVM will primarily use this annotate to determine if the current expression is ko.observable or not, based on the target . + /// public class VMPropertyInfoAnnotation { - public VMPropertyInfoAnnotation(MemberInfo? memberInfo, Type? resultType = null, ViewModelPropertyMap? serializationMap = null) + public VMPropertyInfoAnnotation(MemberInfo? memberInfo, Type? resultType = null, ViewModelPropertyMap? serializationMap = null, bool? isObservable = null, ImmutableArray<(JsTreeRole role, int index)>? targetPath = null, ImmutableArray? objectPath = null) { ResultType = resultType ?? memberInfo!.GetResultType(); MemberInfo = memberInfo; SerializationMap = serializationMap; + IsObservable = isObservable; + TargetPath = targetPath ?? DefaultTargetPath; + ObjectPath = objectPath ?? ImmutableArray.Create(memberInfo?.Name); } - public VMPropertyInfoAnnotation(Type resultType) - { - ResultType = resultType; - } + public VMPropertyInfoAnnotation(Type resultType) : this(null, resultType) { } + /// C# type of the expression output value public Type ResultType { get; } + /// Property/Field/Indexer or method info this expression represents public MemberInfo? MemberInfo { get; } + /// Serialziation map of this property public ViewModelPropertyMap? SerializationMap { get; set; } + /// If this property (i.e. the result of this expression) is wrapped in knockout observable. If null, the default based on will be assumed. + public bool? IsObservable { get; } + + /// By default, it is the first expression, but can be configured to be any sub-expression (i.e., the first method argument). + public ImmutableArray<(JsTreeRole role, int index)> TargetPath { get; set; } + + /// Path of the projection in terms of object properties, which is then used assess property observability based on . Fields can be null, if the property name is unknown - the will then be used. If the projection returns a modified version of the original object, empty array should be used as the ObjectPath + public ImmutableArray ObjectPath { get; set; } public static VMPropertyInfoAnnotation FromDotvvmProperty(DotvvmProperty p) => p.PropertyInfo is null ? new VMPropertyInfoAnnotation(p.PropertyType) : new VMPropertyInfoAnnotation(p.PropertyInfo, p.PropertyType); + + /// This expression is a projection of the object in the `Target` property. + public static ImmutableArray<(JsTreeRole role, int index)> DefaultTargetPath = ImmutableArray.Create(((JsTreeRole)JsTreeRoles.TargetExpression, 0)); + /// This expression a method invocation on the projected object - i.e. the object is is Target.Target + public static ImmutableArray<(JsTreeRole role, int index)> InstanceMethodTargetPath = ImmutableArray.Create(((JsTreeRole)JsTreeRoles.TargetExpression, 0), ((JsTreeRole)JsTreeRoles.TargetExpression, 0)); + /// The expression a function invocation with the projected object as the first argument - i.e. the object is in Arguments[0] + public static ImmutableArray<(JsTreeRole role, int index)> FirstArgumentMethodTargetPath = ImmutableArray.Create(((JsTreeRole)JsTreeRoles.Argument, 0)); + + /// Gets the target expression from the annotated expression. + public JsNode? EvaluateTargetPath(JsNode expression) + { + JsNode? node = expression; + foreach (var (role, index) in TargetPath) + { + if (index < 0) throw new Exception("Invalid target path index"); + + if (node is null) + { + return null; + } + + node = GetChild(node, role, index); + } + return node; + + static JsNode? GetChild(JsNode node, JsTreeRole role, int index) + { + foreach (var child in node.Children) + { + if (child.Role == role) + { + if (index == 0) + { + return child; + } + + index--; + } + } + return null; + } + } + } public class CompositeJavascriptTranslator: IJavascriptMethodTranslator diff --git a/src/Framework/Framework/Compilation/Javascript/JsViewModelPropertyAdjuster.cs b/src/Framework/Framework/Compilation/Javascript/JsViewModelPropertyAdjuster.cs index 1cc6a40fc1..8534403854 100644 --- a/src/Framework/Framework/Compilation/Javascript/JsViewModelPropertyAdjuster.cs +++ b/src/Framework/Framework/Compilation/Javascript/JsViewModelPropertyAdjuster.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections; using System.Collections.Generic; using System.Diagnostics; @@ -56,22 +56,25 @@ protected override void DefaultVisit(JsNode node) if (node.Annotation() is { MemberInfo: var member } propAnnotation) { - var target = node.GetChildByRole(JsTreeRoles.TargetExpression)!; - if (target.HasAnnotation(ObservableUnwrapInvocationAnnotation.Instance)) + var target = propAnnotation.EvaluateTargetPath(node); + + if (target is {} && target.HasAnnotation(ObservableUnwrapInvocationAnnotation.Instance)) target = target.GetChildByRole(JsTreeRoles.TargetExpression); - else if (target.HasAnnotation(ObservableSetterInvocationAnnotation.Instance)) + else if (target is {} && target.HasAnnotation(ObservableSetterInvocationAnnotation.Instance)) throw new NotImplementedException(); var propertyType = propAnnotation.ResultType; var annotation = node.Annotation() ?? new ViewModelInfoAnnotation(propertyType); var targetAnnotation = target?.Annotation(); + var propertyIsObservable = propAnnotation.IsObservable; if (targetAnnotation is {}) { propAnnotation.SerializationMap ??= targetAnnotation.SerializationMap?.Properties .FirstOrDefault(p => p.PropertyInfo == member); - annotation.ContainsObservables ??= targetAnnotation.ContainsObservables; + annotation.ObservableMap = targetAnnotation.ObservableMap.GetChildObject(propAnnotation.ObjectPath.AsSpan()).OverrideWith(annotation.ObservableMap); + propertyIsObservable ??= targetAnnotation.ObservableMap.IsPropertyObservable(propAnnotation.ObjectPath.AsSpan()); } if (propAnnotation.SerializationMap is ViewModelPropertyMap propertyMap) { @@ -99,9 +102,7 @@ member is {} && throw new NotSupportedException($"Cannot translate property {member.Name} on custom primitive type {member.DeclaringType.ToCode()} to Javascript. Use the ToString() method to get the underlying value."); } - annotation.ContainsObservables ??= !this.preferUsingState; // we don't know -> guess what is the current preference - - if (annotation.ContainsObservables == true) + if (propertyIsObservable ?? !preferUsingState) // if we don't know -> assume what is the current preference { node.AddAnnotation(ResultIsObservableAnnotation.Instance); diff --git a/src/Framework/Framework/Hosting/DotvvmPresenter.cs b/src/Framework/Framework/Hosting/DotvvmPresenter.cs index dee997e54a..db046d8779 100644 --- a/src/Framework/Framework/Hosting/DotvvmPresenter.cs +++ b/src/Framework/Framework/Hosting/DotvvmPresenter.cs @@ -538,7 +538,8 @@ Cross site iframe are disabled in this application. await context.RejectRequest($""" Pages can not be loaded using Javascript for security reasons. Try refreshing the page to get rid of the error. - If you are the developer, you can disable this check by setting DotvvmConfiguration.Security.VerifySecFetchForPages.ExcludeRoute("{route}"). [dest: {dest}, site: {site}] + If you are the developer, you can disable this check by setting DotvvmConfiguration.Security.VerifySecFetchForPages.ExcludeRoute("{route}"). [dest: {dest}, site: {site}]. + Note that this security check is not compatible with JavaScript-based prefetching, such as TurboLinks, Cloudflare Speed Brain, or similar, and you'll need to disable one of them. The check is "only" a deference-in-depth measure against XSS, and disabling it is not a vulnerability by itself. """); if (site != "same-origin") await context.RejectRequest($"Cross site SPA requests are disabled."); diff --git a/src/Tests/Binding/JavascriptCompilationTests.cs b/src/Tests/Binding/JavascriptCompilationTests.cs index d387558513..900911b7e4 100644 --- a/src/Tests/Binding/JavascriptCompilationTests.cs +++ b/src/Tests/Binding/JavascriptCompilationTests.cs @@ -30,13 +30,17 @@ static JavascriptCompilationTests() { configuration = DotvvmTestHelper.CreateConfiguration(); configuration.RegisterApiClient(typeof(TestApiClient), "http://server/api", "./apiscript.js", "_testApi"); + var methods = configuration.Markup.JavascriptTranslator.MethodCollection; + methods.AddMethodTranslator(() => TestJsTransations.GetTestPlainObject(), new GenericMethodCompiler(args => new JsIdentifierExpression("testPlainObject").WithAnnotation(new ViewModelInfoAnnotation(typeof(TestArraysViewModel), containsObservables: false)))); + methods.AddMethodTranslator(() => TestJsTransations.GetTestObjectWithObservables(), new GenericMethodCompiler(args => new JsIdentifierExpression("testPlainObject").WithAnnotation(new ViewModelInfoAnnotation(typeof(TestArraysViewModel), containsObservables: true)))); + configuration.Freeze(); bindingHelper = new BindingTestHelper(configuration); } public string CompileBinding(string expression, params Type[] contexts) => CompileBinding(expression, contexts, expectedType: typeof(object)); public string CompileBinding(string expression, NamespaceImport[] imports, params Type[] contexts) => CompileBinding(expression, contexts, expectedType: typeof(object), imports); - public string CompileBinding(string expression, Type[] contexts, Type expectedType, NamespaceImport[] imports = null) + public string CompileBinding(string expression, Type[] contexts, Type expectedType, NamespaceImport[] imports = null, bool nullChecks = false) { - return bindingHelper.ValueBindingToJs(expression, contexts, expectedType, imports, niceMode: false); + return bindingHelper.ValueBindingToJs(expression, contexts, expectedType, imports, nullChecks, niceMode: false); } public static string FormatKnockoutScript(ParametrizedCode code) => JavascriptTranslator.FormatKnockoutScript(code); @@ -531,6 +535,13 @@ public void JsTranslator_DictionaryIndexer_Get() Assert.AreEqual("dotvvm.translations.dictionary.getItem(Dictionary(),1)", result); } + [TestMethod] + public void JsTranslator_DictionaryIndexer_GetObject() + { + var result = CompileBinding("StringVmDictionary['test'].Collection[5].StringValue.Length", [typeof(TestViewModel)], typeof(object), nullChecks: true); + Assert.AreEqual("(()=>{let a;return ((a=dotvvm.translations.dictionary.getItem(StringVmDictionary(),\"test\")?.Collection())&&a[5]())?.StringValue()?.length;})()", result); + } + [TestMethod] public void JsTranslator_ReadOnlyDictionaryIndexer_Get() { @@ -607,6 +618,62 @@ public void JsTranslator_EnumerableSelect(string binding) Assert.AreEqual("LongArray().map((item)=>-ko.unwrap(item))", result); } + [TestMethod] + public void JsTranslator_EnumerableSelectObservableHandling1() + { + var result = CompileBinding("VmArray.Select(item => item.MyProperty).ToArray()[0] == 1", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).MyProperty())[0]==1", result); + } + + [TestMethod] + public void JsTranslator_EnumerableSelectObservableHandling2() + { + var result = CompileBinding("VmArray.Select(item => item.MyProperty + 1).ToArray()[0] == 1", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).MyProperty()+1)[0]==1", result); + } + + [TestMethod] + public void JsTranslator_EnumerableSelectObservableHandling3() + { + var result = CompileBinding("VmArray.Select(item => item.ChildObject).ToArray()[0].SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).ChildObject())[0].SomeString()==\"abc\"", result); + } + + [TestMethod] + public void JsTranslator_EnumerableSelectFirstObservableHandling() + { + var result = CompileBinding("VmArray.Select(item => item.ChildObject).FirstOrDefault(i => i.SomeString.StartsWith('a')).SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).ChildObject()).find((i)=>ko.unwrap(i).SomeString().startsWith(\"a\")).SomeString()==\"abc\"", result); + // ^^^^^ not observable, because .map unwraps it ^^ + // without the select, we'd need observable unwrap after .find + var withoutSelect = CompileBinding("VmArray.FirstOrDefault(i => i.ChildObject.SomeString.StartsWith('a')).ChildObject.SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().find((i)=>ko.unwrap(i).ChildObject().SomeString().startsWith(\"a\"))().ChildObject().SomeString()==\"abc\"", withoutSelect); + } + + [TestMethod] + public void JsTranslator_EnumerableSelectOrderObservableHandling() + { + var result = CompileBinding("VmArray.Select(item => item.ChildObject).OrderBy(i => i.SomeString).ToArray()[0].SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("dotvvm.translations.array.orderBy(VmArray().map((item)=>ko.unwrap(item).ChildObject()),(i)=>ko.unwrap(i).SomeString(),null)[0].SomeString()==\"abc\"", result); + // not observable, because .map unwraps it ^^^ + } + + [TestMethod] + public void JsTranslator_EnumerableSelectFilterObservableHandling() + { + var result = CompileBinding("VmArray.Select(item => item.ChildObject).Where(i => i != null).ElementAt(1).SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).ChildObject()).filter((i)=>ko.unwrap(i)!=null)[1].SomeString()==\"abc\"", result); + // not observable, because .map unwraps it ^^^ + } + + [TestMethod] + public void JsTranslator_EnumerableSelectSliceObservableHandling() + { + var result = CompileBinding("VmArray.Select(item => item.ChildObject).Take(4).LastOrDefault().SomeString == 'abc'", new[] { new NamespaceImport("System.Linq") }, new[] { typeof(TestViewModel) }); + Assert.AreEqual("VmArray().map((item)=>ko.unwrap(item).ChildObject()).slice(0,4).at(-1).SomeString()==\"abc\"", result); + // not observable, because .map unwraps it ^^^^^^^ + } + [TestMethod] [DataRow("Enumerable.Concat(LongArray, LongArray)", DisplayName = "Regular call of Enumerable.Concat")] [DataRow("LongArray.Concat(LongArray)", DisplayName = "Syntax sugar - extension method")] @@ -657,23 +724,23 @@ public void JsTranslator_ListAddRange() } [TestMethod] - [DataRow("Enumerable.All(LongArray, (long item) => item > 0)", DisplayName = "Regular call of Enumerable.All")] - [DataRow("LongArray.All((long item) => item > 0)", DisplayName = "Syntax sugar - extension method")] - [DataRow("LongArray.ToImmutableArray().All((long item) => item > 0)", DisplayName = "Immutable array - extension method")] + [DataRow("!Enumerable.All(LongArray, (long item) => item > 0)", DisplayName = "Regular call of Enumerable.All")] + [DataRow("!LongArray.All((long item) => item > 0)", DisplayName = "Syntax sugar - extension method")] + [DataRow("!LongArray.ToImmutableArray().All((long item) => item > 0)", DisplayName = "Immutable array - extension method")] public void JsTranslator_EnumerableAll(string binding) { var result = CompileBinding(binding, new[] { new NamespaceImport("System.Linq"), new NamespaceImport("System.Collections.Immutable") }, new[] { typeof(TestViewModel) }); - Assert.AreEqual("LongArray().every((item)=>ko.unwrap(item)>0)", result); + Assert.AreEqual("!LongArray().every((item)=>ko.unwrap(item)>0)", result); } [TestMethod] - [DataRow("Enumerable.Any(LongArray, (long item) => item > 0)", DisplayName = "Regular call of Enumerable.Any")] - [DataRow("LongArray.Any((long item) => item > 0)", DisplayName = "Syntax sugar - extension method")] - [DataRow("LongArray.ToImmutableArray().Any((long item) => item > 0)", DisplayName = "Immutable array - extension method")] + [DataRow("!Enumerable.Any(LongArray, (long item) => item > 0)", DisplayName = "Regular call of Enumerable.Any")] + [DataRow("!LongArray.Any((long item) => item > 0)", DisplayName = "Syntax sugar - extension method")] + [DataRow("!LongArray.ToImmutableArray().Any((long item) => item > 0)", DisplayName = "Immutable array - extension method")] public void JsTranslator_EnumerableAny(string binding) { var result = CompileBinding(binding, new[] { new NamespaceImport("System.Linq"), new NamespaceImport("System.Collections.Immutable") }, new[] { typeof(TestViewModel) }); - Assert.AreEqual("LongArray().some((item)=>ko.unwrap(item)>0)", result); + Assert.AreEqual("!LongArray().some((item)=>ko.unwrap(item)>0)", result); } [TestMethod] @@ -703,13 +770,13 @@ public void JsTranslator_EnumerableEmpty(string binding) [TestMethod] - [DataRow("Enumerable.FirstOrDefault(LongArray)", DisplayName = "Regular call of Enumerable.FirstOrDefault")] - [DataRow("LongArray.FirstOrDefault()", DisplayName = "Syntax sugar - extension method")] - [DataRow("LongArray.ToImmutableArray().FirstOrDefault()", DisplayName = "Immutable array - extension method")] + [DataRow("Enumerable.FirstOrDefault(LongArray) == 1", DisplayName = "Regular call of Enumerable.FirstOrDefault")] + [DataRow("LongArray.FirstOrDefault() == 1", DisplayName = "Syntax sugar - extension method")] + [DataRow("LongArray.ToImmutableArray().FirstOrDefault() == 1", DisplayName = "Immutable array - extension method")] public void JsTranslator_EnumerableFirstOrDefault(string binding) { var result = CompileBinding(binding, new[] { new NamespaceImport("System.Linq"), new NamespaceImport("System.Collections.Immutable") }, new[] { typeof(TestViewModel) }); - Assert.AreEqual("LongArray()[0]", result); + Assert.AreEqual("LongArray()[0]()==1", result); } [TestMethod] @@ -722,6 +789,22 @@ public void JsTranslator_EnumerableFirstOrDefaultParametrized(string binding) Assert.AreEqual("LongArray().find((item)=>ko.unwrap(item)>0)", result); } + [TestMethod] + public void JsTranslator_EnumerableFirstOrDefaultObservableHandling() + { + var withUnwraps = CompileBinding("TestJsTransations.GetTestObjectWithObservables().ObjectArray.FirstOrDefault(o => o.Enum == 'Value1').Int == 12", new[] { new NamespaceImport("System.Linq"), new NamespaceImport(typeof(TestJsTransations).Namespace) }, new[] { typeof(TestViewModel) }); + + Assert.AreEqual("testPlainObject.ObjectArray().find((o)=>ko.unwrap(o).Enum()==\"Value1\")().Int()==12", withUnwraps); + } + + [TestMethod] + public void JsTranslator_EnumerableFirstOrDefaultObservableHandling2() + { + var withoutUnwraps = CompileBinding("TestJsTransations.GetTestPlainObject().ObjectArray.OrderBy(a => a.Enum).LastOrDefault().Int == 12", new[] { new NamespaceImport("System.Linq"), new NamespaceImport(typeof(TestJsTransations).Namespace) }, new[] { typeof(TestViewModel) }); + + Assert.AreEqual("dotvvm.translations.array.orderBy(testPlainObject.ObjectArray,(a)=>ko.unwrap(a).Enum(),\"ObApbFFA2mUjzIJF\").at(-1).Int==12", withoutUnwraps); + } + [TestMethod] public void JsTranslator_ListInsert() { @@ -1409,6 +1492,14 @@ public string SomeProperty } } + public static class TestJsTransations + { + // returns object which is not wrapped in ko.observables + public static TestArraysViewModel GetTestPlainObject() => new TestArraysViewModel(); + // returns object which is wrapped in ko.observables + public static TestArraysViewModel GetTestObjectWithObservables() => new TestArraysViewModel(); + } + public class TestExtensionParameterConflictViewModel { [Bind(Name = "IndexProperty")]