Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Indexed Properties no longer Shadow Properties #13926

Merged
merged 1 commit into from
Nov 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/EFCore/ChangeTracking/Internal/InternalEntityEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ public virtual TProperty GetRelationshipSnapshotValue<TProperty>([NotNull] IProp
/// </summary>
protected virtual object ReadPropertyValue([NotNull] IPropertyBase propertyBase)
{
Debug.Assert(propertyBase.IsIndexedProperty || !propertyBase.IsShadowProperty);
Debug.Assert(!propertyBase.IsShadowProperty);

return ((PropertyBase)propertyBase).Getter.GetClrValue(Entity);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ protected override T ReadShadowValue<T>(int shadowIndex)
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
protected override object ReadPropertyValue(IPropertyBase propertyBase)
=> propertyBase.IsIndexedProperty || !propertyBase.IsShadowProperty
=> !propertyBase.IsShadowProperty
? base.ReadPropertyValue(propertyBase)
: _shadowValues[propertyBase.GetShadowIndex()];

Expand Down
15 changes: 15 additions & 0 deletions src/EFCore/ChangeTracking/Internal/SnapshotFactoryFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,21 @@ private Expression CreateSnapshotExpression(
CreateReadShadowValueExpression(parameter, propertyBase),
propertyBase);
}
else if (propertyBase.IsIndexedProperty)
{
var indexerAccessExpression = (Expression)Expression.MakeIndex(
entityVariable,
propertyBase.PropertyInfo,
new List<Expression>() { Expression.Constant(propertyBase.Name) });
if (propertyBase.PropertyInfo.PropertyType != propertyBase.ClrType)
{
indexerAccessExpression =
Expression.Convert(indexerAccessExpression, propertyBase.ClrType);
}

arguments[i] =
CreateSnapshotValueExpression(indexerAccessExpression, propertyBase);
}
else
{
var memberAccess = (Expression)Expression.MakeMemberAccess(
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/ClrAccessorFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ private static readonly MethodInfo _genericCreate
/// directly from your code. This API may change or be removed in future releases.
/// </summary>
public virtual TAccessor Create([NotNull] IPropertyBase property)
=> property as TAccessor ?? Create(null, property);
=> property as TAccessor ?? Create(property.PropertyInfo, property);
lajones marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore/Metadata/Internal/EntityMaterializerSource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ public virtual Expression CreateMaterializeExpression(
.Concat(
entityType
.GetProperties()
.Where(p => !p.IsShadowProperty || p.IsIndexedProperty)));
.Where(p => !p.IsShadowProperty)));

foreach (var consumedProperty in constructorBinding
.ParameterBindings
Expand Down
19 changes: 13 additions & 6 deletions src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1597,7 +1597,7 @@ public virtual Property AddProperty(
ClrType?.GetMembersInHierarchy(name).FirstOrDefault(),
configurationSource,
typeConfigurationSource,
isIndexProperty: false);
isIndexedProperty: false);
}

/// <summary>
Expand Down Expand Up @@ -1628,7 +1628,7 @@ public virtual Property AddProperty(
memberInfo,
configurationSource,
configurationSource,
isIndexProperty: false);
isIndexedProperty: false);
}


Expand All @@ -1646,13 +1646,19 @@ public virtual Property AddIndexedProperty(
Check.NotNull(name, nameof(name));
Check.NotNull(propertyType, nameof(propertyType));

var indexerProperty = this.EFIndexerProperty();
if (indexerProperty == null)
{
throw new InvalidOperationException(CoreStrings.NoIndexer(name, this.DisplayName()));
}

return AddProperty(
name,
propertyType,
null,
indexerProperty,
configurationSource,
typeConfigurationSource,
isIndexProperty: true);
isIndexedProperty: true);
}

private Property AddProperty(
Expand All @@ -1661,7 +1667,7 @@ private Property AddProperty(
MemberInfo memberInfo,
ConfigurationSource configurationSource,
ConfigurationSource? typeConfigurationSource,
bool isIndexProperty)
bool isIndexedProperty)
{
var conflictingMember = FindMembersInHierarchy(name).FirstOrDefault();
if (conflictingMember != null)
Expand All @@ -1685,6 +1691,7 @@ private Property AddProperty(
else
{
if (memberInfo != null
&& !isIndexedProperty
&& propertyType != memberInfo.GetMemberType())
{
throw new InvalidOperationException(
Expand All @@ -1698,7 +1705,7 @@ private Property AddProperty(

var property = new Property(
name, propertyType, memberInfo as PropertyInfo, memberInfo as FieldInfo, this,
configurationSource, typeConfigurationSource, isIndexProperty);
configurationSource, typeConfigurationSource);

_properties.Add(property.Name, property);
PropertyMetadataChanged();
Expand Down
6 changes: 3 additions & 3 deletions src/EFCore/Metadata/Internal/IndexedPropertyGetterFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ public class IndexedPropertyGetterFactory : ClrAccessorFactory<IClrPropertyGette
protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullableEnumValue>(
PropertyInfo propertyInfo, IPropertyBase propertyBase)
{
Debug.Assert(propertyInfo != null);
Debug.Assert(propertyBase != null);

var indexerPropertyInfo = propertyBase.DeclaringType.EFIndexerProperty();
if (indexerPropertyInfo == null)
if (!propertyInfo.IsEFIndexerProperty())
{
throw new InvalidOperationException(
CoreStrings.NoIndexer(propertyBase.Name, propertyBase.DeclaringType.DisplayName()));
Expand All @@ -37,7 +37,7 @@ protected override IClrPropertyGetter CreateGeneric<TEntity, TValue, TNonNullabl
var entityParameter = Expression.Parameter(typeof(TEntity), "entity");
var indexerParameterList = new List<Expression>() { Expression.Constant(propertyBase.Name) };
Expression readExpression = Expression.MakeIndex(
entityParameter, indexerPropertyInfo, indexerParameterList);
entityParameter, propertyInfo, indexerParameterList);

if (readExpression.Type != typeof(TValue))
{
Expand Down
9 changes: 5 additions & 4 deletions src/EFCore/Metadata/Internal/IndexedPropertySetterFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ public class IndexedPropertySetterFactory : ClrAccessorFactory<IClrPropertySette
protected override IClrPropertySetter CreateGeneric<TEntity, TValue, TNonNullableEnumValue>(
PropertyInfo propertyInfo, IPropertyBase propertyBase)
{
Debug.Assert(propertyInfo != null);
Debug.Assert(propertyBase != null);

var indexerPropertyInfo = propertyBase.DeclaringType.EFIndexerProperty();
if (indexerPropertyInfo == null)
if (!propertyInfo.IsEFIndexerProperty())
{
throw new InvalidOperationException(
CoreStrings.NoIndexer(propertyBase.Name, propertyBase.DeclaringType.DisplayName()));
Expand All @@ -37,14 +37,15 @@ protected override IClrPropertySetter CreateGeneric<TEntity, TValue, TNonNullabl
var valueParameter = Expression.Parameter(typeof(TValue), "value");
var indexerParameterList = new List<Expression>() { Expression.Constant(propertyBase.Name) };

// the indexer expects the value to be an object, so cast it to that if necessary
// the indexer expects the value to be an object, but the indexed property
// can have been declared as a different type so cast it to that if necessary
var propertyType = propertyBase.ClrType;
var convertedParameter = propertyType == typeof(object)
? (Expression)valueParameter
: Expression.TypeAs(valueParameter, typeof(object));

Expression writeExpression = Expression.Assign(
Expression.MakeIndex(entityParameter, indexerPropertyInfo, indexerParameterList),
Expression.MakeIndex(entityParameter, propertyInfo, indexerParameterList),
convertedParameter);

var setter = Expression.Lambda<Action<TEntity, TValue>>(
Expand Down
5 changes: 2 additions & 3 deletions src/EFCore/Metadata/Internal/Property.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ public Property(
[CanBeNull] FieldInfo fieldInfo,
[NotNull] EntityType declaringEntityType,
ConfigurationSource configurationSource,
ConfigurationSource? typeConfigurationSource,
bool isIndexProperty = false)
: base(name, propertyInfo, fieldInfo, isIndexProperty)
ConfigurationSource? typeConfigurationSource)
: base(name, propertyInfo, fieldInfo)
{
Check.NotNull(clrType, nameof(clrType));
Check.NotNull(declaringEntityType, nameof(declaringEntityType));
Expand Down
8 changes: 4 additions & 4 deletions src/EFCore/Metadata/Internal/PropertyBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ public abstract class PropertyBase : ConventionalAnnotatable, IMutablePropertyBa
{
private FieldInfo _fieldInfo;
private ConfigurationSource? _fieldInfoConfigurationSource;
private bool _isIndexedProperty;

// Warning: Never access these fields directly as access needs to be thread-safe
private IClrPropertyGetter _getter;
private IClrPropertySetter _setter;
private PropertyAccessors _accessors;
private PropertyIndexes _indexes;
private bool _isIndexedProperty;

/// <summary>
/// This API supports the Entity Framework Core infrastructure and is not intended to be used
Expand All @@ -33,15 +33,15 @@ public abstract class PropertyBase : ConventionalAnnotatable, IMutablePropertyBa
protected PropertyBase(
[NotNull] string name,
[CanBeNull] PropertyInfo propertyInfo,
[CanBeNull] FieldInfo fieldInfo,
bool isIndexProperty = false)
[CanBeNull] FieldInfo fieldInfo)
{
Check.NotEmpty(name, nameof(name));

Name = name;
PropertyInfo = propertyInfo;
_fieldInfo = fieldInfo;
_isIndexedProperty = isIndexProperty;
_isIndexedProperty = propertyInfo != null
&& propertyInfo.IsEFIndexerProperty();
}

/// <summary>
Expand Down
5 changes: 5 additions & 0 deletions src/EFCore/Metadata/Internal/PropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ public static string ToDebugString([NotNull] this IProperty property, bool singl
builder.Append(" Shadow");
}

if (property.IsIndexedProperty)
{
builder.Append(" Indexed");
}

if (!property.IsNullable)
{
builder.Append(" Required");
Expand Down
6 changes: 1 addition & 5 deletions src/EFCore/Metadata/Internal/TypeBaseExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,7 @@ public static PropertyInfo EFIndexerProperty([NotNull] this ITypeBase type)
: type.ClrType.GetRuntimeProperties();

// find the indexer with single argument of type string which returns an object
return (from p in runtimeProperties
where p.PropertyType == typeof(object)
let q = p.GetIndexParameters()
where q.Length == 1 && q[0].ParameterType == typeof(string)
select p).FirstOrDefault();
return runtimeProperties.FirstOrDefault(p => p.IsEFIndexerProperty());
}
}
}
15 changes: 15 additions & 0 deletions src/Shared/PropertyInfoExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,21 @@ public static bool IsCandidateProperty(this PropertyInfo propertyInfo, bool need
&& propertyInfo.GetMethod != null && (!publicOnly || propertyInfo.GetMethod.IsPublic)
&& propertyInfo.GetIndexParameters().Length == 0;

public static bool IsEFIndexerProperty([NotNull] this PropertyInfo propertyInfo)
{
if (propertyInfo.PropertyType == typeof(object))
{
var indexParams = propertyInfo.GetIndexParameters();
if (indexParams.Length == 1
&& indexParams[0].ParameterType == typeof(string))
{
return true;
}
}

return false;
}

public static PropertyInfo FindGetterProperty([NotNull] this PropertyInfo propertyInfo)
=> propertyInfo.DeclaringType
.GetPropertiesInHierarchy(propertyInfo.GetSimpleMemberName())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,10 @@ public void Exception_is_returned_for_indexed_property_without_indexer()
{
var entityType = new Model().AddEntityType(typeof(NonIndexedClass));
var idProperty = entityType.AddProperty("Id", typeof(int));
var propertyA = entityType.AddIndexedProperty("PropertyA", typeof(string));
var propertyB = entityType.AddIndexedProperty("PropertyB", typeof(int));

var nonIndexedClass = new NonIndexedClass();
Assert.Throws<InvalidOperationException>(
() => new IndexedPropertyGetterFactory().Create(propertyA).GetClrValue(nonIndexedClass));
() => entityType.AddIndexedProperty("PropA", typeof(string)));
Assert.Throws<InvalidOperationException>(
() => new IndexedPropertyGetterFactory().Create(propertyB).GetClrValue(nonIndexedClass));
() => entityType.AddIndexedProperty("PropB", typeof(int)));
}

private class IndexedClass
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// 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 Xunit;

Expand Down Expand Up @@ -29,27 +28,6 @@ public void Delegate_setter_is_returned_for_indexed_property()
Assert.Equal(456, indexedClass["PropertyB"]);
}

[Fact]
public void Exception_is_returned_when_setting_indexed_property_without_indexer()
{
var entityType = new Model().AddEntityType(typeof(NonIndexedClass));
var idProperty = entityType.AddProperty("Id", typeof(int));
var propertyA = entityType.AddIndexedProperty("PropertyA", typeof(string));
var propertyB = entityType.AddIndexedProperty("PropertyB", typeof(int));

var indexedClass = new NonIndexedClass
{
Id = 1,
PropA = "PropAValue",
PropB = 123
};

Assert.Throws<InvalidOperationException>(
() => new IndexedPropertySetterFactory().Create(propertyA).SetClrValue(indexedClass, "UpdatedValueA"));
Assert.Throws<InvalidOperationException>(
() => new IndexedPropertySetterFactory().Create(propertyB).SetClrValue(indexedClass, 456));
}

private class IndexedClass
{
private Dictionary<string, object> _internalValues = new Dictionary<string, object>()
Expand All @@ -66,12 +44,5 @@ public object this[string name]
set => _internalValues[name] = value;
}
}

private class NonIndexedClass
{
internal int Id { get; set; }
public string PropA { get; set; }
public int PropB { get; set; }
}
}
}