Skip to content

Commit

Permalink
Fix to #19900 - Query/Test: add automatic null propagation for AsserQ…
Browse files Browse the repository at this point in the history
…uery baselines (part1)

Added visitor that injects Maybe methods for all non-scalar member accesses.
Also refactored Maybe methods to be more composible and so that when used manually, they don't require type arguments to be specified.
  • Loading branch information
maumar committed Feb 13, 2020
1 parent 812e241 commit 49fddb7
Show file tree
Hide file tree
Showing 13 changed files with 455 additions and 1,082 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.TestModels.NullSemanticsModel;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

// ReSharper disable SimplifyConditionalTernaryExpression
Expand Down Expand Up @@ -849,7 +850,7 @@ public virtual Task Select_IndexOf(bool async)
return AssertQueryScalar(
async,
ss => ss.Set<NullSemanticsEntity1>().OrderBy(e => e.Id).Select(e => e.NullableStringA.IndexOf("oo")),
ss => ss.Set<NullSemanticsEntity1>().OrderBy(e => e.Id).Select(e => MaybeScalar<int>(e.NullableStringA, () => e.NullableStringA.IndexOf("oo")) ?? 0),
ss => ss.Set<NullSemanticsEntity1>().OrderBy(e => e.Id).Select(e => e.NullableStringA.MaybeScalar(x => x.IndexOf("oo")) ?? 0),
assertOrder: true);
}

Expand All @@ -861,25 +862,22 @@ await AssertQueryScalar(
async,
ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableStringA.IndexOf("oo") == e.NullableStringB.IndexOf("ar")).Select(e => e.Id),
ss => ss.Set<NullSemanticsEntity1>().Where(
e => MaybeScalar<int>(e.NullableStringA, () => e.NullableStringA.IndexOf("oo"))
== MaybeScalar<int>(
e.NullableStringB, () => e.NullableStringB.IndexOf("ar"))).Select(e => e.Id));
e => e.NullableStringA.MaybeScalar(x => x.IndexOf("oo"))
== e.NullableStringB.MaybeScalar(x => x.IndexOf("ar"))).Select(e => e.Id));

await AssertQueryScalar(
async,
ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableStringA.IndexOf("oo") != e.NullableStringB.IndexOf("ar")).Select(e => e.Id),
ss => ss.Set<NullSemanticsEntity1>().Where(
e => MaybeScalar<int>(e.NullableStringA, () => e.NullableStringA.IndexOf("oo"))
!= MaybeScalar<int>(
e.NullableStringB, () => e.NullableStringB.IndexOf("ar"))).Select(e => e.Id));
e => e.NullableStringA.MaybeScalar(x => x.IndexOf("oo"))
!= e.NullableStringB.MaybeScalar(x => x.IndexOf("ar"))).Select(e => e.Id));

await AssertQueryScalar(
async,
ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableStringA.IndexOf("oo") != e.NullableStringA.IndexOf("ar")).Select(e => e.Id),
ss => ss.Set<NullSemanticsEntity1>().Where(
e => MaybeScalar<int>(e.NullableStringA, () => e.NullableStringA.IndexOf("oo"))
!= MaybeScalar<int>(
e.NullableStringA, () => e.NullableStringA.IndexOf("ar"))).Select(e => e.Id));
e => e.NullableStringA.MaybeScalar(x => x.IndexOf("oo"))
!= e.NullableStringA.MaybeScalar(x => x.IndexOf("ar"))).Select(e => e.Id));
}

[ConditionalTheory]
Expand Down Expand Up @@ -940,7 +938,7 @@ public virtual Task Null_semantics_function(bool async)
return AssertQueryScalar(
async,
ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableStringA.Substring(0, e.IntA) != e.NullableStringB).Select(e => e.Id),
ss => ss.Set<NullSemanticsEntity1>().Where(e => Maybe(e.NullableIntA, () => e.NullableStringA.Substring(0, e.IntA)) != e.NullableStringB).Select(e => e.Id));
ss => ss.Set<NullSemanticsEntity1>().Where(e => e.NullableStringA.Maybe(x => x.Substring(0, e.IntA)) != e.NullableStringB).Select(e => e.Id));
}

[ConditionalTheory]
Expand Down
916 changes: 232 additions & 684 deletions test/EFCore.Specification.Tests/Query/ComplexNavigationsQueryTestBase.cs

Large diffs are not rendered by default.

111 changes: 46 additions & 65 deletions test/EFCore.Specification.Tests/Query/FunkyDataQueryTestBase.cs

Large diffs are not rendered by default.

327 changes: 70 additions & 257 deletions test/EFCore.Specification.Tests/Query/GearsOfWarQueryTestBase.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -927,12 +927,9 @@ public virtual Task Multiple_collection_navigation_with_FirstOrDefault_chained(b
ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Select(
c => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails.OrderBy(od => od.ProductID).FirstOrDefault()),
ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Select(
c => Maybe(
Maybe(
c.Orders.OrderBy(o => o.OrderID).FirstOrDefault(),
() => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails),
() => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails.OrderBy(od => od.ProductID)
.FirstOrDefault())));
c => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault()
.Maybe(x => x.OrderDetails)
.Maybe(xx => xx.OrderBy(od => od.ProductID).FirstOrDefault())));
}

[ConditionalTheory]
Expand All @@ -945,12 +942,9 @@ public virtual Task Multiple_collection_navigation_with_FirstOrDefault_chained_p
c => (int?)c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails.OrderBy(od => od.ProductID).FirstOrDefault()
.ProductID),
ss => ss.Set<Customer>().OrderBy(c => c.CustomerID).Select(
c => MaybeScalar<int>(
Maybe(
c.Orders.OrderBy(o => o.OrderID).FirstOrDefault(),
() => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails),
() => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault().OrderDetails.OrderBy(od => od.ProductID).FirstOrDefault()
.ProductID)));
c => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault()
.Maybe(x => x.OrderDetails)
.MaybeScalar(x => x.OrderBy(od => od.ProductID).FirstOrDefault().ProductID)));
}

[ConditionalTheory]
Expand Down Expand Up @@ -1785,11 +1779,7 @@ public virtual Task Collection_Last_member_access_in_projection_translated(bool
ss => ss.Set<Customer>().Where(c => c.CustomerID.StartsWith("F"))
.Where(c => c.Orders.OrderByDescending(o => o.OrderID).Last().CustomerID == c.CustomerID),
ss => ss.Set<Customer>().Where(c => c.CustomerID.StartsWith("F"))
.Where(
c => Maybe(
c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault(),
() => c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault().CustomerID)
== c.CustomerID),
.Where(c => c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault().Maybe(x => x.CustomerID) == c.CustomerID),
entryCount: 7);
}

Expand All @@ -1802,11 +1792,7 @@ public virtual Task Collection_LastOrDefault_member_access_in_projection_transla
ss => ss.Set<Customer>().Where(c => c.CustomerID.StartsWith("F"))
.Where(c => c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault().CustomerID == c.CustomerID),
ss => ss.Set<Customer>().Where(c => c.CustomerID.StartsWith("F"))
.Where(
c => Maybe(
c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault(),
() => c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault().CustomerID)
== c.CustomerID),
.Where(c => c.Orders.OrderByDescending(o => o.OrderID).LastOrDefault().Maybe(x => x.CustomerID) == c.CustomerID),
entryCount: 7);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,6 @@ public virtual Task Select_Where_Navigation_Null_Deep(bool async)
ss => from e in ss.Set<Employee>()
where e.Manager.Manager == null
select e,
ss => from e in ss.Set<Employee>()
where Maybe(e.Manager, () => e.Manager.Manager) == null
select e,
entryCount: 6);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1669,7 +1669,7 @@ public virtual Task Project_uint_through_collection_FirstOrDefault(bool async)
async,
ss => ss.Set<Customer>().Select(c => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault()).Select(e => e.EmployeeID),
ss => ss.Set<Customer>().Select(c => c.Orders.OrderBy(o => o.OrderID).FirstOrDefault())
.Select(e => MaybeScalar(e, () => e.EmployeeID)));
.Select(e => e.MaybeScalar(x => x.EmployeeID)));
}

[ConditionalTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -794,11 +794,13 @@ public virtual async Task Where_equals_on_null_nullable_int_types(bool async)
await AssertQuery(
async,
ss => ss.Set<Employee>().Where(e => nullableIntPrm.Equals(e.ReportsTo)),
ss => ss.Set<Employee>().Where(e => e.ReportsTo == nullableIntPrm),
entryCount: 1);

await AssertQuery(
async,
ss => ss.Set<Employee>().Where(e => e.ReportsTo.Equals(nullableIntPrm)),
ss => ss.Set<Employee>().Where(e => e.ReportsTo == nullableIntPrm),
entryCount: 1);
}

Expand Down
20 changes: 0 additions & 20 deletions test/EFCore.Specification.Tests/Query/QueryTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1130,26 +1130,6 @@ protected void AssertGrouping<TKey, TElement>(
AssertCollection(expected, actual, ordered, elementSorter, elementAsserter);
}

protected static TResult Maybe<TResult>(object caller, Func<TResult> expression)
where TResult : class
{
return caller == null ? null : expression();
}

protected static TResult? MaybeScalar<TResult>(object caller, Func<TResult?> expression)
where TResult : struct
{
return caller == null ? null : expression();
}

protected static IEnumerable<TResult> MaybeDefaultIfEmpty<TResult>(IEnumerable<TResult> caller)
where TResult : class
{
return caller == null
? new List<TResult> { default }
: caller.DefaultIfEmpty();
}

protected static async Task AssertTranslationFailed(Func<Task> query)
=> Assert.Contains(
CoreStrings.TranslationFailed("").Substring(21),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Linq.Expressions;
using System.Reflection;

namespace Microsoft.EntityFrameworkCore.TestUtilities
{
public class ExpectedQueryRewritingVisitor : ExpressionVisitor
{
private static readonly MethodInfo _maybeMethod
= typeof(QueryTestExtensions).GetMethod(nameof(QueryTestExtensions.Maybe));

protected override Expression VisitMember(MemberExpression memberExpression)
{
if (!memberExpression.Type.IsValueType
&& !memberExpression.Type.IsNullableValueType()
&& memberExpression.Expression != null)
{
var expression = Visit(memberExpression.Expression);

var lambdaParameter = Expression.Parameter(expression.Type, "x");
var lambda = Expression.Lambda(memberExpression.Update(lambdaParameter), lambdaParameter);
var method = _maybeMethod.MakeGenericMethod(expression.Type, memberExpression.Type);

return Expression.Call(method, expression, lambda);
}

return base.VisitMember(memberExpression);
}
}
}
36 changes: 23 additions & 13 deletions test/EFCore.Specification.Tests/TestUtilities/QueryAsserter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ public override async Task AssertSingleResultTyped<TResult>(
Assert.Equal(entryCount, context.ChangeTracker.Entries().Count());
}

private IList<TResult> GetExpectedResults<TResult>(Func<ISetSource, IQueryable<TResult>> expectedQueryFunc)
{
var expectedQuery = expectedQueryFunc(ExpectedData);
var expectedQueryExpression = expectedQuery.Expression;
var rewrittenExpectedQueryExpression = new ExpectedQueryRewritingVisitor().Visit(expectedQueryExpression);
var newExpectedQuery = expectedQuery.Provider.CreateQuery<TResult>(rewrittenExpectedQueryExpression);

return newExpectedQuery.ToList();
}

public override async Task AssertQuery<TResult>(
Func<ISetSource, IQueryable<TResult>> actualQuery,
Func<ISetSource, IQueryable<TResult>> expectedQuery,
Expand All @@ -89,12 +99,12 @@ public override async Task AssertQuery<TResult>(
OrderingSettingsVerifier(assertOrder, query.Expression.Type, elementSorter);

var actual = async
? await query.ToArrayAsync()
: query.ToArray();
? await query.ToListAsync()
: query.ToList();

AssertRogueExecution(actual.Length, query);
AssertRogueExecution(actual.Count, query);

var expected = expectedQuery(ExpectedData).ToArray();
var expected = GetExpectedResults(expectedQuery);

if (!assertOrder
&& elementSorter == null)
Expand Down Expand Up @@ -159,12 +169,12 @@ public override async Task AssertQueryScalar<TResult>(
OrderingSettingsVerifier(assertOrder, query.Expression.Type);

var actual = async
? await query.ToArrayAsync()
: query.ToArray();
? await query.ToListAsync()
: query.ToList();

AssertRogueExecution(actual.Length, query);
AssertRogueExecution(actual.Count, query);

var expected = expectedQuery(ExpectedData).ToArray();
var expected = GetExpectedResults(expectedQuery);

TestHelpers.AssertResults(
expected,
Expand Down Expand Up @@ -194,12 +204,12 @@ public override async Task AssertQueryScalar<TResult>(
OrderingSettingsVerifier(assertOrder, query.Expression.Type);

var actual = async
? await query.ToArrayAsync()
: query.ToArray();
? await query.ToListAsync()
: query.ToList();

AssertRogueExecution(actual.Length, query);
AssertRogueExecution(actual.Count, query);

var expected = expectedQuery(ExpectedData).ToArray();
var expected = GetExpectedResults(expectedQuery);

TestHelpers.AssertResults(
expected,
Expand Down Expand Up @@ -237,7 +247,7 @@ public override async Task<List<TResult>> AssertIncludeQuery<TResult>(

AssertRogueExecution(actual.Count, query);

var expected = expectedQuery(ExpectedData).ToList();
var expected = GetExpectedResults(expectedQuery);

if (!assertOrder
&& elementSorter == null)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.EntityFrameworkCore.TestUtilities
{
public static class QueryTestExtensions
{
public static TResult Maybe<TSource, TResult>(this TSource caller, Func<TSource, TResult> result)
where TResult : class
=> caller == null ? null : result(caller);

public static TResult? MaybeScalar<TSource, TResult>(this TSource caller, Func<TSource, TResult> result)
where TResult : struct
=> caller != null ? (TResult?)result(caller) : null;

public static TResult? MaybeScalar<TSource, TResult>(this TSource caller, Func<TSource, TResult?> result)
where TResult : struct
=> caller != null ? result(caller) : null;

public static IEnumerable<TResult> MaybeDefaultIfEmpty<TResult>(this IEnumerable<TResult> caller)
where TResult : class
=> caller == null ? new List<TResult> { default } : caller.DefaultIfEmpty();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6331,20 +6331,17 @@ FROM [Gears] AS [g]
WHERE [g].[Discriminator] IN (N'Gear', N'Officer') AND ([g].[Nickname] <> N'Dom')");
}

public override async Task
Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation(bool async)
public override async Task Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation(bool async)
{
await base.Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation(async);

AssertSql(
@"");
}

public override async Task
Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(bool async)
public override async Task Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(bool async)
{
await base.Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(
async);
await base.Null_semantics_is_correctly_applied_for_function_comparisons_that_take_arguments_from_optional_navigation_complex(async);

AssertSql(
@"SELECT [t].[Id], [t].[GearNickName], [t].[GearSquadId], [t].[Note]
Expand Down

0 comments on commit 49fddb7

Please sign in to comment.