Skip to content

Commit

Permalink
add support for IsNew and GetType in LINQ provider and LambdaToJavasc…
Browse files Browse the repository at this point in the history 
…riptConverter (useful for ToString)
  • Loading branch information
@olmobrutall
olmobrutall committed Dec 12, 2021
1 parent 3410e83 commit 644151a
Show file tree
Hide file tree
Showing 12 changed files with 327 additions and 28 deletions.
29 changes: 29 additions & 0 deletions Signum.Engine.Extensions/Cache/ToStringExpressionVisitor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ protected override Expression VisitMember(MemberExpression node)

if (exp is CachedEntityExpression cee)
{
if (node.Member.Name == "IsNew")
return Expression.Constant(false);

Field field =
cee.FieldEmbedded != null ? cee.FieldEmbedded.GetField(node.Member) :
cee.FieldMixin != null ? cee.FieldMixin.GetField(node.Member) :
Expand All @@ -53,6 +56,26 @@ protected override Expression VisitMember(MemberExpression node)
return node.Update(exp);
}

protected override Expression VisitConditional(ConditionalExpression c) // a.IsNew
{
Expression test = this.Visit(c.Test);
if (test is ConstantExpression co)
{
if ((bool)co.Value!)
return this.Visit(c.IfTrue);
else
return this.Visit(c.IfFalse);
}

Expression ifTrue = this.Visit(c.IfTrue);
Expression ifFalse = this.Visit(c.IfFalse);
if (test != c.Test || ifTrue != c.IfTrue || ifFalse != c.IfFalse)
{
return Expression.Condition(test, ifTrue, ifFalse);
}
return c;
}

private Expression BindMember(CachedEntityExpression n, Field field, Expression? prevPrimaryKey)
{
Expression body = GetField(field, n.Constructor, prevPrimaryKey);
Expand Down Expand Up @@ -192,6 +215,7 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
var obj = base.Visit(node.Object);
var args = base.Visit(node.Arguments);


if (node.Method.Name == "ToString" && node.Arguments.IsEmpty() && obj is CachedEntityExpression ce && ce.Type.IsEntity())
{
var table = (Table)ce.Constructor.table;
Expand All @@ -212,6 +236,11 @@ protected override Expression VisitMethodCall(MethodCallExpression node)
}
}

if(node.Method.Name == "GetType" && obj is CachedEntityExpression ce2)
{
return Expression.Constant(ce2.Type);
}

LambdaExpression? lambda = ExpressionCleaner.GetFieldExpansion(obj?.Type, node.Method);

if (lambda != null)
Expand Down
2 changes: 1 addition & 1 deletion Signum.Engine/Database.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static T Save<T>(this T entity)
}
catch (Exception e)
{
e.Data["entity"] = ((Entity)(IEntity)entity).BaseToString();
e.Data["entity"] = entity;

throw;
}
Expand Down
74 changes: 74 additions & 0 deletions Signum.Engine/Linq/ExpressionVisitor/QueryBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,6 +1487,26 @@ protected override MemberAssignment VisitMemberAssignment(MemberAssignment assig
return assignment;
}

protected override Expression VisitConditional(ConditionalExpression c) // a.IsNew
{
Expression test = this.Visit(c.Test);
if (test is ConstantExpression co)
{
if ((bool)co.Value!)
return this.Visit(c.IfTrue);
else
return this.Visit(c.IfFalse);
}

Expression ifTrue = this.Visit(c.IfTrue);
Expression ifFalse = this.Visit(c.IfFalse);
if (test != c.Test || ifTrue != c.IfTrue || ifFalse != c.IfFalse)
{
return Expression.Condition(test, ifTrue, ifFalse);
}
return c;
}

protected override Expression VisitMember(MemberExpression m)
{
Expression ex = base.VisitMember(m);
Expand Down Expand Up @@ -1539,6 +1559,7 @@ public Expression BindMethodCall(MethodCallExpression m)
return result;
}


if (source is ImplementedByExpression ib)
{
if (m.Method.IsExtensionMethod())
Expand Down Expand Up @@ -1625,9 +1646,39 @@ public Expression BindMethodCall(MethodCallExpression m)
return tablePeriod;
}

Expression PartialEval(Expression ee)
{
if (m.Method.IsExtensionMethod())
{
return ExpressionEvaluator.PartialEval(Expression.Call(null, m.Method, m.Arguments.Skip(1).PreAnd(ee)));
}
else
{
return ExpressionEvaluator.PartialEval(Expression.Call(ee, m.Method, m.Arguments));
}
}

if (source is TypeEntityExpression type)
{
return Condition(Expression.NotEqual(type.ExternalId, NullId(type.ExternalId.ValueType)),
ifTrue: PartialEval(Expression.Constant(type.TypeValue)),
ifFalse: Expression.Constant(null, m.Type));
}

if(source is TypeImplementedByExpression typeIB)
{
return typeIB.TypeImplementations.Aggregate(
(Expression)Expression.Constant(null, m.Type),
(acum, kvp) => Condition(Expression.NotEqual(kvp.Value, NullId(kvp.Value.Value.Type)),
ifTrue: PartialEval(Expression.Constant(kvp.Key)),
ifFalse: acum));
}

return m;
}




private ConditionalExpression DispatchConditional(MethodCallExpression m, Expression test, Expression ifTrue, Expression ifFalse)
{
Expand Down Expand Up @@ -1748,6 +1799,9 @@ public Expression BindMemberAccess(MemberExpression m)
if (pi != null && ReflectionTools.PropertyEquals(pi, EntityExpression.IdOrNullProperty))
return ee.ExternalId;

if (m.Member.Name == nameof(Entity.IsNew))
return Expression.Constant(false);

FieldInfo? fi = m.Member as FieldInfo ?? Reflector.TryFindFieldInfo(ee.Type, pi!);

if (fi == null)
Expand Down Expand Up @@ -1862,6 +1916,25 @@ public Expression BindMemberAccess(MemberExpression m)
_ => throw new InvalidOperationException("The member {0} of MListElement is not accesible on queries".FormatWith(m.Member)),
};
}
case DbExpressionType.TypeEntity:
{
TypeEntityExpression type = (TypeEntityExpression)source;

return Condition(Expression.NotEqual(type.ExternalId, NullId(type.ExternalId.ValueType)),
ifTrue: ExpressionEvaluator.PartialEval(Expression.MakeMemberAccess(Expression.Constant(type.TypeValue), m.Member)),
ifFalse: Expression.Constant(null, m.Type));
}

case DbExpressionType.TypeImplementedBy:
{
TypeImplementedByExpression typeIB = (TypeImplementedByExpression)source;

return typeIB.TypeImplementations.Aggregate(
(Expression)Expression.Constant(null, m.Type),
(acum, kvp) => Condition(Expression.NotEqual(kvp.Value, NullId(kvp.Value.Value.Type)),
ifTrue: ExpressionEvaluator.PartialEval(Expression.MakeMemberAccess(Expression.Constant(kvp.Key), m.Member)),
ifFalse: acum));
}
}
}
break;
Expand Down Expand Up @@ -3477,6 +3550,7 @@ protected override Expression VisitConditional(ConditionalExpression c)
var ifTrue = Visit(c.IfTrue);
var ifFalse = Visit(c.IfFalse);


if (colExpression is LiteReferenceExpression)
{
return Combiner<LiteReferenceExpression>(ifTrue, ifFalse, (col, l, r) =>
Expand Down
2 changes: 1 addition & 1 deletion Signum.Engine/Operations/OperationLogic.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ public static Dictionary<OperationSymbol, string> ServiceCanExecute(Entity entit
}
catch(Exception e)
{
e.Data["entity"] = entity.BaseToString();
e.Data["entity"] = entity;
throw;
}
}
Expand Down
13 changes: 5 additions & 8 deletions Signum.Entities/Entity.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,12 @@ protected bool SetIfNew<T>(ref T field, T value, [CallerMemberName]string? autom
return base.Set<T>(ref field, value, automaticPropertyName!);
}

public override string ToString()
{
return BaseToString();
}

public string BaseToString()
{
return "{0} ({1})".FormatWith(GetType().NiceName(), id.HasValue ? id.ToString() : LiteMessage.New_G.NiceToString().ForGenderAndNumber(this.GetType().GetGender()));
}
[AutoExpressionField]
public override string ToString() => As.Expression(() => BaseToString());

[AutoExpressionField]
public string BaseToString() => As.Expression(() => IsNew ? GetType().NewNiceName() : GetType().NiceName() + " " + Id);

public override bool Equals(object? obj)
{
Expand Down
2 changes: 0 additions & 2 deletions Signum.Entities/Lite.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,6 @@ public enum LiteMessage
IdNotValid,
[Description("Invalid Format")]
InvalidFormat,
[Description("New")]
New_G,
[Description("Type {0} not found")]
Type0NotFound,
[Description("Text")]
Expand Down
13 changes: 13 additions & 0 deletions Signum.Entities/Reflection/Reflector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -480,4 +480,17 @@ public static int NumDecimals(string format)

return str.Length;
}

public static string NiceCount(this Type type, int count)
{
if (count == 1)
return "1 " + type.NiceName();

return count + " " + type.NicePluralName();
}

public static string NewNiceName(this Type type)
{
return NormalWindowMessage.New0_G.NiceToString().ForGenderAndNumber(type.GetGender()).FormatWith(type.NiceName());
}
}
98 changes: 86 additions & 12 deletions Signum.React/Facades/LambdaToJavascriptConverter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ internal class LambdaToJavascriptConverter
if (lambdaExpression == null)
return null;

var body = ToJavascript(lambdaExpression.Parameters.Single(), lambdaExpression.Body);
var newLambda = (LambdaExpression)ExpressionCleaner.Clean(lambdaExpression)!;

var body = ToJavascript(newLambda.Parameters.Single(), newLambda.Body);

if (body == null)
return null;
Expand All @@ -30,14 +32,22 @@ internal class LambdaToJavascriptConverter
if (param == expr)
return "e";

if (expr is ConstantExpression ce && expr.Type == typeof(string))
if (expr is ConstantExpression ce)
{
var str = (string)ce.Value!;
if (expr.Type == typeof(string))
{
var str = (string)ce.Value!;

if (!str.HasText())
return "\"\"";
if (!str.HasText())
return "\"\"";

return "\"" + str.Replace(replacements) + "\"";
return "\"" + str.Replace(replacements) + "\"";
}

if(ce.Value == null)
{
return "null";
}
}

if (expr is MemberExpression me)
Expand All @@ -58,22 +68,69 @@ internal class LambdaToJavascriptConverter
return a + "." + me.Member.Name.FirstLower();
}

if (expr is BinaryExpression be && be.NodeType == ExpressionType.Add)
if (expr is BinaryExpression be)
{
var a = ToJavascriptToString(param, be.Left);
var b = ToJavascriptToString(param, be.Right);
if (be.NodeType == ExpressionType.Add && be.Type == typeof(string))
{

if (a != null && b != null)
return "(" + a + " + " + b + ")";
var a = ToJavascriptToString(param, be.Left);
var b = ToJavascriptToString(param, be.Right);

return null;
if (a != null && b != null)
return "(" + a + " + " + b + ")";

return null;
}
else
{
var a = ToJavascript(param, be.Left);
var b = ToJavascript(param, be.Right);

var op = ToJsOperator(be.NodeType);

if (a != null && op != null && b != null)
{
return a + op + b;
}

return null;
}
}

if (expr is MethodCallExpression mc)
{
if (mc.Method.Name == "ToString")
return ToJavascriptToString(param, mc.Object!, mc.TryGetArgument("format") is ConstantExpression format ? (string)format.Value! : null);

if (mc.Method.Name == "GetType" && mc.Object != null && mc.Object.Type.IsIEntity())
{
var obj = ToJavascript(param, mc.Object!);
if (obj == null)
return null;

return "getTypeInfo(" + obj + ")";
}

if (mc.Method.IsExtensionMethod() && mc.Arguments.Only()?.Type == typeof(Type))
{
var obj = ToJavascript(param, mc.Arguments.SingleEx());
if (obj == null)
return null;

if (mc.Method.Name == nameof(DescriptionManager.NiceName))
return obj + ".niceName";


if (mc.Method.Name == nameof(DescriptionManager.NicePluralName))
return obj + ".nicePluralName";

if (mc.Method.Name == nameof(Reflector.NewNiceName))
return "newNiceName(" + obj + ")";

return null;
}


if (mc.Method.DeclaringType == typeof(DateTime))
{
switch (mc.Method.Name)
Expand Down Expand Up @@ -129,6 +186,23 @@ internal class LambdaToJavascriptConverter
return null;
}

private static string? ToJsOperator(ExpressionType nodeType)
{
return nodeType switch
{
ExpressionType.Add => "+",
ExpressionType.Subtract => "-",
ExpressionType.Equal => "==",
ExpressionType.NotEqual => "!=",
ExpressionType.LessThan => "<",
ExpressionType.LessThanOrEqual => "<",
ExpressionType.GreaterThan => ">",
ExpressionType.GreaterThanOrEqual => ">=",
ExpressionType.Coalesce => "??",
_ => null
};
}

private static string? ToJavascriptToString(ParameterExpression param, Expression expr, string? format = null)
{
var r = ToJavascript(param, expr);
Expand Down
4 changes: 1 addition & 3 deletions Signum.React/Scripts/Reflection.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { DateTime, DateTimeFormatOptions, Duration, DurationObjectUnits, Settings } from 'luxon';
import { Dic } from './Globals';
import { ModifiableEntity, Entity, Lite, MListElement, ModelState, MixinEntity } from './Signum.Entities'; //ONLY TYPES or Cyclic problems in Webpack!
import type { ModifiableEntity, Entity, Lite, MListElement, ModelState, MixinEntity } from './Signum.Entities'; //ONLY TYPES or Cyclic problems in Webpack!
import { ajaxGet } from './Services';
import { MList } from "./Signum.Entities";
import * as AppContext from './AppContext';
Expand Down Expand Up @@ -1506,8 +1506,6 @@ export interface ISymbol {

let missingSymbols: ISymbol[] = [];



function getMember(key: string): MemberInfo | undefined {

if (!key.contains("."))
Expand Down
Loading

3 comments on commit 644151a

@olmobrutall
Copy link
Collaborator Author

@olmobrutall olmobrutall commented on 644151a Dec 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IsNew and GetType() members supported in ToStringExpression

This commit solves an old limitation of not being able to use .IsNew or .GetType() members (Name, FullName, NiceName. NicePluralName or the new NewNiceName) in LINQ provider.

For normal queries, this methods are not particularly useful, (IsNew returns just false), but they are very useful for defining ToString.

The reasons this methods are particularly tricky is that they behave like constant C# sub-expressions - typically processed early in the LINQ provider using ExpressionCleaner- but they are discovered in a later stage - the QueryBinder- when the expression tree is already a mixture of C# and SQL nodes.

So after two failing attempts to move/generalize ExpressionCleaner, a little bit of code duplication made it work.

Changes in default Entity.ToString()

This is the old ToString defined in the Entity class:

    public override string ToString()
    {
        return BaseToString();
    }

    public string BaseToString()
    {
        return "{0} ({1})".FormatWith(GetType().NiceName(), id.HasValue ? id.ToString() : LiteMessage.New_G.NiceToString().ForGenderAndNumber(this.GetType().GetGender()));
    }

And this is the new one:

    [AutoExpressionField]
    public override string ToString() => As.Expression(() => BaseToString());

    [AutoExpressionField]
    public string BaseToString() => As.Expression(() => IsNew ? GetType().NewNiceName() : GetType().NiceName() + " " + Id);

Of course IsNew is erased-out from your SQL query, that ends up being something like SELECT @p0 + t.Id FROM Ticket with @p0="Ticket", this expression is completely translated to SQL so it can be used for filters and autocomplete.

The new approach has a lot of benefits:

  • Thanks to NiceName/NewNiceName the ToString function is localized to the current user culture, not the the culture of the last user that saved the entity.
  • After the first save, the ToString evaluates already to "Ticket 123" instead of "Ticket (New)".
  • No ToStr column needed by default. You can opt-in in your own Entity by having a ToString without AutoExpressionField or ToStirngExpression for performance reasons.

Also, some changes in LambdaToJavascriptConverter (the class that translate your ToString definition to Javascript for client-side created entities) where needed to allow this new methods.

How to Upgrade

There is no code change needed, but expect the Synchronizer / SQL Migrations to remove some unnecessary columns:

ALTER TABLE auth.ResetPasswordRequest DROP COLUMN ToStr;

ALTER TABLE auth.UserTicket DROP COLUMN ToStr;

ALTER TABLE scheduler.SchedulerTaskExceptionLine DROP COLUMN ToStr;

ALTER TABLE dashboard.CachedQuery DROP COLUMN ToStr;

ALTER TABLE viewLog.ViewLog DROP COLUMN ToStr;

ALTER TABLE scheduler.SystemEventLog DROP COLUMN ToStr;

Enjoy!

@rezanos
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent improvement! 👏 Great benefits 👌

@MehdyKarimpour
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job 👍

Please sign in to comment.