Skip to content

Commit

Permalink
Query: Clean up query validation for various negative cases (#21344)
Browse files Browse the repository at this point in the history
Resolves #15312
  • Loading branch information
smitpatel authored Jun 20, 2020
1 parent 7ba2f47 commit 973af84
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 42 deletions.
22 changes: 3 additions & 19 deletions src/EFCore/Properties/CoreStrings.Designer.cs

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

6 changes: 0 additions & 6 deletions src/EFCore/Properties/CoreStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -534,9 +534,6 @@
<data name="AnnotationNotFound" xml:space="preserve">
<value>The annotation '{annotation}' was not found. Ensure that the annotation has been added.</value>
</data>
<data name="IncludeBadNavigation" xml:space="preserve">
<value>The property '{property}' is not a navigation property of entity type '{entityType}'. The 'Include(string)' method can only be used with a '.' separated list of navigation property names.</value>
</data>
<data name="LogQueryExecutionPlanned" xml:space="preserve">
<value>{plan}</value>
<comment>Debug CoreEventId.QueryExecutionPlanned string</comment>
Expand Down Expand Up @@ -908,9 +905,6 @@
<data name="NonComparableKeyTypes" xml:space="preserve">
<value>Property '{entityType}.{property}' cannot be used as a key because it has type '{modelType}' and provider type '{providerType}' neither of which implement 'IComparable&lt;T&gt;', 'IComparable' or 'IStructuralComparable'. Make '{modelType}' implement one of these interfaces to use it as a key.</value>
</data>
<data name="IncludeNotSpecifiedDirectlyOnEntityType" xml:space="preserve">
<value>The Include operation '{include}' is not supported. '{invalidNavigation}' must be a navigation property defined on an entity type.</value>
</data>
<data name="LogPossibleUnintendedCollectionNavigationNullComparison" xml:space="preserve">
<value>Collection navigations are only considered null if their parent entity is null. Use '.Any()' to check whether collection navigation '{navigationPath}' is empty.</value>
<comment>Warning CoreEventId.PossibleUnintendedCollectionNavigationNullComparisonWarning string</comment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Linq.Expressions;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.TestModels.GearsOfWarModel;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
Expand Down Expand Up @@ -3572,7 +3573,7 @@ public virtual Task Include_on_derived_multi_level(bool async)
elementAsserter: (e, a) => AssertInclude(e, a, expectedIncludes));
}

[ConditionalTheory(Skip = "Issue#15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_collection_and_invalid_navigation_using_string_throws(bool async)
{
Expand All @@ -3581,9 +3582,8 @@ public virtual async Task Include_collection_and_invalid_navigation_using_string
async,
ss => ss.Set<Gear>().Include("Reports.Foo")))).Message;

Assert.Equal(
CoreStrings.IncludeBadNavigation("Foo", "Gear"),
message);
Assert.Contains(CoreResources.LogInvalidIncludePath(new TestLogger<TestLoggingDefinitions>())
.GenerateMessage("Reports.Foo", "Foo"), message);
}

[ConditionalTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,36 +100,36 @@ void TestJsonSerialization(bool useNewtonsoft, bool ignoreLoops, bool writeInden
}
}

[ConditionalTheory(Skip = "issue #15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_property_after_navigation(bool async)
{
Assert.Equal(
CoreStrings.IncludeBadNavigation("CustomerID", nameof(Customer)),
CoreStrings.InvalidLambdaExpressionInsideInclude,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<Order>().Include(o => o.Customer.CustomerID)))).Message);
}

[ConditionalTheory(Skip = "issue #15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_property(bool async)
{
Assert.Equal(
CoreStrings.IncludeBadNavigation("OrderDate", nameof(Order)),
CoreStrings.InvalidLambdaExpressionInsideInclude,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<Order>().Include(o => o.OrderDate)))).Message);
}

[ConditionalTheory(Skip = "issue #15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_property_expression_invalid(bool async)
{
Assert.Equal(
CoreStrings.InvalidIncludeExpression(" "),
CoreStrings.InvalidIncludeExpression("new <>f__AnonymousType358`2(Customer = o.Customer, OrderDetails = o.OrderDetails)"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
Expand All @@ -153,12 +153,12 @@ public virtual Task Then_include_collection_order_by_collection_column(bool asyn
entryCount: 55);
}

[ConditionalTheory(Skip = "issue#15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Then_include_property_expression_invalid(bool async)
{
Assert.Equal(
CoreStrings.InvalidIncludeExpression(" "),
CoreStrings.InvalidIncludeExpression("new <>f__AnonymousType358`2(Customer = o.Customer, OrderDetails = o.OrderDetails)"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
Expand Down Expand Up @@ -1303,12 +1303,12 @@ public virtual Task Include_collection_with_conditional_order_by(bool async)
entryCount: 921);
}
[ConditionalTheory(Skip = "issue #15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_specified_on_non_entity_not_supported(bool async)
{
Assert.Equal(
CoreStrings.IncludeNotSpecifiedDirectlyOnEntityType(@"Include(""Item1.Orders"")", "Item1"),
CoreStrings.IncludeOnNonEntity,
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.EntityFrameworkCore.TestModels.Northwind;
using Xunit;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;

// ReSharper disable InconsistentNaming
// ReSharper disable StringStartsWithIsCultureSpecific
Expand All @@ -29,18 +30,41 @@ protected NorthwindStringIncludeQueryTestBase(TFixture fixture)
{
}

[ConditionalTheory(Skip = "issue #15312")]
[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Include_non_existing_navigation(bool async)
{
Assert.Equal(
CoreStrings.IncludeBadNavigation("ArcticMonkeys", nameof(Order)),
Assert.Contains(
CoreResources.LogInvalidIncludePath(new TestLogger<TestLoggingDefinitions>())
.GenerateMessage("ArcticMonkeys", "ArcticMonkeys"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<Order>().Include("ArcticMonkeys")))).Message);
}

public override async Task Include_property(bool async)
{
Assert.Contains(
CoreResources.LogInvalidIncludePath(new TestLogger<TestLoggingDefinitions>())
.GenerateMessage("OrderDate", "OrderDate"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<Order>().Include(o => o.OrderDate)))).Message);
}

public override async Task Include_property_after_navigation(bool async)
{
Assert.Contains(
CoreResources.LogInvalidIncludePath(new TestLogger<TestLoggingDefinitions>())
.GenerateMessage("Customer.CustomerID", "CustomerID"),
(await Assert.ThrowsAsync<InvalidOperationException>(
() => AssertQuery(
async,
ss => ss.Set<Order>().Include(o => o.Customer.CustomerID)))).Message);
}

// Property expression cannot be converted to string include
public override Task Include_property_expression_invalid(bool async) => Task.CompletedTask;

Expand Down

0 comments on commit 973af84

Please sign in to comment.