Skip to content

Commit

Permalink
Follow-up to schema-enums (605)
Browse files Browse the repository at this point in the history 
- Includes changes resulting from schema-ranges (626):
  - Moves default schema handling out of the migrations generator
  - Injects `ISqlGenerationHelper` instead of hard-coding
  • Loading branch information
@austindrenski
austindrenski committed Oct 30, 2018
1 parent b3cca03 commit 25fa864
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 43 deletions.
23 changes: 18 additions & 5 deletions src/EFCore.PG/Metadata/PostgresEnum.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public static PostgresEnum GetOrAddPostgresEnum(
if (FindPostgresEnum(annotatable, schema, name) is PostgresEnum enumType)
return enumType;

enumType = new PostgresEnum(annotatable, BuildAnnotationName(schema, name));
enumType = new PostgresEnum(annotatable, BuildAnnotationName(annotatable, schema, name));
enumType.SetData(labels);
return enumType;
}
Expand All @@ -48,13 +48,22 @@ public static PostgresEnum FindPostgresEnum(
Check.NotNull(annotatable, nameof(annotatable));
Check.NotEmpty(name, nameof(name));

var annotationName = BuildAnnotationName(schema, name);
var annotationName = BuildAnnotationName(annotatable, schema, name);

return annotatable[annotationName] == null ? null : new PostgresEnum(annotatable, annotationName);
}

static string BuildAnnotationName(string schema, string name)
=> NpgsqlAnnotationNames.EnumPrefix + (schema == null ? name : schema + '.' + name);
[NotNull]
static string BuildAnnotationName(IAnnotatable annotatable, string schema, string name)
{
if (schema != null)
return $"{NpgsqlAnnotationNames.EnumPrefix}{schema}.{name}";

if (annotatable[RelationalAnnotationNames.DefaultSchema] is string defaultSchema)
return $"{NpgsqlAnnotationNames.EnumPrefix}{defaultSchema}.{name}";

return $"{NpgsqlAnnotationNames.EnumPrefix}{name}";
}

public static IEnumerable<PostgresEnum> GetPostgresEnums([NotNull] IAnnotatable annotatable)
{
Expand All @@ -65,12 +74,16 @@ public static IEnumerable<PostgresEnum> GetPostgresEnums([NotNull] IAnnotatable
.Select(a => new PostgresEnum(annotatable, a.Name));
}

[NotNull]
public Annotatable Annotatable => (Annotatable)_annotatable;

public string Schema => GetData().Schema;
[CanBeNull]
public string Schema => GetData().Schema ?? (string)_annotatable[RelationalAnnotationNames.DefaultSchema];

[NotNull]
public string Name => GetData().Name;

[NotNull]
public string[] Labels
{
get => GetData().Labels;
Expand Down
27 changes: 10 additions & 17 deletions src/EFCore.PG/Migrations/NpgsqlMigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,10 @@ protected override void Generate(
Check.NotNull(operation, nameof(operation));
Check.NotNull(builder, nameof(builder));

// Alter operations may not have a default schema attached. If not, try to attach one for schema-qualified types.
if (operation[RelationalAnnotationNames.DefaultSchema] == null)
operation[RelationalAnnotationNames.DefaultSchema] = model[RelationalAnnotationNames.DefaultSchema];

GenerateEnumStatements(operation, model, builder);
GenerateRangeStatements(operation, model, builder);

Expand Down Expand Up @@ -703,7 +707,7 @@ protected virtual void GenerateEnumStatements(
foreach (var enumTypeToDrop in PostgresEnum.GetPostgresEnums(operation.OldDatabase)
.Where(oe => PostgresEnum.GetPostgresEnums(operation).All(ne => ne.Name != oe.Name)))
{
GenerateDropEnum(enumTypeToDrop, operation.OldDatabase, builder);
GenerateDropEnum(enumTypeToDrop, builder);
}

// TODO: Some forms of enum alterations are actually supported...
Expand All @@ -720,16 +724,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));
Expand All @@ -745,16 +747,11 @@ protected virtual void GenerateCreateEnum(
builder.AppendLine(");");
}

protected virtual void GenerateDropEnum(
[NotNull] PostgresEnum enumType,
[CanBeNull] IAnnotatable oldDatabase,
[NotNull] MigrationCommandListBuilder builder)
protected virtual void GenerateDropEnum([NotNull] PostgresEnum enumType, [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(";");
}

Expand Down Expand Up @@ -1118,10 +1115,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;

/// <summary>
/// True if <see cref="_postgresVersion"/> is null, greater than, or equal to the specified version.
/// </summary>
Expand Down
20 changes: 7 additions & 13 deletions src/EFCore.PG/Storage/Internal/Mapping/NpgsqlEnumTypeMapping.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,21 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping
{
public class NpgsqlEnumTypeMapping : RelationalTypeMapping
{
[NotNull] static readonly NpgsqlSqlGenerationHelper SqlGenerationHelper =
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies());

[NotNull] readonly ISqlGenerationHelper _sqlGenerationHelper;
[NotNull] readonly INpgsqlNameTranslator _nameTranslator;

[CanBeNull] readonly string _storeTypeSchema;

/// <summary>
/// Translates the CLR member value to the PostgreSQL value label.
/// </summary>
[NotNull] readonly Dictionary<object, string> _members;

[NotNull]
public override string StoreType => SqlGenerationHelper.DelimitIdentifier(base.StoreType, _storeTypeSchema);

public NpgsqlEnumTypeMapping(
[NotNull] string storeType,
[CanBeNull] string storeTypeSchema,
[NotNull] Type enumType,
[NotNull] ISqlGenerationHelper sqlGenerationHelper,
[CanBeNull] INpgsqlNameTranslator nameTranslator = null)
: base(storeType, enumType)
: base(sqlGenerationHelper.DelimitIdentifier(storeType, storeTypeSchema), enumType)
{
if (!enumType.IsEnum || !enumType.IsValueType)
throw new ArgumentException($"Enum type mappings require a CLR enum. {enumType.FullName} is not an enum.");
Expand All @@ -37,23 +31,23 @@ public NpgsqlEnumTypeMapping(
nameTranslator = NpgsqlConnection.GlobalTypeMapper.DefaultNameTranslator;

_nameTranslator = nameTranslator;
_storeTypeSchema = storeTypeSchema;
_sqlGenerationHelper = sqlGenerationHelper;
_members = CreateValueMapping(enumType, nameTranslator);
}

protected NpgsqlEnumTypeMapping(
RelationalTypeMappingParameters parameters,
[CanBeNull] string storeTypeSchema,
[NotNull] ISqlGenerationHelper sqlGenerationHelper,
[NotNull] INpgsqlNameTranslator nameTranslator)
: base(parameters)
{
_nameTranslator = nameTranslator;
_storeTypeSchema = storeTypeSchema;
_sqlGenerationHelper = sqlGenerationHelper;
_members = CreateValueMapping(parameters.CoreParameters.ClrType, nameTranslator);
}

protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
=> new NpgsqlEnumTypeMapping(parameters, _storeTypeSchema, _nameTranslator);
=> new NpgsqlEnumTypeMapping(parameters, _sqlGenerationHelper, _nameTranslator);

protected override string GenerateNonNullSqlLiteral(object value) => $"'{_members[value]}'::{StoreType}";

Expand Down
10 changes: 5 additions & 5 deletions src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc
StoreTypeMappings = new ConcurrentDictionary<string, RelationalTypeMapping[]>(storeTypeMappings, StringComparer.OrdinalIgnoreCase);
ClrTypeMappings = new ConcurrentDictionary<Type, RelationalTypeMapping>(clrTypeMappings);

LoadUserDefinedTypeMappings();
LoadUserDefinedTypeMappings(sqlGenerationHelper);

if (npgsqlOptions == null)
return;
Expand All @@ -263,15 +263,15 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc
/// To be used in case user-defined mappings are added late, after this TypeMappingSource has already been initialized.
/// This is basically only for test usage.
/// </summary>
public void LoadUserDefinedTypeMappings()
public void LoadUserDefinedTypeMappings([NotNull] ISqlGenerationHelper sqlGenerationHelper)
{
SetupEnumMappings();
SetupEnumMappings(sqlGenerationHelper);
}

/// <summary>
/// Gets all global enum mappings from the ADO.NET layer and creates mappings for them
/// </summary>
void SetupEnumMappings()
void SetupEnumMappings([NotNull] ISqlGenerationHelper sqlGenerationHelper)
{
foreach (var adoMapping in NpgsqlConnection.GlobalTypeMapper.Mappings.Where(m => m.TypeHandlerFactory is IEnumTypeHandlerFactory))
{
Expand All @@ -290,7 +290,7 @@ void SetupEnumMappings()
var schema = components.Length > 1 ? components.First() : null;
var name = components.Length > 1 ? string.Join(null, components.Skip(1)) : storeType;

var mapping = new NpgsqlEnumTypeMapping(name, schema, clrType, nameTranslator);
var mapping = new NpgsqlEnumTypeMapping(name, schema, clrType, sqlGenerationHelper, nameTranslator);
ClrTypeMappings[clrType] = mapping;
StoreTypeMappings[mapping.StoreType] = new RelationalTypeMapping[] { mapping };
}
Expand Down
2 changes: 1 addition & 1 deletion test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -707,7 +707,7 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
base.OnModelCreating(modelBuilder, context);

NpgsqlConnection.GlobalTypeMapper.MapEnum<Mood>();
((NpgsqlTypeMappingSource)context.GetService<ITypeMappingSource>()).LoadUserDefinedTypeMappings();
((NpgsqlTypeMappingSource)context.GetService<ITypeMappingSource>()).LoadUserDefinedTypeMappings(context.GetService<ISqlGenerationHelper>());

modelBuilder.HasPostgresExtension("hstore");
modelBuilder.ForNpgsqlHasEnum("mood", new[] { "happy", "sad" });
Expand Down
18 changes: 16 additions & 2 deletions test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -267,14 +267,28 @@ public void GenerateSqlLiteral_returns_json_literal()
[Fact]
public void GenerateSqlLiteral_returns_enum_literal()
{
var mapping = new NpgsqlEnumTypeMapping("dummy_enum", null, typeof(DummyEnum), new NpgsqlSnakeCaseNameTranslator());
var mapping =
new NpgsqlEnumTypeMapping(
"dummy_enum",
null,
typeof(DummyEnum),
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
new NpgsqlSnakeCaseNameTranslator());

Assert.Equal("'sad'::dummy_enum", mapping.GenerateSqlLiteral(DummyEnum.Sad));
}

[Fact]
public void GenerateSqlLiteral_returns_enum_uppercase_literal()
{
var mapping = new NpgsqlEnumTypeMapping("DummyEnum", null, typeof(DummyEnum), new NpgsqlSnakeCaseNameTranslator());
var mapping =
new NpgsqlEnumTypeMapping(
"DummyEnum",
null,
typeof(DummyEnum),
new NpgsqlSqlGenerationHelper(new RelationalSqlGenerationHelperDependencies()),
new NpgsqlSnakeCaseNameTranslator());

Assert.Equal("'sad'::\"DummyEnum\"", mapping.GenerateSqlLiteral(DummyEnum.Sad));
}

Expand Down

0 comments on commit 25fa864

Please sign in to comment.