From 05e313138c4c8466117ce93d93a3c4da1dc750af Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Tue, 30 Oct 2018 18:41:33 -0400 Subject: [PATCH 1/4] Follow-up to schema-enums (605, 724) Refactors `PostgresEnum` to properly handle schema qualification without breaking the standard patterns for migration operations. Schema-qualified enums are now yielded from the annotation provider with the default schema "baked in" such that migration operations are self-contained. To get this right, `PostgresEnum` is refactored to be less mutable, with simpler access patterns. The `GetData()` and `SetData(...)` functions are removed in support this simpler, more immutable enum representation. Address review comments: - Rollback some of the layout changes - Move the "get or add" logic back into `GetOrAddPostgresEnum` - Move default schema access back into `PostgresEnum` - Note that this depends on `CreateAnnotation` (or similar) --- .../Internal/NpgsqlAnnotationCodeGenerator.cs | 2 +- src/EFCore.PG/Metadata/PostgresEnum.cs | 172 +++++++++++------- .../NpgsqlMigrationsAnnotationProvider.cs | 17 +- .../NpgsqlMigrationsSqlGenerator.cs | 20 +- 4 files changed, 128 insertions(+), 83 deletions(-) diff --git a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs index 25b2381c0..5ad63448b 100644 --- a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs +++ b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs @@ -75,7 +75,7 @@ public override MethodCallCodeFragment GenerateFluentApi(IModel model, IAnnotati extension.Name); } - if (annotation.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix)) + if (annotation.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)) { var enumTypeDef = new PostgresEnum(model, annotation.Name); diff --git a/src/EFCore.PG/Metadata/PostgresEnum.cs b/src/EFCore.PG/Metadata/PostgresEnum.cs index 929da007c..eba299126 100644 --- a/src/EFCore.PG/Metadata/PostgresEnum.cs +++ b/src/EFCore.PG/Metadata/PostgresEnum.cs @@ -9,94 +9,138 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata { + /// + /// Represents the metadata for a PostgreSQL enum. + /// public class PostgresEnum { - readonly IAnnotatable _annotatable; - readonly string _annotationName; - - internal PostgresEnum(IAnnotatable annotatable, string annotationName) + [NotNull] readonly IAnnotatable _annotatable; + [NotNull] readonly string _annotationName; + + /// + /// Creates a . + /// + /// The annotatable to search for the annotation. + /// The annotation name to search for in the annotatable. + /// + /// + internal PostgresEnum([NotNull] IAnnotatable annotatable, [NotNull] string annotationName) { - _annotatable = annotatable; - _annotationName = annotationName; + _annotatable = Check.NotNull(annotatable, nameof(annotatable)); + _annotationName = Check.NotNull(annotationName, nameof(annotationName)); } - public static PostgresEnum GetOrAddPostgresEnum( - [NotNull] IMutableAnnotatable annotatable, - [CanBeNull] string schema, - [NotNull] string name, - [NotNull] string[] labels) + /// + /// Creates an that represents a PostgreSQL enum. + /// + /// The enum schema or null to use the model's default schema. + /// The enum name. + /// The enum labels. + /// + /// An representing a PostgreSQL enum. + /// + /// + /// + /// + [NotNull] + public static IAnnotation CreateAnnotation([CanBeNull] string schema, [NotNull] string name, [NotNull] string[] labels) { - if (FindPostgresEnum(annotatable, schema, name) is PostgresEnum enumType) - return enumType; + Check.NullButNotEmpty(schema, nameof(schema)); + Check.NotNull(name, nameof(name)); + Check.NotNull(labels, nameof(labels)); + + var annotationName = + schema != null + ? $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}" + : $"{NpgsqlAnnotationNames.EnumPrefix}{name}"; - enumType = new PostgresEnum(annotatable, BuildAnnotationName(schema, name)); - enumType.SetData(labels); - return enumType; + var annotationValue = string.Join(",", labels); + + return new Annotation(annotationName, annotationValue); } + /// + /// Gets or adds a from or to the . + /// + /// The annotatable from which to get or add the enum. + /// The enum schema or null to use the model's default schema. + /// The enum name. + /// The enum labels. + /// + /// The from the . + /// + /// + /// + /// + /// + [NotNull] public static PostgresEnum GetOrAddPostgresEnum( [NotNull] IMutableAnnotatable annotatable, + [CanBeNull] string schema, [NotNull] string name, [NotNull] string[] labels) - => GetOrAddPostgresEnum(annotatable, null, name, labels); - - public static PostgresEnum FindPostgresEnum( - [NotNull] IAnnotatable annotatable, - [CanBeNull] string schema, - [NotNull] string name) { Check.NotNull(annotatable, nameof(annotatable)); - Check.NotEmpty(name, nameof(name)); + Check.NullButNotEmpty(schema, nameof(schema)); + Check.NotNull(name, nameof(name)); + Check.NotNull(labels, nameof(labels)); - var annotationName = BuildAnnotationName(schema, name); + var annotation = CreateAnnotation(schema, name, labels); - return annotatable[annotationName] == null ? null : new PostgresEnum(annotatable, annotationName); - } + var postgresEnum = new PostgresEnum(annotatable, annotation.Name); - static string BuildAnnotationName(string schema, string name) - => NpgsqlAnnotationNames.EnumPrefix + (schema == null ? name : schema + '.' + name); + if (postgresEnum.Annotation == null && postgresEnum._annotatable is IMutableAnnotatable mutable) + mutable.SetAnnotation(annotation.Name, annotation.Value); - public static IEnumerable GetPostgresEnums([NotNull] IAnnotatable annotatable) - { - Check.NotNull(annotatable, nameof(annotatable)); - - return annotatable.GetAnnotations() - .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)) - .Select(a => new PostgresEnum(annotatable, a.Name)); + return postgresEnum; } - public Annotatable Annotatable => (Annotatable)_annotatable; - - public string Schema => GetData().Schema; - - public string Name => GetData().Name; - - public string[] Labels - { - get => GetData().Labels; - set => SetData(value); - } - - (string Schema, string Name, string[] Labels) GetData() - { - return !(Annotatable[_annotationName] is string annotationValue) - ? (null, null, null) - : Deserialize(_annotationName, annotationValue); - } - - void SetData(string[] labels) - => Annotatable[_annotationName] = string.Join(",", labels); - - static (string schema, string name, string[] labels) Deserialize( - [NotNull] string annotationName, - [NotNull] string annotationValue) + /// + /// Gets the collection of stored in the . + /// + /// The annotatable to search for annotations. + /// + /// The collection of stored in the . + /// + /// + [NotNull] + [ItemNotNull] + public static IEnumerable GetPostgresEnums([NotNull] IAnnotatable annotatable) + => Check.NotNull(annotatable, nameof(annotatable)) + .GetAnnotations() + .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)) + .Select(a => new PostgresEnum(annotatable, a.Name)); + + [CanBeNull] + IAnnotation Annotation => _annotatable.FindAnnotation(_annotationName); + + /// + /// The enum schema or null to represent the default schema. + /// + [CanBeNull] + public string Schema => Deserialize(Annotation).Schema ?? (string)_annotatable[RelationalAnnotationNames.DefaultSchema]; + + /// + /// The enum name. + /// + [NotNull] + public string Name => Deserialize(Annotation).Name; + + /// + /// The enum labels. + /// + [NotNull] + public string[] Labels => Deserialize(Annotation).Labels; + + static (string Schema, string Name, string[] Labels) Deserialize([CanBeNull] IAnnotation annotation) { - Check.NotEmpty(annotationValue, nameof(annotationValue)); + if (annotation == null || !(annotation.Value is string value) || string.IsNullOrEmpty(value)) + return (null, null, null); - var labels = annotationValue.Split(',').ToArray(); + var labels = value.Split(',').ToArray(); // Yes, this doesn't support dots in the schema/enum name, let somebody complain first. - var schemaAndName = annotationName.Substring(NpgsqlAnnotationNames.EnumPrefix.Length).Split('.'); + var schemaAndName = annotation.Name.Substring(NpgsqlAnnotationNames.EnumPrefix.Length).Split('.'); switch (schemaAndName.Length) { case 1: @@ -104,7 +148,7 @@ void SetData(string[] labels) case 2: return (schemaAndName[0], schemaAndName[1], labels); default: - throw new ArgumentException("Cannot parse enum name from annotation: " + annotationName); + throw new ArgumentException($"Cannot parse enum name from annotation: {annotation.Name}"); } } } diff --git a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs index 0569df4f1..f8f885738 100644 --- a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs +++ b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs @@ -48,9 +48,18 @@ public override IEnumerable For(IIndex index) } public override IEnumerable For(IModel model) - => model.GetAnnotations().Where(a => - a.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal) || - a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal) || - a.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)); + { + foreach (var annotation in model.GetAnnotations()) + { + if (annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)) + yield return annotation; + + if (annotation.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)) + yield return annotation; + } + + foreach (var e in model.Npgsql().PostgresEnums) + yield return PostgresEnum.CreateAnnotation(e.Schema, e.Name, e.Labels); + } } } diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs index bfc10b98e..fe654c2e5 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs @@ -638,6 +638,7 @@ protected override void Generate( MigrationCommandListBuilder builder) { Check.NotNull(operation, nameof(operation)); + Check.NotNull(model, nameof(model)); Check.NotNull(builder, nameof(builder)); GenerateEnumStatements(operation, model, builder); @@ -691,7 +692,7 @@ protected virtual void GenerateEnumStatements( foreach (var enumTypeToDrop in operation.Npgsql().OldPostgresEnums .Where(oe => operation.Npgsql().PostgresEnums.All(ne => ne.Name != oe.Name))) { - GenerateDropEnum(enumTypeToDrop, operation.OldDatabase, builder); + GenerateDropEnum(enumTypeToDrop, builder); } // TODO: Some forms of enum alterations are actually supported... @@ -708,16 +709,14 @@ protected virtual void GenerateCreateEnum( [NotNull] IModel model, [NotNull] MigrationCommandListBuilder builder) { - var schema = GetSchemaOrDefault(enumType.Schema, model); - // Schemas are normally created (or rather ensured) by the model differ, which scans all tables, sequences // and other database objects. However, it isn't aware of enums, so we always ensure schema on enum creation. - if (schema != null) - Generate(new EnsureSchemaOperation { Name = schema }, model, builder); + if (enumType.Schema != null) + Generate(new EnsureSchemaOperation { Name = enumType.Schema }, model, builder); builder .Append("CREATE TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, enumType.Schema)) .Append(" AS ENUM ("); var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); @@ -735,14 +734,11 @@ protected virtual void GenerateCreateEnum( protected virtual void GenerateDropEnum( [NotNull] PostgresEnum enumType, - [CanBeNull] IAnnotatable oldDatabase, [NotNull] MigrationCommandListBuilder builder) { - var schema = GetSchemaOrDefault(enumType.Schema, oldDatabase); - builder .Append("DROP TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, enumType.Schema)) .AppendLine(";"); } @@ -1117,10 +1113,6 @@ static string GenerateStorageParameterValue(object value) #region Helpers - [CanBeNull] - static string GetSchemaOrDefault([CanBeNull] string schema, [CanBeNull] IAnnotatable model) - => schema ?? model?.FindAnnotation(RelationalAnnotationNames.DefaultSchema)?.Value as string; - /// /// True if is null, greater than, or equal to the specified version. /// From 9061fa4efb38104ca861c7a0b5fc1f4058939654 Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Wed, 5 Dec 2018 10:07:25 -0500 Subject: [PATCH 2/4] Address review - Late-access default schema (SQL generation time). - Return `IReadOnlyList` for `PostgresEnum.Labels` since items of that collection are not writable (i.e. returns a copy from the annotation value). - Revert changes in the annotation provider. If the default schema is late-accessed for SQL generation, then we can return an unmodified annotation. - Match the semantics of `Sequence.cs` such that `PostgresEnum.Schema` returns the annotation's schema when it is not null, otherwise attempts to find the annotatable's default schema annotation. This is not, however, used for any of the migration operations, so that the the default schema is only implied when `PostgresEnum.Schema == null`. --- src/EFCore.PG/Metadata/PostgresEnum.cs | 50 ++++++------------- .../NpgsqlMigrationsAnnotationProvider.cs | 17 ++----- .../NpgsqlMigrationsSqlGenerator.cs | 19 ++++--- 3 files changed, 31 insertions(+), 55 deletions(-) diff --git a/src/EFCore.PG/Metadata/PostgresEnum.cs b/src/EFCore.PG/Metadata/PostgresEnum.cs index eba299126..f240878ee 100644 --- a/src/EFCore.PG/Metadata/PostgresEnum.cs +++ b/src/EFCore.PG/Metadata/PostgresEnum.cs @@ -30,35 +30,6 @@ internal PostgresEnum([NotNull] IAnnotatable annotatable, [NotNull] string annot _annotationName = Check.NotNull(annotationName, nameof(annotationName)); } - /// - /// Creates an that represents a PostgreSQL enum. - /// - /// The enum schema or null to use the model's default schema. - /// The enum name. - /// The enum labels. - /// - /// An representing a PostgreSQL enum. - /// - /// - /// - /// - [NotNull] - public static IAnnotation CreateAnnotation([CanBeNull] string schema, [NotNull] string name, [NotNull] string[] labels) - { - Check.NullButNotEmpty(schema, nameof(schema)); - Check.NotNull(name, nameof(name)); - Check.NotNull(labels, nameof(labels)); - - var annotationName = - schema != null - ? $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}" - : $"{NpgsqlAnnotationNames.EnumPrefix}{name}"; - - var annotationValue = string.Join(",", labels); - - return new Annotation(annotationName, annotationValue); - } - /// /// Gets or adds a from or to the . /// @@ -85,12 +56,15 @@ public static PostgresEnum GetOrAddPostgresEnum( Check.NotNull(name, nameof(name)); Check.NotNull(labels, nameof(labels)); - var annotation = CreateAnnotation(schema, name, labels); + var annotationName = + schema != null + ? $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}" + : $"{NpgsqlAnnotationNames.EnumPrefix}{name}"; - var postgresEnum = new PostgresEnum(annotatable, annotation.Name); + var postgresEnum = new PostgresEnum(annotatable, annotationName); - if (postgresEnum.Annotation == null && postgresEnum._annotatable is IMutableAnnotatable mutable) - mutable.SetAnnotation(annotation.Name, annotation.Value); + if (postgresEnum.Annotation == null) + postgresEnum.Labels = labels; return postgresEnum; } @@ -130,15 +104,21 @@ public static IEnumerable GetPostgresEnums([NotNull] IAnnotatable /// The enum labels. /// [NotNull] - public string[] Labels => Deserialize(Annotation).Labels; + public IReadOnlyList Labels + { + get => Deserialize(Annotation).Labels; + // ReSharper disable once MemberCanBePrivate.Global + set => ((IMutableAnnotatable)_annotatable)[_annotationName] = string.Join(",", value); + } static (string Schema, string Name, string[] Labels) Deserialize([CanBeNull] IAnnotation annotation) { if (annotation == null || !(annotation.Value is string value) || string.IsNullOrEmpty(value)) return (null, null, null); - var labels = value.Split(',').ToArray(); + var labels = value.Split(','); + // TODO: This would be a safer operation if we stored schema and name in the annotation value (see Sequence.cs). // Yes, this doesn't support dots in the schema/enum name, let somebody complain first. var schemaAndName = annotation.Name.Substring(NpgsqlAnnotationNames.EnumPrefix.Length).Split('.'); switch (schemaAndName.Length) diff --git a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs index f8f885738..0569df4f1 100644 --- a/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs +++ b/src/EFCore.PG/Migrations/Internal/NpgsqlMigrationsAnnotationProvider.cs @@ -48,18 +48,9 @@ public override IEnumerable For(IIndex index) } public override IEnumerable For(IModel model) - { - foreach (var annotation in model.GetAnnotations()) - { - if (annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)) - yield return annotation; - - if (annotation.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)) - yield return annotation; - } - - foreach (var e in model.Npgsql().PostgresEnums) - yield return PostgresEnum.CreateAnnotation(e.Schema, e.Name, e.Labels); - } + => model.GetAnnotations().Where(a => + a.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal) || + a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal) || + a.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)); } } diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs index fe654c2e5..9d335b33c 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs @@ -692,7 +692,7 @@ protected virtual void GenerateEnumStatements( foreach (var enumTypeToDrop in operation.Npgsql().OldPostgresEnums .Where(oe => operation.Npgsql().PostgresEnums.All(ne => ne.Name != oe.Name))) { - GenerateDropEnum(enumTypeToDrop, builder); + GenerateDropEnum(enumTypeToDrop, model, builder); } // TODO: Some forms of enum alterations are actually supported... @@ -709,23 +709,25 @@ protected virtual void GenerateCreateEnum( [NotNull] IModel model, [NotNull] MigrationCommandListBuilder builder) { + var schema = enumType.Schema ?? model.Relational().DefaultSchema; + // Schemas are normally created (or rather ensured) by the model differ, which scans all tables, sequences // and other database objects. However, it isn't aware of enums, so we always ensure schema on enum creation. - if (enumType.Schema != null) - Generate(new EnsureSchemaOperation { Name = enumType.Schema }, model, builder); + if (schema != null) + Generate(new EnsureSchemaOperation { Name = schema }, model, builder); builder .Append("CREATE TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, enumType.Schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, schema)) .Append(" AS ENUM ("); var stringTypeMapping = Dependencies.TypeMappingSource.GetMapping(typeof(string)); var labels = enumType.Labels; - for (var i = 0; i < labels.Length; i++) + for (var i = 0; i < labels.Count; i++) { builder.Append(stringTypeMapping.GenerateSqlLiteral(labels[i])); - if (i < labels.Length - 1) + if (i < labels.Count - 1) builder.Append(", "); } @@ -734,11 +736,14 @@ protected virtual void GenerateCreateEnum( protected virtual void GenerateDropEnum( [NotNull] PostgresEnum enumType, + [NotNull] IModel model, [NotNull] MigrationCommandListBuilder builder) { + var schema = enumType.Schema ?? model.Relational().DefaultSchema; + builder .Append("DROP TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, enumType.Schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(enumType.Name, schema)) .AppendLine(";"); } From cdb5e90c01ccf0baedab6d1e4079267f9f629a09 Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Thu, 6 Dec 2018 13:40:37 -0500 Subject: [PATCH 3/4] Address comments --- src/EFCore.PG/Metadata/PostgresEnum.cs | 69 ++++++++++++++++++++------ 1 file changed, 53 insertions(+), 16 deletions(-) diff --git a/src/EFCore.PG/Metadata/PostgresEnum.cs b/src/EFCore.PG/Metadata/PostgresEnum.cs index f240878ee..19af39cec 100644 --- a/src/EFCore.PG/Metadata/PostgresEnum.cs +++ b/src/EFCore.PG/Metadata/PostgresEnum.cs @@ -12,6 +12,7 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata /// /// Represents the metadata for a PostgreSQL enum. /// + [PublicAPI] public class PostgresEnum { [NotNull] readonly IAnnotatable _annotatable; @@ -53,22 +54,50 @@ public static PostgresEnum GetOrAddPostgresEnum( { Check.NotNull(annotatable, nameof(annotatable)); Check.NullButNotEmpty(schema, nameof(schema)); - Check.NotNull(name, nameof(name)); + Check.NotEmpty(name, nameof(name)); Check.NotNull(labels, nameof(labels)); - var annotationName = - schema != null - ? $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}" - : $"{NpgsqlAnnotationNames.EnumPrefix}{name}"; + if (FindPostgresEnum(annotatable, schema, name) is PostgresEnum enumType) + return enumType; - var postgresEnum = new PostgresEnum(annotatable, annotationName); + var annotationName = BuildAnnotationName(schema, name); - if (postgresEnum.Annotation == null) - postgresEnum.Labels = labels; + return new PostgresEnum(annotatable, annotationName) { Labels = labels }; + } - return postgresEnum; + /// + /// Finds a in the , or returns null if not found. + /// + /// The annotatable to search for the enum. + /// The enum schema or null to use the model's default schema. + /// The enum name. + /// + /// The from the . + /// + /// + /// + /// + [CanBeNull] + public static PostgresEnum FindPostgresEnum( + [NotNull] IAnnotatable annotatable, + [CanBeNull] string schema, + [NotNull] string name) + { + Check.NotNull(annotatable, nameof(annotatable)); + Check.NullButNotEmpty(schema, nameof(schema)); + Check.NotEmpty(name, nameof(name)); + + var annotationName = BuildAnnotationName(schema, name); + + return annotatable[annotationName] == null ? null : new PostgresEnum(annotatable, annotationName); } + [NotNull] + static string BuildAnnotationName(string schema, string name) + => schema != null + ? $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}" + : $"{NpgsqlAnnotationNames.EnumPrefix}{name}"; + /// /// Gets the collection of stored in the . /// @@ -85,20 +114,23 @@ public static IEnumerable GetPostgresEnums([NotNull] IAnnotatable .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.EnumPrefix, StringComparison.Ordinal)) .Select(a => new PostgresEnum(annotatable, a.Name)); - [CanBeNull] - IAnnotation Annotation => _annotatable.FindAnnotation(_annotationName); + /// + /// The that stores the enum. + /// + [NotNull] + public Annotatable Annotatable => (Annotatable)_annotatable; /// /// The enum schema or null to represent the default schema. /// [CanBeNull] - public string Schema => Deserialize(Annotation).Schema ?? (string)_annotatable[RelationalAnnotationNames.DefaultSchema]; + public string Schema => GetData().Schema; /// /// The enum name. /// [NotNull] - public string Name => Deserialize(Annotation).Name; + public string Name => GetData().Name; /// /// The enum labels. @@ -106,11 +138,16 @@ public static IEnumerable GetPostgresEnums([NotNull] IAnnotatable [NotNull] public IReadOnlyList Labels { - get => Deserialize(Annotation).Labels; - // ReSharper disable once MemberCanBePrivate.Global - set => ((IMutableAnnotatable)_annotatable)[_annotationName] = string.Join(",", value); + get => GetData().Labels; + set => SetData(value); } + (string Schema, string Name, string[] Labels) GetData() + => Deserialize(Annotatable.FindAnnotation(_annotationName)); + + void SetData([NotNull] IEnumerable labels) + => Annotatable[_annotationName] = string.Join(",", labels); + static (string Schema, string Name, string[] Labels) Deserialize([CanBeNull] IAnnotation annotation) { if (annotation == null || !(annotation.Value is string value) || string.IsNullOrEmpty(value)) From a4cf4ddf42cdbaf877a1dcb9276230d2d7c8e924 Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Thu, 6 Dec 2018 16:04:27 -0500 Subject: [PATCH 4/4] Address comments 2 --- src/EFCore.PG/Metadata/PostgresEnum.cs | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/EFCore.PG/Metadata/PostgresEnum.cs b/src/EFCore.PG/Metadata/PostgresEnum.cs index 19af39cec..c42566ef6 100644 --- a/src/EFCore.PG/Metadata/PostgresEnum.cs +++ b/src/EFCore.PG/Metadata/PostgresEnum.cs @@ -65,6 +65,25 @@ public static PostgresEnum GetOrAddPostgresEnum( return new PostgresEnum(annotatable, annotationName) { Labels = labels }; } + /// + /// Gets or adds a from or to the . + /// + /// The annotatable from which to get or add the enum. + /// The enum name. + /// The enum labels. + /// + /// The from the . + /// + /// + /// + /// + [NotNull] + public static PostgresEnum GetOrAddPostgresEnum( + [NotNull] IMutableAnnotatable annotatable, + [NotNull] string name, + [NotNull] string[] labels) + => GetOrAddPostgresEnum(annotatable, null, name, labels); + /// /// Finds a in the , or returns null if not found. /// @@ -146,7 +165,7 @@ public IReadOnlyList Labels => Deserialize(Annotatable.FindAnnotation(_annotationName)); void SetData([NotNull] IEnumerable labels) - => Annotatable[_annotationName] = string.Join(",", labels); + => Annotatable[_annotationName] = string.Join(",", labels); static (string Schema, string Name, string[] Labels) Deserialize([CanBeNull] IAnnotation annotation) {