Skip to content

Commit

Permalink
Don't persist keys for embedded entities
Browse files Browse the repository at this point in the history
  • Loading branch information
AndriySvyryd committed Jul 26, 2019
1 parent 03a48a5 commit 0eaa5b2
Show file tree
Hide file tree
Showing 14 changed files with 205 additions and 58 deletions.
23 changes: 22 additions & 1 deletion src/EFCore.Cosmos/Extensions/CosmosPropertyExtensions.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// 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.Linq;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
Expand All @@ -20,7 +21,27 @@ public static class CosmosPropertyExtensions
/// <returns> The property name used when targeting Cosmos. </returns>
public static string GetCosmosPropertyName([NotNull] this IProperty property) =>
(string)property[CosmosAnnotationNames.PropertyName]
?? property.Name;
?? GetDefaultPropertyName(property);

private static string GetDefaultPropertyName(IProperty property)
{
var entityType = property.DeclaringEntityType;
var ownership = entityType.FindOwnership();

if (ownership != null
&& !entityType.IsDocumentRoot())
{
var pk = property.FindContainingPrimaryKey();
if (pk != null
&& pk.Properties.Count == ownership.Properties.Count + (ownership.IsUnique ? 0 : 1)
&& ownership.Properties.All(fkProperty => pk.Properties.Contains(fkProperty)))
{
return "";
}
}

return property.Name;
}

/// <summary>
/// Sets the property name used when targeting Cosmos.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Cosmos.Query.Internal;
using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
using Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration.Internal;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Query;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.EntityFrameworkCore.ValueGeneration;

// ReSharper disable once CheckNamespace
namespace Microsoft.Extensions.DependencyInjection
Expand Down Expand Up @@ -53,6 +55,7 @@ public static IServiceCollection AddEntityFrameworkCosmos([NotNull] this IServic
.TryAdd<IDbContextTransactionManager, CosmosTransactionManager>()
.TryAdd<IModelValidator, CosmosModelValidator>()
.TryAdd<IProviderConventionSetBuilder, CosmosConventionSetBuilder>()
.TryAdd<IValueGeneratorSelector, CosmosValueGeneratorSelector>()
.TryAdd<IDatabaseCreator, CosmosDatabaseCreator>()
.TryAdd<IQueryContextFactory, CosmosQueryContextFactory>()
.TryAdd<ITypeMappingSource, CosmosTypeMappingSource>()
Expand Down

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions src/EFCore.Cosmos/Query/Internal/ObjectAccessExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ public ObjectAccessExpression(INavigation navigation, Expression accessExpressio
AccessExpression = accessExpression;
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public override ExpressionType NodeType => ExpressionType.Extension;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
21 changes: 19 additions & 2 deletions src/EFCore.Cosmos/Storage/Internal/CosmosDatabaseCreator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Threading;
using System.Threading.Tasks;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Storage;
Expand Down Expand Up @@ -55,7 +56,7 @@ public virtual bool EnsureCreated()
{
created |= _cosmosClient.CreateContainerIfNotExists(
entityType.GetCosmosContainer(),
entityType.GetCosmosPartitionKeyStoreName());
GetCosmosPartitionKeyStoreName(entityType));
}

if (created)
Expand Down Expand Up @@ -89,7 +90,7 @@ public virtual async Task<bool> EnsureCreatedAsync(CancellationToken cancellatio
{
created |= await _cosmosClient.CreateContainerIfNotExistsAsync(
entityType.GetCosmosContainer(),
entityType.GetCosmosPartitionKeyStoreName(),
GetCosmosPartitionKeyStoreName(entityType),
cancellationToken);
}

Expand Down Expand Up @@ -145,5 +146,21 @@ public virtual bool CanConnect()
/// </summary>
public virtual Task<bool> CanConnectAsync(CancellationToken cancellationToken = default)
=> throw new NotImplementedException();

/// <summary>
/// Returns the store name of the property that is used to store the partition key.
/// </summary>
/// <param name="entityType"> The entity type to get the partition key property name for. </param>
/// <returns> The name of the partition key property. </returns>
private static string GetCosmosPartitionKeyStoreName([NotNull] IEntityType entityType)
{
var name = entityType.GetCosmosPartitionKeyPropertyName();
if (name != null)
{
return entityType.FindProperty(name).GetCosmosPropertyName();
}

return CosmosClientWrapper.DefaultPartitionKey;
}
}
}
8 changes: 8 additions & 0 deletions src/EFCore.Cosmos/Update/Internal/DocumentSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ public virtual JObject CreateDocument(IUpdateEntry entry)
{
document[storeName] = ConvertPropertyValue(property, entry.GetCurrentValue(property));
}
else if (entry.HasTemporaryValue(property))
{
((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property);
}
}

foreach (var ownedNavigation in entry.EntityType.GetNavigations())
Expand Down Expand Up @@ -130,6 +134,10 @@ public virtual JObject UpdateDocument(JObject document, IUpdateEntry entry)
document[storeName] = ConvertPropertyValue(property, entry.GetCurrentValue(property));
anyPropertyUpdated = true;
}
else if (entry.HasTemporaryValue(property))
{
((InternalEntityEntry)entry)[property] = entry.GetCurrentValue(property);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// 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 Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.ValueGeneration;
using Microsoft.EntityFrameworkCore.ValueGeneration.Internal;

namespace Microsoft.EntityFrameworkCore.Cosmos.ValueGeneration.Internal
{
public class CosmosValueGeneratorSelector : ValueGeneratorSelector
{
public CosmosValueGeneratorSelector(ValueGeneratorSelectorDependencies dependencies)
: base(dependencies)
{
}

public override ValueGenerator Create(IProperty property, IEntityType entityType)
{
var type = property.ClrType.UnwrapNullableType().UnwrapEnumType();

if (property.GetCosmosPropertyName() == ""
&& type == typeof(int))
{
return new TemporaryIntValueGenerator();
}

return base.Create(property, entityType);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System.Collections;
using System.Linq;
using System.Text;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.ChangeTracking;
using Microsoft.EntityFrameworkCore.Cosmos.Storage.Internal;
using Microsoft.EntityFrameworkCore.ValueGeneration;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ private class FromSqlParameterApplyingExpressionVisitor : ExpressionVisitor
private readonly IDictionary<FromSqlExpression, Expression> _visitedFromSqlExpressions
= new Dictionary<FromSqlExpression, Expression>(ReferenceEqualityComparer.Instance);

private readonly ISqlExpressionFactory _SqlExpressionFactory;
private readonly ISqlExpressionFactory _sqlExpressionFactory;
private readonly ParameterNameGenerator _parameterNameGenerator;
private readonly IReadOnlyDictionary<string, object> _parametersValues;

public FromSqlParameterApplyingExpressionVisitor(
ISqlExpressionFactory _sqlExpressionFactory,
ISqlExpressionFactory sqlExpressionFactory,
ParameterNameGenerator parameterNameGenerator,
IReadOnlyDictionary<string, object> parametersValues)
{
_SqlExpressionFactory = _sqlExpressionFactory;
_sqlExpressionFactory = sqlExpressionFactory;
_parameterNameGenerator = parameterNameGenerator;
_parametersValues = parametersValues;
}
Expand Down Expand Up @@ -67,7 +67,7 @@ public override Expression Visit(Expression expression)
new TypeMappedRelationalParameter(
parameterName,
parameterName,
_SqlExpressionFactory.GetTypeMappingForValue(parameterValues[i]),
_sqlExpressionFactory.GetTypeMappingForValue(parameterValues[i]),
parameterValues[i]?.GetType().IsNullableType()));
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ public SelectExpression Optimize(SelectExpression selectExpression, IReadOnlyDic
_parameterNameGeneratorFactory.Create(),
parametersValues).Visit(query);


return (SelectExpression)query;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
Expand Down Expand Up @@ -86,7 +87,7 @@ public virtual SqlExpression TranslateAverage(Expression expression)
return inputType == typeof(float)
? _sqlExpressionFactory.Convert(
_sqlExpressionFactory.Function(
"AVG", new[] { sqlExpression }, typeof(double), null),
"AVG", new[] { sqlExpression }, typeof(double)),
sqlExpression.Type,
sqlExpression.TypeMapping)
: (SqlExpression)_sqlExpressionFactory.Function(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,7 @@ public static Expression CreateKeyAccessExpression(
AnonymousObject.AnonymousObjectCtor,
Expression.NewArrayInit(
typeof(object),
properties
.Select(p => Expression.Convert(CreatePropertyExpression(target, p, addNullCheck), typeof(object)))
.Cast<Expression>()
.ToArray()));
properties.Select(p => Expression.Convert(CreatePropertyExpression(target, p, addNullCheck), typeof(object)))));

private static Expression CreatePropertyExpression(Expression target, IProperty property, bool addNullCheck)
{
Expand Down
11 changes: 4 additions & 7 deletions test/EFCore.Cosmos.FunctionalTests/NestedDocumentsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ public virtual async Task Can_update_owner_with_dependents()
}
}

// #12086
//[ConditionalFact]
[ConditionalFact]
public virtual async Task Can_add_collection_dependent_to_owner()
{
await using (var testDatabase = CreateTestStore())
Expand Down Expand Up @@ -158,9 +157,8 @@ public virtual async Task Can_add_collection_dependent_to_owner()
var json = context.Entry(people[1]).Property<JObject>("__jObject").CurrentValue;
var jsonAddress = (JObject)((JArray)json["Stored Addresses"])[0];
Assert.Equal("Second", jsonAddress[nameof(Address.Street)]);
// Uncomment when issue #13578 is fixed
//Assert.Equal(2, jsonAddress["unmappedId"]);
//Assert.Equal(2, jsonAddress.Count);
Assert.Equal(2, jsonAddress["unmappedId"]);
Assert.Equal(2, jsonAddress.Count);

addresses = people[2].Addresses.ToList();
Assert.Equal(3, addresses.Count);
Expand Down Expand Up @@ -192,8 +190,7 @@ public virtual async Task Can_query_just_nested_reference()
}
}

// #12086
//[ConditionalFact]
[ConditionalFact]
public virtual async Task Can_query_just_nested_collection()
{
await using (var testDatabase = CreateTestStore())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.EntityFrameworkCore.TestUtilities.Xunit;
using Xunit;
using Xunit.Abstractions;

Expand Down

0 comments on commit 0eaa5b2

Please sign in to comment.