Skip to content

Commit

Permalink
Allow principals and dependents using key generation to be added in a…
Browse files Browse the repository at this point in the history 
…ny Order

See issues #1271 and #1207. The main issue here is that if a dependent entity is added to the context before its principal has been added then the primary key value of the principal has not yet been generated. This means that people need to be careful which order to add things in, which in turn means that AttachGraph would not work in some scenarios,

The fix is to find the value generator for the principal of the relationship (possibly traversing several relationships to do so) and use it to generate a temporary key for the dependent entity. When the principal is finally added fixup will happen and the actual principal value will be propagated down to the dependents. The value generator for the ultimate principal needs to be used (instead of some other generator) so that key conflicts in the generated values do not happen as values propagate down.
  • Loading branch information
@ajcvickers
ajcvickers committed Dec 16, 2014
1 parent 5583857 commit c6bece8
Show file tree
Hide file tree
Showing 19 changed files with 780 additions and 231 deletions.
56 changes: 24 additions & 32 deletions src/EntityFramework.Core/ChangeTracking/NavigationFixer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ public class NavigationFixer : IEntityStateListener, IRelationshipListener
private readonly ClrPropertySetterSource _setterSource;
private readonly ClrPropertyGetterSource _getterSource;
private readonly ClrCollectionAccessorSource _collectionAccessorSource;
private readonly StoreGeneratedValuesFactory _storeGeneratedValuesFactory;
private readonly DbContextService<IModel> _model;
private bool _inFixup;

Expand All @@ -35,19 +34,16 @@ protected NavigationFixer()
[NotNull] ClrPropertyGetterSource getterSource,
[NotNull] ClrPropertySetterSource setterSource,
[NotNull] ClrCollectionAccessorSource collectionAccessorSource,
[NotNull] StoreGeneratedValuesFactory storeGeneratedValuesFactory,
[NotNull] DbContextService<IModel> model)
{
Check.NotNull(getterSource, "getterSource");
Check.NotNull(setterSource, "setterSource");
Check.NotNull(collectionAccessorSource, "collectionAccessorSource");
Check.NotNull(storeGeneratedValuesFactory, "storeGeneratedValuesFactory");
Check.NotNull(model, "model");

_getterSource = getterSource;
_setterSource = setterSource;
_collectionAccessorSource = collectionAccessorSource;
_storeGeneratedValuesFactory = storeGeneratedValuesFactory;
_model = model;
}

Expand Down Expand Up @@ -111,7 +107,7 @@ private void NavigationReferenceChangedAction(StateEntry entry, INavigation navi
{
if (newValue != null)
{
SetForeignKeyValue(entry, dependentProperties, entry.StateManager.GetOrCreateEntry(newValue), principalProperties);
SetForeignKeyValue(foreignKey, entry, entry.StateManager.GetOrCreateEntry(newValue));
}
else
{
Expand All @@ -124,7 +120,7 @@ private void NavigationReferenceChangedAction(StateEntry entry, INavigation navi

if (newValue != null)
{
SetForeignKeyValue(entry.StateManager.GetOrCreateEntry(newValue), dependentProperties, entry, principalProperties);
SetForeignKeyValue(foreignKey, entry.StateManager.GetOrCreateEntry(newValue), entry);
}

if (oldValue != null)
Expand Down Expand Up @@ -159,8 +155,9 @@ public virtual void NavigationCollectionChanged(StateEntry entry, INavigation na
{
Debug.Assert(navigation.IsCollection());

var principalProperties = navigation.ForeignKey.ReferencedProperties;
var dependentProperties = navigation.ForeignKey.Properties;
var principalValues = navigation.ForeignKey.ReferencedProperties.Select(p => entry[p]).ToList();
var principalValues = principalProperties.Select(p => entry[p]).ToList();

// TODO: What if the entity is not yet being tracked?
// Issue #323
Expand All @@ -172,7 +169,7 @@ public virtual void NavigationCollectionChanged(StateEntry entry, INavigation na

foreach (var entity in added)
{
SetForeignKeyValue(entry.StateManager.GetOrCreateEntry(entity), dependentProperties, principalValues);
SetForeignKeyValue(navigation.ForeignKey, entry.StateManager.GetOrCreateEntry(entity), principalValues);
SetInverse(entry, navigation, entity);
}
}
Expand All @@ -182,11 +179,9 @@ public virtual void PrincipalKeyPropertyChanged(StateEntry entry, IProperty prop
Check.NotNull(entry, "entry");
Check.NotNull(property, "property");

PerformFixup(() => PrincipalKeyPropertyChangedAction(entry, property, oldValue, newValue));
}
// We don't prevent recursive entry here because changed of principal key can have cascading effects
// when principal key is also foreign key.

private void PrincipalKeyPropertyChangedAction(StateEntry entry, IProperty property, object oldValue, object newValue)
{
foreach (var foreignKey in _model.Service.EntityTypes.SelectMany(
e => e.ForeignKeys.Where(f => f.ReferencedProperties.Contains(property))))
{
Expand All @@ -197,11 +192,7 @@ private void PrincipalKeyPropertyChangedAction(StateEntry entry, IProperty prope
e => e.EntityType == foreignKey.EntityType
&& oldKey.Equals(e.GetDependentKeyValue(foreignKey))).ToList())
{
if (dependent.TryGetSidecar(Sidecar.WellKnownNames.StoreGeneratedValues) == null)
{
dependent.AddSidecar(_storeGeneratedValuesFactory.Create(dependent));
}
SetForeignKeyValue(dependent, foreignKey.Properties, newKeyValues);
SetForeignKeyValue(foreignKey, dependent, newKeyValues);
}
}
}
Expand Down Expand Up @@ -267,9 +258,8 @@ private void InitialFixup(StateEntry entry, EntityState oldState)

var navigationEntityType = navigation.EntityType;

for (var stateEntryIterator = 0; stateEntryIterator < stateEntries.Count; ++stateEntryIterator)
foreach (var relatedEntry in stateEntries)
{
var relatedEntry = stateEntries[stateEntryIterator];
if (relatedEntry.EntityType != navigationEntityType
|| relatedEntry == entry)
{
Expand Down Expand Up @@ -444,24 +434,26 @@ private void StealReference(IForeignKey foreignKey, StateEntry dependentEntry)
}

private static void SetForeignKeyValue(
StateEntry dependentEntry, IReadOnlyList<IProperty> dependentProperties,
StateEntry principalEntry, IReadOnlyList<IProperty> principalProperties)
IForeignKey foreignKey,
StateEntry dependentEntry,
StateEntry principalEntry)
{
Debug.Assert(principalProperties.Count == dependentProperties.Count);

SetForeignKeyValue(dependentEntry, dependentProperties, principalProperties.Select(p => principalEntry[p]).ToList());
SetForeignKeyValue(foreignKey, dependentEntry, foreignKey.ReferencedProperties.Select(p => principalEntry[p]).ToList());
}

private static void SetForeignKeyValue(
StateEntry dependentEntry, IReadOnlyList<IProperty> dependentProperties, IReadOnlyList<object> principalValues)
private static void SetForeignKeyValue(IForeignKey foreignKey, StateEntry dependentEntry, IReadOnlyList<object> principalValues)
{
for (var i = 0; i < dependentProperties.Count; i++)
for (var i = 0; i < foreignKey.Properties.Count; i++)
{
// TODO: Consider nullable/non-nullable assignment issues
// Issue #740
var dependentProperty = dependentProperties[i];
dependentEntry[dependentProperty] = principalValues[i];
dependentEntry.RelationshipsSnapshot.TakeSnapshot(dependentProperty);
if (!foreignKey.FindRootValueGenerationProperty(i).GenerateValueOnAdd
|| !foreignKey.ReferencedProperties[i].PropertyType.IsDefaultValue(principalValues[i]))
{
// TODO: Consider nullable/non-nullable assignment issues
// Issue #740
var dependentProperty = foreignKey.Properties[i];
dependentEntry[dependentProperty] = principalValues[i];
dependentEntry.RelationshipsSnapshot.TakeSnapshot(dependentProperty);
}
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/EntityFramework.Core/ChangeTracking/PropertyBagEntryExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ public static bool HasDefaultValue([NotNull] this IPropertyBagEntry entry, [NotN
Check.NotNull(entry, "entry");
Check.NotNull(property, "property");

var value = entry[property];
return value == null || value.Equals(property.PropertyType.GetDefaultValue());
return property.PropertyType.IsDefaultValue(entry[property]);
}
}
}
8 changes: 4 additions & 4 deletions src/EntityFramework.Core/ChangeTracking/StateEntry.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,10 @@ public virtual void AutoCommitSidecars()

public virtual StateEntry PrepareToSave()
{
if (EntityType.Properties.Any(NeedsStoreValue))
{
AddSidecar(_metadataServices.CreateStoreGeneratedValues(this));
}
// TODO: Issue #1303
//if (EntityType.Properties.Any(NeedsStoreValue))

AddSidecar(_metadataServices.CreateStoreGeneratedValues(this));

return this;
}
Expand Down
9 changes: 3 additions & 6 deletions src/EntityFramework.Core/ChangeTracking/ValueGenerationManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ public virtual void Generate([NotNull] StateEntry entry)
}
else
{
var valueGenerator = _valueGeneratorCache.Service.GetGenerator(property);
var generatedValue = valueGenerator == null
? null
: valueGenerator.Next(property, _dataStoreServices);
var generatedValue = _valueGeneratorCache.Service.GetGenerator(property)?.Next(property, _dataStoreServices);

SetGeneratedValue(entry, generatedValue, property);
}
Expand All @@ -73,14 +70,14 @@ public virtual async Task GenerateAsync([NotNull] StateEntry entry, Cancellation
{
if (isForeignKey)
{
_foreignKeyValuePropagator.PropagateValue(entry, property);
await _foreignKeyValuePropagator.PropagateValueAsync(entry, property, cancellationToken).WithCurrentCulture();
}
else
{
var valueGenerator = _valueGeneratorCache.Service.GetGenerator(property);
var generatedValue = valueGenerator == null
? null
: await valueGenerator.NextAsync(property, _dataStoreServices, cancellationToken);
: await valueGenerator.NextAsync(property, _dataStoreServices, cancellationToken).WithCurrentCulture();

SetGeneratedValue(entry, generatedValue, property);
}
Expand Down
2 changes: 1 addition & 1 deletion src/EntityFramework.Core/Extensions/EntityServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ public static class EntityServiceCollectionExtensions
.AddSingleton<CollectionTypeFactory>()
.AddSingleton<EntityMaterializerSource>()
.AddSingleton<CompositeEntityKeyFactory>()
.AddSingleton<ForeignKeyValuePropagator>()
.AddSingleton<MemberMapper>()
.AddSingleton<FieldMatcher>()
.AddSingleton<OriginalValuesFactory>()
.AddSingleton<RelationshipsSnapshotFactory>()
.AddSingleton<StoreGeneratedValuesFactory>()
.AddSingleton<ValueGeneratorSelector>()
.AddSingleton<StateEntryMetadataServices>()
.AddScoped<ForeignKeyValuePropagator>()
.AddScoped<DataStoreSelector>()
.AddScoped<StateEntryFactory>()
.AddScoped<NavigationFixer>()
Expand Down
90 changes: 88 additions & 2 deletions src/EntityFramework.Core/Identity/ForeignKeyValuePropagator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using JetBrains.Annotations;
using Microsoft.Data.Entity.ChangeTracking;
using Microsoft.Data.Entity.Infrastructure;
using Microsoft.Data.Entity.Metadata;
using Microsoft.Data.Entity.Storage;
using Microsoft.Data.Entity.Utilities;

namespace Microsoft.Data.Entity.Identity
Expand All @@ -15,6 +19,8 @@ public class ForeignKeyValuePropagator
{
private readonly ClrPropertyGetterSource _getterSource;
private readonly ClrCollectionAccessorSource _collectionAccessorSource;
private readonly DbContextService<ValueGeneratorCache> _valueGeneratorCache;
private readonly DbContextService<DataStoreServices> _storeServices;

/// <summary>
/// This constructor is intended only for use when creating test doubles that will override members
Expand All @@ -27,13 +33,19 @@ protected ForeignKeyValuePropagator()

public ForeignKeyValuePropagator(
[NotNull] ClrPropertyGetterSource getterSource,
[NotNull] ClrCollectionAccessorSource collectionAccessorSource)
[NotNull] ClrCollectionAccessorSource collectionAccessorSource,
[NotNull] DbContextService<ValueGeneratorCache> valueGeneratorCache,
[NotNull] DbContextService<DataStoreServices> storeServices)
{
Check.NotNull(getterSource, "getterSource");
Check.NotNull(collectionAccessorSource, "collectionAccessorSource");
Check.NotNull(valueGeneratorCache, "valueGeneratorCache");
Check.NotNull(storeServices, "storeServices");

_getterSource = getterSource;
_collectionAccessorSource = collectionAccessorSource;
_valueGeneratorCache = valueGeneratorCache;
_storeServices = storeServices;
}

public virtual void PropagateValue([NotNull] StateEntry stateEntry, [NotNull] IProperty property)
Expand All @@ -43,6 +55,43 @@ public virtual void PropagateValue([NotNull] StateEntry stateEntry, [NotNull] IP

Debug.Assert(property.IsForeignKey());

if (!TryPropagateValue(stateEntry, property)
&& property.IsKey())
{
var valueGenerator = TryGetValueGenerator(stateEntry, property);

if (valueGenerator != null)
{
stateEntry[property] = valueGenerator.Next(property, _storeServices).Value;
}
}
}

public virtual async Task PropagateValueAsync(
[NotNull] StateEntry stateEntry,
[NotNull] IProperty property,
CancellationToken cancellationToken = default(CancellationToken))
{
Check.NotNull(stateEntry, "stateEntry");
Check.NotNull(property, "property");

Debug.Assert(property.IsForeignKey());

if (!TryPropagateValue(stateEntry, property)
&& property.IsKey())
{
var valueGenerator = TryGetValueGenerator(stateEntry, property);

if (valueGenerator != null)
{
stateEntry[property] =
(await valueGenerator.NextAsync(property, _storeServices, cancellationToken).WithCurrentCulture()).Value;
}
}
}

private bool TryPropagateValue(StateEntry stateEntry, IProperty property)
{
var entityType = property.EntityType;
var stateManager = stateEntry.StateManager;

Expand All @@ -52,6 +101,8 @@ public virtual void PropagateValue([NotNull] StateEntry stateEntry, [NotNull] IP
{
if (property == foreignKey.Properties[propertyIndex])
{
object valueToPropagte = null;

foreach (var navigation in entityType.Navigations
.Concat(foreignKey.ReferencedEntityType.Navigations)
.Where(n => n.ForeignKey == foreignKey)
Expand All @@ -62,13 +113,48 @@ public virtual void PropagateValue([NotNull] StateEntry stateEntry, [NotNull] IP
if (principal != null)
{
var principalEntry = stateManager.GetOrCreateEntry(principal);
var principalProperty = foreignKey.ReferencedProperties[propertyIndex];

stateEntry[property] = principalEntry[foreignKey.ReferencedProperties[propertyIndex]];
var principalValue = principalEntry[principalProperty];
if (!principalProperty.PropertyType.IsDefaultValue(principalValue))
{
valueToPropagte = principalValue;
break;
}
}
}

if (valueToPropagte != null)
{
stateEntry[property] = valueToPropagte;
return true;
}
}
}
}

return false;
}

private IValueGenerator TryGetValueGenerator(StateEntry stateEntry, IProperty property)
{
foreach (var foreignKey in property.EntityType.ForeignKeys)
{
for (var propertyIndex = 0; propertyIndex < foreignKey.Properties.Count; propertyIndex++)
{
if (property == foreignKey.Properties[propertyIndex]
&& property.IsKey())
{
var generationProperty = foreignKey.FindRootValueGenerationProperty(propertyIndex);

if (generationProperty.GenerateValueOnAdd)
{
return _valueGeneratorCache.Service.GetGenerator(generationProperty);
}
}
}
}
return null;
}

private object TryFindPrincipal(StateManager stateManager, INavigation navigation, object dependentEntity)
Expand Down
6 changes: 0 additions & 6 deletions src/EntityFramework.Core/Identity/ValueGeneratorCache.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,6 @@ public virtual IValueGenerator GetGenerator([NotNull] IProperty property)
Check.NotNull(property, "property");

var factory = _selector.Select(property);

if (factory == null)
{
return null;
}

var pool = _cache.GetOrAdd(factory.GetCacheKey(property), k => CreatePool(property, factory));

return pool.GetGenerator();
Expand Down
10 changes: 0 additions & 10 deletions src/EntityFramework.Core/Identity/ValueGeneratorSelector.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,6 @@ public virtual IValueGeneratorFactory Select([NotNull] IProperty property)
{
Check.NotNull(property, "property");

if (!property.GenerateValueOnAdd)
{
return null;
}

var propertyType = property.PropertyType;

if (propertyType == typeof(Guid))
Expand All @@ -73,10 +68,5 @@ public virtual IValueGeneratorFactory Select([NotNull] IProperty property)
throw new NotSupportedException(
Strings.NoValueGenerator(property.Name, property.EntityType.SimpleName, propertyType.Name));
}

public virtual SimpleValueGeneratorFactory<GuidValueGenerator> GuidFactory
{
get { return _guidFactory; }
}
}
}

0 comments on commit c6bece8

Please sign in to comment.