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.

Sign up for GitHub

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

Jump to bottom

API review changes #22037

Merged
2 commits merged into from
Aug 12, 2020
Merged

API review changes #22037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3815a0d
API review changes
ajcvickers Aug 12, 2020
8625cc7
Tweak DbFunctionAttribute
ajcvickers Aug 12, 2020
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
13 changes: 4 additions & 9 deletions src/EFCore.Abstractions/DbFunctionAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ namespace Microsoft.EntityFrameworkCore
public class DbFunctionAttribute : Attribute
#pragma warning restore CA1813 // Avoid unsealed attributes
{
private static readonly bool DefaultNullable = true;

private string _name;
private string _schema;
private bool _builtIn;
Expand All @@ -36,14 +34,12 @@ public DbFunctionAttribute()
/// </summary>
/// <param name="name">The name of the function in the database.</param>
/// <param name="schema">The schema of the function in the database.</param>
/// <param name="builtIn"> The value indicating whether the database function is built-in or not. </param>
public DbFunctionAttribute([NotNull] string name, [CanBeNull] string schema = null, bool builtIn = false)
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
public DbFunctionAttribute([NotNull] string name, [CanBeNull] string schema = null)
{
Check.NotEmpty(name, nameof(name));

_name = name;
_schema = schema;
_builtIn = builtIn;
}

/// <summary>
Expand Down Expand Up @@ -84,14 +80,13 @@ public virtual bool IsBuiltIn
/// </summary>
public virtual bool IsNullable
{
get => _nullable ?? DefaultNullable;
get => _nullable ?? true;
set => _nullable = value;
}

/// <summary>
/// Use this method if you want to know the nullability of
/// the database function or <see langword="null"/> if it was not specified.
/// Checks whether <see cref="IsNullable" /> has been explicitly set to a value.
/// </summary>
public bool? GetIsNullable() => _nullable;
public bool IsNullableHasValue => _nullable.HasValue;
}
}
23 changes: 13 additions & 10 deletions src/EFCore.Abstractions/IndexAttribute.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,44 +15,47 @@ namespace Microsoft.EntityFrameworkCore
[AttributeUsage(AttributeTargets.Class, AllowMultiple = true)]
public sealed class IndexAttribute : Attribute
{
private static readonly bool DefaultIsUnique = false;
private bool? _isUnique;
private string _name;

/// <summary>
/// Initializes a new instance of the <see cref="IndexAttribute" /> class.
/// Initializes a new instance of the <see cref="IndexAttribute" /> class.
/// </summary>
/// <param name="propertyNames"> The properties which constitute the index, in order (there must be at least one). </param>
public IndexAttribute(params string[] propertyNames)
public IndexAttribute([CanBeNull] params string[] propertyNames)
{
Check.NotEmpty(propertyNames, nameof(propertyNames));
Check.HasNoEmptyElements(propertyNames, nameof(propertyNames));

PropertyNames = propertyNames.ToList();
}

/// <summary>
/// The properties which constitute the index, in order.
/// </summary>
public List<string> PropertyNames { get; }
public IReadOnlyList<string> PropertyNames { get; }

/// <summary>
/// The name of the index.
/// </summary>
public string Name { get; [param: NotNull] set; }

public string Name
{
get => _name;
[param: NotNull] set => _name = Check.NotNull(value, nameof(value));
}

/// <summary>
/// Whether the index is unique.
/// </summary>
public bool IsUnique
{
get => _isUnique ?? DefaultIsUnique;
get => _isUnique ?? false;
set => _isUnique = value;
}

/// <summary>
/// Use this method if you want to know the uniqueness of
/// the index or <see langword="null"/> if it was not specified.
/// Checks whether <see cref="IsUnique" /> has been explicitly set to a value.
/// </summary>
public bool? GetIsUnique() => _isUnique;
public bool IsUniqueHasValue => _isUnique.HasValue;
ajcvickers marked this conversation as resolved.
Show resolved Hide resolved
}
}
6 changes: 3 additions & 3 deletions src/EFCore.InMemory/Storage/Internal/InMemoryStore.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public virtual IReadOnlyList<InMemoryTableSnapshot> GetTables(IEntityType entity
{
foreach (var et in entityType.GetDerivedTypesInclusive().Where(et => !et.IsAbstract()))
{
var key = _useNameMatching ? (object)et.DisplayName() : et;
var key = _useNameMatching ? (object)et.FullName() : et;
if (_tables.TryGetValue(key, out var table))
{
data.Add(new InMemoryTableSnapshot(et, table.SnapshotRows()));
Expand Down Expand Up @@ -218,7 +218,7 @@ private IInMemoryTable EnsureTable(IEntityType entityType)
var entityTypes = entityType.GetAllBaseTypesInclusive();
foreach (var currentEntityType in entityTypes)
{
var key = _useNameMatching ? (object)currentEntityType.DisplayName() : currentEntityType;
var key = _useNameMatching ? (object)currentEntityType.FullName() : currentEntityType;
if (!_tables.TryGetValue(key, out var table))
{
_tables.Add(key, table = _tableFactory.Create(currentEntityType, baseTable));
Expand All @@ -228,7 +228,7 @@ private IInMemoryTable EnsureTable(IEntityType entityType)
}


return _tables[_useNameMatching ? (object)entityType.DisplayName() : entityType];
return _tables[_useNameMatching ? (object)entityType.FullName() : entityType];
}
}
}
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Metadata/Conventions/RelationalDbFunctionAttributeConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ protected virtual void ProcessDbFunctionAdded(
dbFunctionBuilder.IsBuiltIn(dbFunctionAttribute.IsBuiltIn, fromDataAnnotation: true);
}

if (dbFunctionAttribute.GetIsNullable() is bool value)
if (dbFunctionAttribute.IsNullableHasValue)
{
dbFunctionBuilder.IsNullable(value, fromDataAnnotation: true);
dbFunctionBuilder.IsNullable(dbFunctionAttribute.IsNullable, fromDataAnnotation: true);
}
}
}
Expand Down
58 changes: 23 additions & 35 deletions src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
using System.Collections.Generic;
using System.Reflection;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
Expand All @@ -16,8 +15,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
/// <summary>
/// A convention that configures database indexes based on the <see cref="IndexAttribute" />.
/// </summary>
public class IndexAttributeConvention : IEntityTypeAddedConvention,
IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention
public class IndexAttributeConvention : IEntityTypeAddedConvention, IEntityTypeBaseTypeChangedConvention, IModelFinalizingConvention
{
/// <summary>
/// Creates a new instance of <see cref="IndexAttributeConvention" />.
Expand All @@ -37,9 +35,7 @@ public IndexAttributeConvention([NotNull] ProviderConventionSetBuilderDependenci
public virtual void ProcessEntityTypeAdded(
IConventionEntityTypeBuilder entityTypeBuilder,
IConventionContext<IConventionEntityTypeBuilder> context)
{
CheckIndexAttributesAndEnsureIndex(entityTypeBuilder.Metadata, false);
}
=> CheckIndexAttributesAndEnsureIndex(entityTypeBuilder.Metadata, false);

/// <inheritdoc/>
public virtual void ProcessEntityTypeBaseTypeChanged(
Expand Down Expand Up @@ -67,7 +63,7 @@ public virtual void ProcessModelFinalizing(
}
}

private void CheckIndexAttributesAndEnsureIndex(
private static void CheckIndexAttributesAndEnsureIndex(
IConventionEntityType entityType,
bool shouldThrow)
{
Expand All @@ -79,7 +75,7 @@ private void CheckIndexAttributesAndEnsureIndex(
foreach (var indexAttribute in
entityType.ClrType.GetCustomAttributes<IndexAttribute>(true))
{
IConventionIndexBuilder indexBuilder = null;
IConventionIndexBuilder indexBuilder;
if (!shouldThrow)
{
var indexProperties = new List<IConventionProperty>();
Expand Down Expand Up @@ -125,18 +121,14 @@ private void CheckIndexAttributesAndEnsureIndex(
{
CheckIgnoredProperties(indexAttribute, entityType);
}
else
else if (indexAttribute.IsUniqueHasValue)
{
var shouldBeUnique = indexAttribute.GetIsUnique();
if (shouldBeUnique.HasValue)
{
indexBuilder.IsUnique(shouldBeUnique.Value, fromDataAnnotation: true);
}
indexBuilder.IsUnique(indexAttribute.IsUnique, fromDataAnnotation: true);
}
}
}

private void CheckIgnoredProperties(
private static void CheckIgnoredProperties(
IndexAttribute indexAttribute,
IConventionEntityType entityType)
{
Expand All @@ -152,20 +144,18 @@ private void CheckIgnoredProperties(
indexAttribute.PropertyNames.Format(),
propertyName));
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnIgnoredProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}

throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnIgnoredProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName));
}
}
}

private void CheckMissingProperties(
private static void CheckMissingProperties(
IndexAttribute indexAttribute,
IConventionEntityType entityType,
InvalidOperationException innerException)
Expand All @@ -184,16 +174,14 @@ private void CheckMissingProperties(
propertyName),
innerException);
}
else
{
throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnNonExistentProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName),
innerException);
}

throw new InvalidOperationException(
CoreStrings.NamedIndexDefinedOnNonExistentProperty(
indexAttribute.Name,
entityType.DisplayName(),
indexAttribute.PropertyNames.Format(),
propertyName),
innerException);
}
}
}
Expand Down
1 change: 0 additions & 1 deletion src/Shared/Check.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ public static IReadOnlyList<T> HasNoNulls<T>(IReadOnlyList<T> value, [InvokerPar
return value;
}


public static IReadOnlyList<string> HasNoEmptyElements(IReadOnlyList<string> value, [InvokerParameterName][NotNull] string parameterName)
{
NotNull(value, parameterName);
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Relational.Specification.Tests/Query/UdfDbFunctionTestBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ public enum ReportingPeriod
Fall
}

[DbFunction(Name = "len", IsBuiltIn = true)]
[DbFunction("len", IsBuiltIn = true)]
public static long MyCustomLengthStatic(string s) => throw new Exception();
public static bool IsDateStatic(string date) => throw new Exception();
public static int AddOneStatic(int num) => num + 1;
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.Relational.Tests/Metadata/DbFunctionMetadataTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public static int MethodA(string a, int b)
throw new NotImplementedException();
}

[DbFunction(Schema = "bar", Name = "MethodFoo")]
[DbFunction("MethodFoo", "bar")]
public static int MethodB(string c, int d)
{
throw new NotImplementedException();
Expand Down