From b33f40e777e0af996d73d19bf57932a40e8ba638 Mon Sep 17 00:00:00 2001 From: Austin Drenski Date: Wed, 28 Nov 2018 21:24:03 -0500 Subject: [PATCH] Clean up schemas/annotation interaction - Default schema is only considered at generation-time. - Simplifies the access patterns for `PostgresRange`. --- src/EFCore.PG/Metadata/PostgresRange.cs | 112 +++++++----------- .../NpgsqlMigrationsSqlGenerator.cs | 21 ++-- 2 files changed, 55 insertions(+), 78 deletions(-) diff --git a/src/EFCore.PG/Metadata/PostgresRange.cs b/src/EFCore.PG/Metadata/PostgresRange.cs index ce6823fd2..59a565a70 100644 --- a/src/EFCore.PG/Metadata/PostgresRange.cs +++ b/src/EFCore.PG/Metadata/PostgresRange.cs @@ -16,8 +16,8 @@ public class PostgresRange internal PostgresRange([NotNull] IAnnotatable annotatable, [NotNull] string annotationName) { - _annotatable = annotatable; - _annotationName = annotationName; + _annotatable = Check.NotNull(annotatable, nameof(annotatable)); + _annotationName = Check.NotNull(annotationName, nameof(annotationName)); } public static PostgresRange GetOrAddPostgresRange( @@ -29,37 +29,29 @@ public static PostgresRange GetOrAddPostgresRange( string subtypeOpClass = null, string collation = null, string subtypeDiff = null) - { - if (FindPostgresRange(annotatable, schema, name) is PostgresRange rangeType) - return rangeType; - - rangeType = new PostgresRange(annotatable, BuildAnnotationName(annotatable, schema, name)); - rangeType.SetData(subtype, canonicalFunction, subtypeOpClass, collation, subtypeDiff); - return rangeType; - } - - [CanBeNull] - public static PostgresRange FindPostgresRange( - [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(subtype, nameof(subtype)); - var annotationName = BuildAnnotationName(annotatable, schema, name); + var annotationName = + schema != null + ? $"{NpgsqlAnnotationNames.RangePrefix}{schema}.{name}" + : $"{NpgsqlAnnotationNames.RangePrefix}{name}"; - return annotatable[annotationName] == null ? null : new PostgresRange(annotatable, annotationName); + return annotatable[annotationName] != null + ? new PostgresRange(annotatable, annotationName) + : new PostgresRange(annotatable, annotationName) + { + CanonicalFunction = canonicalFunction, + Collation = collation, + Subtype = subtype, + SubtypeDiff = subtypeDiff, + SubtypeOpClass = subtypeOpClass + }; } - [NotNull] - static string BuildAnnotationName([NotNull] IAnnotatable annotatable, [CanBeNull] string schema, [NotNull] string name) - => !string.IsNullOrEmpty(schema) - ? $"{NpgsqlAnnotationNames.RangePrefix}{schema}.{name}" - : annotatable[RelationalAnnotationNames.DefaultSchema] is string defaultSchema && !string.IsNullOrEmpty(defaultSchema) - ? $"{NpgsqlAnnotationNames.RangePrefix}{defaultSchema}.{name}" - : $"{NpgsqlAnnotationNames.RangePrefix}{name}"; - [NotNull] public static IEnumerable GetPostgresRanges([NotNull] IAnnotatable annotatable) => Check.NotNull(annotatable, nameof(annotatable)) @@ -67,11 +59,8 @@ public static IEnumerable GetPostgresRanges([NotNull] IAnnotatabl .Where(a => a.Name.StartsWith(NpgsqlAnnotationNames.RangePrefix, StringComparison.Ordinal)) .Select(a => new PostgresRange(annotatable, a.Name)); - [NotNull] - public Annotatable Annotatable => (Annotatable)_annotatable; - [CanBeNull] - public string Schema => GetData().Schema ?? (string)_annotatable[RelationalAnnotationNames.DefaultSchema]; + public string Schema => GetData().Schema; [NotNull] public string Name => GetData().Name; @@ -80,75 +69,56 @@ public static IEnumerable GetPostgresRanges([NotNull] IAnnotatabl public string Subtype { get => GetData().Subtype; - set - { - var x = GetData(); - var (_, _, _, canonicalFunction, subtypeOpClass, collation, subtypeDiff) = GetData(); - SetData(value, canonicalFunction, subtypeOpClass, collation, subtypeDiff); - } + private set => SetData(subtype: value); } [CanBeNull] public string CanonicalFunction { get => GetData().CanonicalFunction; - set - { - var x = GetData(); - var (_, _, subtype, _, subtypeOpClass, collation, subtypeDiff) = GetData(); - SetData(subtype, value, subtypeOpClass, collation, subtypeDiff); - } + private set => SetData(canonicalFunction: value); } [CanBeNull] public string SubtypeOpClass { get => GetData().SubtypeOpClass; - set - { - var x = GetData(); - var (_, _, subtype, canonicalFunction, _, collation, subtypeDiff) = GetData(); - SetData(subtype, canonicalFunction, value, collation, subtypeDiff); - } + private set => SetData(subtypeOpClass: value); } [CanBeNull] public string Collation { get => GetData().Collation; - set - { - var x = GetData(); - var (_, _, subtype, canonicalFunction, subtypeOpClass, _, subtypeDiff) = GetData(); - SetData(subtype, canonicalFunction, subtypeOpClass, value, subtypeDiff); - } + private set => SetData(collation: value); } [CanBeNull] public string SubtypeDiff { get => GetData().SubtypeDiff; - set - { - var x = GetData(); - var (_, _, subtype, canonicalFunction, subtypeOpClass, collation, _) = GetData(); - SetData(subtype, canonicalFunction, subtypeOpClass, collation, value); - } + private set => SetData(subtypeDiff: value); } - (string Schema, string Name, string Subtype, string CanonicalFunction, string SubtypeOpClass, string Collation, - string SubtypeDiff) GetData() - => !(Annotatable[_annotationName] is string annotationValue) - ? (null, null, null, null, null, null, null) - : Deserialize(_annotationName, annotationValue); + (string Schema, string Name, string Subtype, string CanonicalFunction, string SubtypeOpClass, string Collation, string SubtypeDiff) GetData() + => Deserialize(_annotationName, _annotatable[_annotationName] as string); - void SetData(string subtype, string canonicalFunction, string subtypeOpClass, string collation, string subtypeDiff) - => Annotatable[_annotationName] = $"{subtype},{canonicalFunction},{subtypeOpClass},{collation},{subtypeDiff}"; + void SetData(string subtype = null, string canonicalFunction = null, string subtypeOpClass = null, string collation = null, string subtypeDiff = null) + { + ((Annotatable)_annotatable)[_annotationName] = + string.Join(",", + subtype ?? Subtype, + canonicalFunction ?? CanonicalFunction, + subtypeOpClass ?? SubtypeOpClass, + collation ?? Collation, + subtypeDiff ?? SubtypeDiff); + } - static (string Schema, string Name, string Subtype, string CanonicalFunction, string SubtypeOpClass, string collation, string SubtypeDiff) - Deserialize([NotNull] string annotationName, [NotNull] string annotationValue) + static (string Schema, string Name, string Subtype, string CanonicalFunction, string SubtypeOpClass, string Collation, string SubtypeDiff) + Deserialize([NotNull] string annotationName, [CanBeNull] string annotationValue) { - Check.NotEmpty(annotationValue, nameof(annotationValue)); + if (annotationValue == null) + return (null, null, null, null, null, null, null); var elements = annotationValue.Split(',').ToArray(); if (elements.Length != 5) diff --git a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs index bfc10b98e..41704d69e 100644 --- a/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs +++ b/src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs @@ -764,7 +764,7 @@ protected virtual void GenerateRangeStatements( foreach (var rangeTypeToDrop in operation.Npgsql().OldPostgresRanges .Where(oe => operation.Npgsql().PostgresRanges.All(ne => ne.Name != oe.Name))) { - GenerateDropRange(rangeTypeToDrop, builder); + GenerateDropRange(rangeTypeToDrop, model, builder); } if (operation.Npgsql().PostgresRanges.FirstOrDefault(nr => @@ -780,14 +780,16 @@ protected virtual void GenerateCreateRange( [NotNull] IModel model, [NotNull] MigrationCommandListBuilder builder) { + var schema = rangeType.Schema ?? model.Npgsql().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 ranges, so we always ensure schema on range creation. - if (rangeType.Schema != null) - Generate(new EnsureSchemaOperation { Name = rangeType.Schema }, model, builder); + if (schema != null) + Generate(new EnsureSchemaOperation { Name = schema }, model, builder); builder .Append("CREATE TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, rangeType.Schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, schema)) .AppendLine($" AS RANGE (") .IncrementIndent(); @@ -811,13 +813,18 @@ protected virtual void GenerateCreateRange( .AppendLine(");"); } - protected virtual void GenerateDropRange([NotNull] PostgresRange rangeType, [NotNull] MigrationCommandListBuilder builder) + protected virtual void GenerateDropRange( + [NotNull] PostgresRange rangeType, + [NotNull] IModel model, + [NotNull] MigrationCommandListBuilder builder) { + var schema = rangeType.Schema ?? model.Npgsql().DefaultSchema; + builder .Append("DROP TYPE ") - .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, rangeType.Schema)) + .Append(Dependencies.SqlGenerationHelper.DelimitIdentifier(rangeType.Name, schema)) .AppendLine(";"); - } + } #endregion Range management