Skip to content

Commit

Permalink
Address more API Review feedback
Browse files Browse the repository at this point in the history 
...with a bit of yak shaving
  • Loading branch information
@bricelam @smitpatel
bricelam authored and smitpatel committed Aug 21, 2019
1 parent 2c3b463 commit f1709f4
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 224 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public virtual void Generate(string builderName, IModel model, IndentedStringBui
stringBuilder.AppendLine(";");
}

GenerateEntityTypes(builderName, Sort(model.GetEntityTypes().Where(et => !et.MigrationsIgnored()).ToList()), stringBuilder);
GenerateEntityTypes(builderName, Sort(model.GetEntityTypes().Where(et => !et.IsIgnoredByMigrations()).ToList()), stringBuilder);
}

private static IReadOnlyList<IEntityType> Sort(IReadOnlyList<IEntityType> entityTypes)
Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,11 @@ public static void SetComment(
/// </summary>
/// <param name="entityType">The entity type.</param>
/// <returns>A value indicating whether the entity type is ignored by Migrations.</returns>
public static bool MigrationsIgnored([NotNull] this IEntityType entityType)
public static bool IsIgnoredByMigrations([NotNull] this IEntityType entityType)
{
if (entityType.BaseType != null)
{
return entityType.BaseType.MigrationsIgnored();
return entityType.BaseType.IsIgnoredByMigrations();
}

var viewDefinition = entityType.FindAnnotation(RelationalAnnotationNames.ViewDefinition);
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Metadata/Internal/TableMapping.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public virtual IEnumerable<ICheckConstraint> GetCheckConstraints()
public static IReadOnlyList<TableMapping> GetTableMappings([NotNull] IModel model)
{
var tables = new Dictionary<(string Schema, string TableName), List<IEntityType>>();
foreach (var entityType in model.GetEntityTypes().Where(et => !et.MigrationsIgnored()))
foreach (var entityType in model.GetEntityTypes().Where(et => !et.IsIgnoredByMigrations()))
{
var fullName = (entityType.GetSchema(), entityType.GetTableName());
if (!tables.TryGetValue(fullName, out var mappedEntityTypes))
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2150,7 +2150,7 @@ protected virtual bool HasDifferences([NotNull] IEnumerable<IAnnotation> source,
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected virtual IEnumerable<string> GetSchemas([NotNull] IModel model)
=> model.GetRootEntityTypes().Where(t => !t.MigrationsIgnored()).Select(t => t.GetSchema())
=> model.GetRootEntityTypes().Where(t => !t.IsIgnoredByMigrations()).Select(t => t.GetSchema())
.Concat(model.GetSequences().Select(s => s.Schema))
.Where(s => !string.IsNullOrEmpty(s))
.Distinct();
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1580,7 +1580,7 @@ protected virtual IEnumerable<IEntityType> FindEntityTypes(
[CanBeNull] string schema,
[NotNull] string tableName)
=> model?.GetEntityTypes().Where(
t => t.GetTableName() == tableName && t.GetSchema() == schema && !t.MigrationsIgnored());
t => t.GetTableName() == tableName && t.GetSchema() == schema && !t.IsIgnoredByMigrations());

/// <summary>
/// <para>
Expand Down
7 changes: 0 additions & 7 deletions src/EFCore.Relational/TypeForwards.cs
View file

This file was deleted.

302 changes: 111 additions & 191 deletions src/EFCore.SqlServer/Migrations/SqlServerMigrationsSqlGenerator.cs
View file

Large diffs are not rendered by default.

90 changes: 70 additions & 20 deletions test/EFCore.SqlServer.FunctionalTests/SqlServerMigrationSqlGeneratorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ PRIMARY KEY ([Id]),
CHECK (SSN > 0),
FOREIGN KEY ([EmployerId]) REFERENCES [Companies] ([Id])
);
EXEC sp_addextendedproperty @name = N'Comment', @value = N'Table comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People';
EXEC sp_addextendedproperty @name = N'Comment', @value = N'Employer ID comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'EmployerId';
EXEC sp_addextendedproperty 'MS_Description', N'Table comment', 'SCHEMA', N'dbo', 'TABLE', N'People';
EXEC sp_addextendedproperty 'MS_Description', N'Employer ID comment', 'SCHEMA', N'dbo', 'TABLE', N'People', 'COLUMN', N'EmployerId';
");
}

[ConditionalFact]
public void CreateTableOperation_default_schema_with_comment()
public void CreateTableOperation_default_schema_with_comments()
{
Generate(
new CreateTableOperation
Expand All @@ -53,18 +53,68 @@ public void CreateTableOperation_default_schema_with_comment()
IsNullable = false,
Comment = "ID comment"
},
new AddColumnOperation
{
Name = "Name",
Table = "People",
ClrType = typeof(string),
IsNullable = false,
Comment = "Name comment"
},
},
Comment = "Table comment"
});

AssertSql(
@"CREATE TABLE [People] (
[Id] int NOT NULL
[Id] int NOT NULL,
[Name] nvarchar(max) NOT NULL
);
DECLARE @defaultSchema AS sysname;
SET @defaultSchema = SCHEMA_NAME();
EXEC sp_addextendedproperty 'MS_Description', N'Table comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People';
EXEC sp_addextendedproperty 'MS_Description', N'ID comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'Id';
EXEC sp_addextendedproperty 'MS_Description', N'Name comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'Name';
");
}

[ConditionalFact]
public void CreateTableOperation_default_schema_with_column_comments()
{
Generate(
new CreateTableOperation
{
Name = "People",
Columns =
{
new AddColumnOperation
{
Name = "Id",
Table = "People",
ClrType = typeof(int),
IsNullable = false,
Comment = "ID comment"
},
new AddColumnOperation
{
Name = "Name",
Table = "People",
ClrType = typeof(string),
IsNullable = false,
Comment = "Name comment"
},
}
});

AssertSql(
@"CREATE TABLE [People] (
[Id] int NOT NULL,
[Name] nvarchar(max) NOT NULL
);
DECLARE @schema AS nvarchar(max);
SET @schema = SCHEMA_NAME();
EXEC sp_addextendedproperty @name = N'Comment', @value = N'Table comment', @level0type = N'Schema', @level0name = @schema, @level1type = N'Table', @level1name = N'People';
EXEC sp_addextendedproperty @name = N'Comment', @value = N'ID comment', @level0type = N'Schema', @level0name = @schema, @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'Id';
DECLARE @defaultSchema AS sysname;
SET @defaultSchema = SCHEMA_NAME();
EXEC sp_addextendedproperty 'MS_Description', N'ID comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'Id';
EXEC sp_addextendedproperty 'MS_Description', N'Name comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'Name';
");
}

Expand Down Expand Up @@ -107,7 +157,7 @@ public void AlterTableOperation_with_new_comment()
});

AssertSql(
@"EXEC sp_addextendedproperty @name = N'Comment', @value = N'My Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People';
@"EXEC sp_addextendedproperty 'MS_Description', N'My Comment', 'SCHEMA', N'dbo', 'TABLE', N'People';
");
}

Expand All @@ -134,8 +184,8 @@ public void AlterTableOperation_with_different_comment_to_existing()
});

AssertSql(
@"EXEC sp_dropextendedproperty @name = N'Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People';
EXEC sp_addextendedproperty @name = N'Comment', @value = N'My Comment 2', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People';
@"EXEC sp_dropextendedproperty 'MS_Description', 'SCHEMA', N'dbo', 'TABLE', N'People';
EXEC sp_addextendedproperty 'MS_Description', N'My Comment 2', 'SCHEMA', N'dbo', 'TABLE', N'People';
");
}

Expand All @@ -161,7 +211,7 @@ public void AlterTableOperation_removing_comment()
});

AssertSql(
@"EXEC sp_dropextendedproperty @name = N'Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People';
@"EXEC sp_dropextendedproperty 'MS_Description', 'SCHEMA', N'dbo', 'TABLE', N'People';
");
}

Expand Down Expand Up @@ -407,9 +457,9 @@ public virtual void AddColumnOperation_with_comment()

AssertSql(
@"ALTER TABLE [People] ADD [FullName] nvarchar(max) NOT NULL;
DECLARE @schema AS nvarchar(max);
SET @schema = SCHEMA_NAME();
EXEC sp_addextendedproperty @name = N'Comment', @value = N'My comment', @level0type = N'Schema', @level0name = @schema, @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'FullName';
DECLARE @defaultSchema AS sysname;
SET @defaultSchema = SCHEMA_NAME();
EXEC sp_addextendedproperty 'MS_Description', N'My comment', 'SCHEMA', @defaultSchema, 'TABLE', N'People', 'COLUMN', N'FullName';
");
}

Expand All @@ -428,7 +478,7 @@ public virtual void AddColumnOperation_with_comment_non_default_schema()

AssertSql(
@"ALTER TABLE [my].[People] ADD [FullName] nvarchar(max) NOT NULL;
EXEC sp_addextendedproperty @name = N'Comment', @value = N'My comment', @level0type = N'Schema', @level0name = N'my', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'FullName';
EXEC sp_addextendedproperty 'MS_Description', N'My comment', 'SCHEMA', N'my', 'TABLE', N'People', 'COLUMN', N'FullName';
");
}

Expand Down Expand Up @@ -1152,7 +1202,7 @@ FROM [sys].[default_constraints] [d]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[dbo].[People]') AND [c].[name] = N'LuckyNumber');
IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [dbo].[People] DROP CONSTRAINT [' + @var0 + '];');
ALTER TABLE [dbo].[People] ALTER COLUMN [LuckyNumber] int NOT NULL;
EXEC sp_addextendedproperty @name = N'Comment', @value = N'My Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'LuckyNumber';
EXEC sp_addextendedproperty 'MS_Description', N'My Comment', 'SCHEMA', N'dbo', 'TABLE', N'People', 'COLUMN', N'LuckyNumber';
");
}

Expand Down Expand Up @@ -1191,8 +1241,8 @@ FROM [sys].[default_constraints] [d]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[dbo].[People]') AND [c].[name] = N'Name');
IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [dbo].[People] DROP CONSTRAINT [' + @var0 + '];');
ALTER TABLE [dbo].[People] ALTER COLUMN [Name] nvarchar(max) NOT NULL;
EXEC sp_dropextendedproperty @name = N'Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'Name';
EXEC sp_addextendedproperty @name = N'Comment', @value = N'My Comment 2', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'Name';
EXEC sp_dropextendedproperty 'MS_Description', 'SCHEMA', N'dbo', 'TABLE', N'People', 'COLUMN', N'Name';
EXEC sp_addextendedproperty 'MS_Description', N'My Comment 2', 'SCHEMA', N'dbo', 'TABLE', N'People', 'COLUMN', N'Name';
");
}

Expand Down Expand Up @@ -1230,7 +1280,7 @@ FROM [sys].[default_constraints] [d]
WHERE ([d].[parent_object_id] = OBJECT_ID(N'[dbo].[People]') AND [c].[name] = N'Name');
IF @var0 IS NOT NULL EXEC(N'ALTER TABLE [dbo].[People] DROP CONSTRAINT [' + @var0 + '];');
ALTER TABLE [dbo].[People] ALTER COLUMN [Name] nvarchar(max) NOT NULL;
EXEC sp_dropextendedproperty @name = N'Comment', @level0type = N'Schema', @level0name = N'dbo', @level1type = N'Table', @level1name = N'People', @level2type = N'Column', @level2name = N'Name';
EXEC sp_dropextendedproperty 'MS_Description', 'SCHEMA', N'dbo', 'TABLE', N'People', 'COLUMN', N'Name';
");
}

Expand Down

0 comments on commit f1709f4

Please sign in to comment.