From 60e713f1df58bee3009b6322f91f4535a7801c4f Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Sat, 1 Dec 2018 22:39:13 -0500 Subject: [PATCH] Refactor PostgresExtension metadata --- .../Internal/NpgsqlAnnotationCodeGenerator.cs | 2 +- .../NpgsqlModelBuilderExtensions.cs | 40 ++- .../Metadata/INpgsqlModelAnnotations.cs | 2 +- src/EFCore.PG/Metadata/IPostgresExtension.cs | 12 - ...NpgsqlAlterDatabaseOperationAnnotations.cs | 11 +- .../NpgsqlDatabaseModelAnnotations.cs | 9 +- .../Metadata/NpgsqlModelAnnotations.cs | 10 +- src/EFCore.PG/Metadata/PostgresExtension.cs | 273 +++++++++--------- .../NpgsqlMigrationBuilderExtensions.cs | 12 +- .../NpgsqlMigrationsSqlGenerator.cs | 2 +- .../Internal/NpgsqlDatabaseModelFactory.cs | 3 +- .../NpgsqlMigrationSqlGeneratorTest.cs | 5 +- 12 files changed, 210 insertions(+), 171 deletions(-) delete mode 100644 src/EFCore.PG/Metadata/IPostgresExtension.cs diff --git a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs index 5ad63448b..63ca9ae48 100644 --- a/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs +++ b/src/EFCore.PG/Design/Internal/NpgsqlAnnotationCodeGenerator.cs @@ -67,7 +67,7 @@ public override MethodCallCodeFragment GenerateFluentApi(IModel model, IAnnotati Check.NotNull(model, nameof(model)); Check.NotNull(annotation, nameof(annotation)); - if (annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix)) + if (annotation.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)) { var extension = new PostgresExtension(model, annotation.Name); diff --git a/src/EFCore.PG/Extensions/NpgsqlModelBuilderExtensions.cs b/src/EFCore.PG/Extensions/NpgsqlModelBuilderExtensions.cs index 00794a0aa..047f90f0e 100644 --- a/src/EFCore.PG/Extensions/NpgsqlModelBuilderExtensions.cs +++ b/src/EFCore.PG/Extensions/NpgsqlModelBuilderExtensions.cs @@ -147,17 +147,53 @@ public static ModelBuilder ForNpgsqlUseIdentityColumns( #region Extensions + /// + /// Registers a PostgreSQL extension in the model. + /// + /// The model builder in which to define the extension. + /// The schema in which to create the extension. + /// The name of the extension to create. + /// The version of the extension. + /// + /// The updated . + /// + /// + /// See: https://www.postgresql.org/docs/current/external-extensions.html + /// + /// + [NotNull] public static ModelBuilder HasPostgresExtension( [NotNull] this ModelBuilder modelBuilder, - [NotNull] string name) + [CanBeNull] string schema, + [NotNull] string name, + [CanBeNull] string version) { Check.NotNull(modelBuilder, nameof(modelBuilder)); + Check.NullButNotEmpty(schema, nameof(schema)); Check.NotEmpty(name, nameof(name)); - modelBuilder.Model.Npgsql().GetOrAddPostgresExtension(name); + modelBuilder.Model.Npgsql().GetOrAddPostgresExtension(schema, name, version); return modelBuilder; } + /// + /// Registers a PostgreSQL extension in the model. + /// + /// The model builder in which to define the extension. + /// The name of the extension to create. + /// + /// The updated . + /// + /// + /// See: https://www.postgresql.org/docs/current/external-extensions.html + /// + /// + [NotNull] + public static ModelBuilder HasPostgresExtension( + [NotNull] this ModelBuilder modelBuilder, + [NotNull] string name) + => modelBuilder.HasPostgresExtension(null, name, null); + #endregion #region Enums diff --git a/src/EFCore.PG/Metadata/INpgsqlModelAnnotations.cs b/src/EFCore.PG/Metadata/INpgsqlModelAnnotations.cs index 8b89d740f..b9cb0256f 100644 --- a/src/EFCore.PG/Metadata/INpgsqlModelAnnotations.cs +++ b/src/EFCore.PG/Metadata/INpgsqlModelAnnotations.cs @@ -14,7 +14,7 @@ public interface INpgsqlModelAnnotations : IRelationalModelAnnotations [NotNull] [ItemNotNull] - IReadOnlyList PostgresExtensions { get; } + IReadOnlyList PostgresExtensions { get; } [NotNull] [ItemNotNull] diff --git a/src/EFCore.PG/Metadata/IPostgresExtension.cs b/src/EFCore.PG/Metadata/IPostgresExtension.cs deleted file mode 100644 index ed73e8c61..000000000 --- a/src/EFCore.PG/Metadata/IPostgresExtension.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Microsoft.EntityFrameworkCore.Infrastructure; - -namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata -{ - public interface IPostgresExtension - { - IAnnotatable Annotatable { get; } - string Name { get; } - string Schema { get; } - string Version { get; } - } -} diff --git a/src/EFCore.PG/Metadata/NpgsqlAlterDatabaseOperationAnnotations.cs b/src/EFCore.PG/Metadata/NpgsqlAlterDatabaseOperationAnnotations.cs index fed04e9b0..9e71d452b 100644 --- a/src/EFCore.PG/Metadata/NpgsqlAlterDatabaseOperationAnnotations.cs +++ b/src/EFCore.PG/Metadata/NpgsqlAlterDatabaseOperationAnnotations.cs @@ -16,12 +16,12 @@ public class NpgsqlAlterDatabaseOperationAnnotations : RelationalAnnotations [NotNull] [ItemNotNull] - public virtual IReadOnlyList PostgresExtensions + public virtual IReadOnlyList PostgresExtensions => PostgresExtension.GetPostgresExtensions(Metadata).ToArray(); [NotNull] [ItemNotNull] - public virtual IReadOnlyList OldPostgresExtensions + public virtual IReadOnlyList OldPostgresExtensions => PostgresExtension.GetPostgresExtensions(_oldDatabase).ToArray(); [NotNull] @@ -48,5 +48,12 @@ public virtual IReadOnlyList OldPostgresRanges public NpgsqlAlterDatabaseOperationAnnotations([NotNull] AlterDatabaseOperation operation) : base(operation) => _oldDatabase = operation.OldDatabase; + + [NotNull] + public virtual PostgresExtension GetOrAddPostgresExtension( + [CanBeNull] string schema, + [NotNull] string name, + [CanBeNull] string version) + => PostgresExtension.GetOrAddPostgresExtension((IMutableAnnotatable)Metadata, schema, name, version); } } diff --git a/src/EFCore.PG/Metadata/NpgsqlDatabaseModelAnnotations.cs b/src/EFCore.PG/Metadata/NpgsqlDatabaseModelAnnotations.cs index 362119be0..df25a2828 100644 --- a/src/EFCore.PG/Metadata/NpgsqlDatabaseModelAnnotations.cs +++ b/src/EFCore.PG/Metadata/NpgsqlDatabaseModelAnnotations.cs @@ -10,7 +10,7 @@ public class NpgsqlDatabaseModelAnnotations : RelationalAnnotations { [NotNull] [ItemNotNull] - public virtual IReadOnlyList PostgresExtensions + public virtual IReadOnlyList PostgresExtensions => PostgresExtension.GetPostgresExtensions(Metadata).ToArray(); [NotNull] @@ -25,5 +25,12 @@ public virtual IReadOnlyList PostgresRanges /// public NpgsqlDatabaseModelAnnotations([NotNull] DatabaseModel model) : base(model) {} + + [NotNull] + public virtual PostgresExtension GetOrAddPostgresExtension( + [CanBeNull] string schema, + [NotNull] string name, + [CanBeNull] string version) + => PostgresExtension.GetOrAddPostgresExtension((IMutableAnnotatable)Metadata, schema, name, version); } } diff --git a/src/EFCore.PG/Metadata/NpgsqlModelAnnotations.cs b/src/EFCore.PG/Metadata/NpgsqlModelAnnotations.cs index 10f7d3762..067d4e0d7 100644 --- a/src/EFCore.PG/Metadata/NpgsqlModelAnnotations.cs +++ b/src/EFCore.PG/Metadata/NpgsqlModelAnnotations.cs @@ -64,10 +64,14 @@ protected virtual bool SetValueGenerationStrategy(NpgsqlValueGenerationStrategy? #region PostgreSQL Extensions - public virtual PostgresExtension GetOrAddPostgresExtension([NotNull] string name) - => PostgresExtension.GetOrAddPostgresExtension((IMutableModel)Model, name); + [NotNull] + public virtual PostgresExtension GetOrAddPostgresExtension( + [CanBeNull] string schema, + [NotNull] string name, + [CanBeNull] string version) + => PostgresExtension.GetOrAddPostgresExtension((IMutableAnnotatable)Model, schema, name, version); - public virtual IReadOnlyList PostgresExtensions + public virtual IReadOnlyList PostgresExtensions => PostgresExtension.GetPostgresExtensions(Model).ToArray(); #endregion diff --git a/src/EFCore.PG/Metadata/PostgresExtension.cs b/src/EFCore.PG/Metadata/PostgresExtension.cs index 1fbad5392..97ed68c54 100644 --- a/src/EFCore.PG/Metadata/PostgresExtension.cs +++ b/src/EFCore.PG/Metadata/PostgresExtension.cs @@ -1,187 +1,188 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Text; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Infrastructure; -using Microsoft.EntityFrameworkCore.Internal; using Microsoft.EntityFrameworkCore.Metadata; using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal; using Npgsql.EntityFrameworkCore.PostgreSQL.Utilities; namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata { - public class PostgresExtension : IPostgresExtension + /// + /// Represents the metadata for a PostgreSQL extension. + /// + [PublicAPI] + public class PostgresExtension { - readonly IAnnotatable _annotatable; - readonly string _annotationName; - - internal PostgresExtension(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 PostgresExtension([NotNull] IAnnotatable annotatable, [NotNull] string annotationName) { - _annotatable = annotatable; - _annotationName = annotationName; + _annotatable = Check.NotNull(annotatable, nameof(annotatable)); + _annotationName = Check.NotNull(annotationName, nameof(annotationName)); } + /// + /// Gets or adds a from or to the . + /// + /// The annotatable from which to get or add the extension. + /// The extension schema or null to use the model's default schema. + /// The extension name. + /// The extension version. + /// + /// The from the . + /// + /// + /// + /// + [NotNull] public static PostgresExtension GetOrAddPostgresExtension( [NotNull] IMutableAnnotatable annotatable, - [NotNull] string extensionName) + [CanBeNull] string schema, + [NotNull] string name, + [CanBeNull] string version) { - var extension = (PostgresExtension)FindPostgresExtension(annotatable, extensionName); - if (extension != null) - return extension; + Check.NotNull(annotatable, nameof(annotatable)); + Check.NullButNotEmpty(schema, nameof(schema)); + Check.NotNull(name, nameof(name)); + + if (FindPostgresExtension(annotatable, schema, name) is PostgresExtension postgresExtension) + return postgresExtension; - extension = new PostgresExtension(annotatable, BuildAnnotationName(extensionName)); - extension.SetData(new PostgresExtensionData { Name = extensionName }); - return extension; + var annotationName = BuildAnnotationName(schema, name); + + return new PostgresExtension(annotatable, annotationName) { Version = version }; } - public static IPostgresExtension FindPostgresExtension( + /// + /// Gets or adds a from or to the . + /// + /// The annotatable from which to get or add the extension. + /// The extension name. + /// The extension version. + /// + /// The from the . + /// + /// + /// + [NotNull] + public static PostgresExtension GetOrAddPostgresExtension( + [NotNull] IMutableAnnotatable annotatable, + [NotNull] string name, + [CanBeNull] string version) + => GetOrAddPostgresExtension(annotatable, null, name, version); + + /// + /// Finds a in the , or returns null if not found. + /// + /// The annotatable to search for the extension. + /// The extension schema. The default schema is never used. + /// The extension name. + /// + /// The from the . + /// + /// + /// + /// + [CanBeNull] + public static PostgresExtension FindPostgresExtension( [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(name); + var annotationName = BuildAnnotationName(schema, name); return annotatable[annotationName] == null ? null : new PostgresExtension(annotatable, annotationName); } - static string BuildAnnotationName(string name) - => NpgsqlAnnotationNames.PostgresExtensionPrefix + name; - - public static IEnumerable GetPostgresExtensions([NotNull] IAnnotatable annotatable) - { - Check.NotNull(annotatable, nameof(annotatable)); - - return annotatable.GetAnnotations() - .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)) - .Select(a => new PostgresExtension(annotatable, a.Name)); - } - + [NotNull] + static string BuildAnnotationName(string schema, string name) + => schema != null + ? $"{NpgsqlAnnotationNames.PostgresExtensionPrefix}{schema}.{name}" + : $"{NpgsqlAnnotationNames.PostgresExtensionPrefix}{name}"; + + /// + /// Gets the collection of stored in the . + /// + /// The annotatable to search for annotations. + /// + /// The collection of stored in the . + /// + /// + [NotNull] + [ItemNotNull] + public static IEnumerable GetPostgresExtensions([NotNull] IAnnotatable annotatable) + => Check.NotNull(annotatable, nameof(annotatable)) + .GetAnnotations() + .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.PostgresExtensionPrefix, StringComparison.Ordinal)) + .Select(a => new PostgresExtension(annotatable, a.Name)); + + /// + /// The that stores the extension. + /// + [NotNull] public Annotatable Annotatable => (Annotatable)_annotatable; - public string Name => GetData().Name; - + /// + /// The extension schema or null to represent the default schema. + /// [CanBeNull] - public string Schema - { - get => GetData().Schema; - set - { - var data = GetData(); - data.Schema = value; - SetData(data); - } - } + public string Schema => GetData().Schema; + + /// + /// The extension name. + /// + [NotNull] + public string Name => GetData().Name; + /// + /// The extension version. + /// [CanBeNull] public string Version { get => GetData().Version; - set - { - var data = GetData(); - data.Version = value; - SetData(data); - } + set => SetData(value); } - PostgresExtensionData GetData() => PostgresExtensionData.Deserialize((string)Annotatable[_annotationName]); + (string Schema, string Name, string Version) GetData() + => Deserialize(Annotatable.FindAnnotation(_annotationName)); - void SetData(PostgresExtensionData data) - => Annotatable[_annotationName] = data.Serialize(); - - IAnnotatable IPostgresExtension.Annotatable => _annotatable; - - public override string ToString() + void SetData([CanBeNull] string version) { - var sb = new StringBuilder(); - if (Schema != null) - { - sb.Append(Schema); - sb.Append('.'); - } - sb.Append(Name); - if (Version != null) - { - sb.Append("' ("); - sb.Append(Version); - sb.Append(')'); - } - return sb.ToString(); + var data = GetData(); + Annotatable[_annotationName] = $"{data.Schema},{data.Name},{version}"; } - class PostgresExtensionData + static (string Schema, string Name, string Version) Deserialize([CanBeNull] IAnnotation annotation) { - public string Name { get; set; } - public string Schema { get; set; } - public string Version { get; set; } + if (annotation == null || !(annotation.Value is string value) || string.IsNullOrEmpty(value)) + return (null, null, null); - public string Serialize() + // TODO: Can't actually use schema and name...they might not be set when this is first called. + var schemaNameValue = value.Split(',').Select(x => x == "" ? null : x).ToArray(); + var schemaAndName = annotation.Name.Substring(NpgsqlAnnotationNames.PostgresExtensionPrefix.Length).Split('.'); + switch (schemaAndName.Length) { - var builder = new StringBuilder(); - - EscapeAndQuote(builder, Name); - builder.Append(", "); - EscapeAndQuote(builder, Schema); - builder.Append(", "); - EscapeAndQuote(builder, Version); - - return builder.ToString(); - } - - public static PostgresExtensionData Deserialize([NotNull] string value) - { - Check.NotEmpty(value, nameof(value)); - - try - { - var data = new PostgresExtensionData(); - - // ReSharper disable PossibleInvalidOperationException - var position = 0; - data.Name = ExtractValue(value, ref position); - data.Schema = ExtractValue(value, ref position); - data.Version = ExtractValue(value, ref position); - // ReSharper restore PossibleInvalidOperationException - - return data; - } - catch (Exception ex) - { - throw new ArgumentException(RelationalStrings.BadSequenceString, ex); - } - } - - private static string ExtractValue(string value, ref int position) - { - position = value.IndexOf('\'', position) + 1; - - var end = value.IndexOf('\'', position); - - while ((end + 1 < value.Length) - && (value[end + 1] == '\'')) - { - end = value.IndexOf('\'', end + 2); - } - - var extracted = value.Substring(position, end - position).Replace("''", "'"); - position = end + 1; - - return extracted.Length == 0 ? null : extracted; - } - - private static void EscapeAndQuote(StringBuilder builder, object value) - { - builder.Append("'"); - - if (value != null) - { - builder.Append(value.ToString().Replace("'", "''")); - } - - builder.Append("'"); + case 1: + return (null, schemaAndName[0], schemaNameValue[2]); + case 2: + return (schemaAndName[0], schemaAndName[1], schemaNameValue[2]); + default: + throw new ArgumentException($"Cannot parse extension name from annotation: {annotation.Name}"); } } } diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationBuilderExtensions.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationBuilderExtensions.cs index fe3c5638f..8ce80d074 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationBuilderExtensions.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationBuilderExtensions.cs @@ -2,12 +2,12 @@ using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.EntityFrameworkCore.Migrations.Operations; -using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata; using Npgsql.EntityFrameworkCore.PostgreSQL.Utilities; // ReSharper disable once CheckNamespace namespace Microsoft.EntityFrameworkCore { + // TODO: This is unused. Can we remove it? public static class NpgsqlMigrationBuilderExtensions { public static MigrationBuilder EnsurePostgresExtension( @@ -22,9 +22,7 @@ public static MigrationBuilder EnsurePostgresExtension( Check.NullButNotEmpty(version, nameof(schema)); var op = new AlterDatabaseOperation(); - var extension = PostgresExtension.GetOrAddPostgresExtension(op, name); - extension.Schema = schema; - extension.Version = version; + op.Npgsql().GetOrAddPostgresExtension(schema, name, version); builder.Operations.Add(op); return builder; @@ -35,15 +33,13 @@ public static MigrationBuilder CreatePostgresExtension( this MigrationBuilder builder, [NotNull] string name, string schema = null, - string version = null - ) + string version = null) => EnsurePostgresExtension(builder, name, schema, version); [Obsolete("This no longer does anything and should be removed.")] public static MigrationBuilder DropPostgresExtension( this MigrationBuilder builder, - [NotNull] string name - ) + [NotNull] string name) => builder; } } diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs index 9d335b33c..82cd01489 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs @@ -651,7 +651,7 @@ protected override void Generate( } protected virtual void GenerateCreateExtension( - IPostgresExtension extension, + PostgresExtension extension, IModel model, MigrationCommandListBuilder builder) { diff --git a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs index 1a209ba3d..201449503 100644 --- a/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs +++ b/src/EFCore.PG/Scaffolding/Internal/NpgsqlDatabaseModelFactory.cs @@ -880,7 +880,8 @@ static void GetExtensions([NotNull] NpgsqlConnection connection, [NotNull] Datab if (name == "plpgsql") // Implicitly installed in all PG databases continue; - PostgresExtension.GetOrAddPostgresExtension(databaseModel, name); + // TODO: how/should we query the schema? + databaseModel.Npgsql().GetOrAddPostgresExtension(null, name, installedVersion); } } } diff --git a/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs b/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs index 3809caead..8fac79ead 100644 --- a/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs +++ b/test/EFCore.PG.FunctionalTests/NpgsqlMigrationSqlGeneratorTest.cs @@ -692,7 +692,7 @@ public void RenameIndexOperation() public void EnsurePostgresExtension() { var op = new AlterDatabaseOperation(); - PostgresExtension.GetOrAddPostgresExtension(op, "hstore"); + op.Npgsql().GetOrAddPostgresExtension(null, "hstore", null); Generate(op); Assert.Equal( @@ -704,8 +704,7 @@ public void EnsurePostgresExtension() public void EnsurePostgresExtension_with_schema() { var op = new AlterDatabaseOperation(); - var extension = PostgresExtension.GetOrAddPostgresExtension(op, "hstore"); - extension.Schema = "myschema"; + op.Npgsql().GetOrAddPostgresExtension("myschema", "hstore", null); Generate(op); Assert.Equal(