Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix for 20662. Linebreaks in HasData() in migration #20699

Merged
merged 1 commit into from
Apr 22, 2020
Merged

Fix for 20662. Linebreaks in HasData() in migration #20699

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/EFCore.Design/Design/Internal/CSharpHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,8 @@ public virtual string Namespace(params string[] name)
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual string Literal(string value) =>
value.Contains('\n') || value.Contains('\r')
? "@\"" + value.Replace("\"", "\"\"") + "\""
: "\"" + value.Replace("\\", "\\\\").Replace("\"", "\\\"") + "\"";
// do not use @"" syntax as in Migrations this can get indented at a newline and so add spaces to the literal
"\"" + value.Replace(@"\", @"\\").Replace("\"", "\\\"").Replace("\n", @"\n").Replace("\r", @"\r") + "\"";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you file an issue to use verbatim strings where it makes sense? (I always wanted us to generate @"...(localdb)\MSSQLLocalDB..." in OnConfiguring)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Raised #20723


/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
Expand Down
4 changes: 2 additions & 2 deletions test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ public void Literal_works_when_many_ByteArray() =>
[ConditionalFact]
public void Literal_works_when_multiline_string() =>
Literal_works(
"multi-line" + Environment.NewLine + "string with \"",
"@\"multi-line" + Environment.NewLine + "string with \"\"\"");
"multi-line\r\nstring\nwith\r\"",
"\"multi-line\\r\\nstring\\nwith\\r\\\"\"");

[ConditionalFact]
[UseCulture("de-DE")]
Expand Down
159 changes: 159 additions & 0 deletions test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2717,6 +2717,54 @@ public void InsertDataOperation_required_args_multiple_rows()
});
}

[ConditionalFact]
public void InsertDataOperation_args_with_linebreaks()
{
Test(
new InsertDataOperation
{
Schema = "dbo",
Table = "TestLineBreaks",
Columns = new[] { "Id", "Description" },
Values = new object[,]
{
{ 0, "Contains\r\na Windows linebreak" },
{ 1, "Contains a\nLinux linebreak" },
{ 2, "Contains a single Backslash r,\rjust in case" },
}
},
"mb.InsertData("
+ _eol
+ " schema: \"dbo\","
+ _eol
+ " table: \"TestLineBreaks\","
+ _eol
+ " columns: new[] { \"Id\", \"Description\" },"
+ _eol
+ " values: new object[,]"
+ _eol
+ " {"
+ _eol
+ " { 0, \"Contains\\r\\na Windows linebreak\" },"
+ _eol
+ " { 1, \"Contains a\\nLinux linebreak\" },"
+ _eol
+ " { 2, \"Contains a single Backslash r,\\rjust in case\" }"
+ _eol
+ " });",
operation =>
{
Assert.Equal("dbo", operation.Schema);
Assert.Equal("TestLineBreaks", operation.Table);
Assert.Equal(2, operation.Columns.Length);
Assert.Equal(3, operation.Values.GetLength(0));
Assert.Equal(2, operation.Values.GetLength(1));
Assert.Equal("Contains\r\na Windows linebreak", operation.Values[0, 1]);
Assert.Equal("Contains a\nLinux linebreak", operation.Values[1, 1]);
Assert.Equal("Contains a single Backslash r,\rjust in case", operation.Values[2, 1]);
});
}

[ConditionalFact]
public void DeleteDataOperation_all_args()
{
Expand Down Expand Up @@ -2860,6 +2908,50 @@ public void DeleteDataOperation_required_args_composite()
});
}

[ConditionalFact]
public void DeleteDataOperation_args_with_linebreaks()
{
Test(
new DeleteDataOperation
{
Table = "TestLineBreaks",
KeyColumns = new[] { "Id", "Description" },
KeyValues = new object[,]
{
{ 0, "Contains\r\na Windows linebreak" },
{ 1, "Contains a\nLinux linebreak" },
{ 2, "Contains a single Backslash r,\rjust in case" },
}
},
"mb.DeleteData("
+ _eol
+ " table: \"TestLineBreaks\","
+ _eol
+ " keyColumns: new[] { \"Id\", \"Description\" },"
+ _eol
+ " keyValues: new object[,]"
+ _eol
+ " {"
+ _eol
+ " { 0, \"Contains\\r\\na Windows linebreak\" },"
+ _eol
+ " { 1, \"Contains a\\nLinux linebreak\" },"
+ _eol
+ " { 2, \"Contains a single Backslash r,\\rjust in case\" }"
+ _eol
+ " });",
operation =>
{
Assert.Equal("TestLineBreaks", operation.Table);
Assert.Equal(2, operation.KeyColumns.Length);
Assert.Equal(3, operation.KeyValues.GetLength(0));
Assert.Equal(2, operation.KeyValues.GetLength(1));
Assert.Equal("Contains\r\na Windows linebreak", operation.KeyValues[0, 1]);
Assert.Equal("Contains a\nLinux linebreak", operation.KeyValues[1, 1]);
Assert.Equal("Contains a single Backslash r,\rjust in case", operation.KeyValues[2, 1]);
});
}

[ConditionalFact]
public void UpdateDataOperation_all_args()
{
Expand Down Expand Up @@ -3265,6 +3357,73 @@ public void UpdateDataOperation_required_args_multi()
});
}

[ConditionalFact]
public void UpdateDataOperation_with_linebreaks()
{
Test(
new UpdateDataOperation
{
Schema = "dbo",
Table = "TestLineBreaks",
KeyColumns = new[] { "Id" },
KeyValues = new object[,] { { 0 }, { 1 }, { 2 }, },
Columns = new[] { "Description" },
Values = new object[,]
{
{ "Contains\r\na Windows linebreak" },
{ "Contains a\nLinux linebreak" },
{ "Contains a single Backslash r,\rjust in case" },
}
},
"mb.UpdateData("
+ _eol
+ " schema: \"dbo\","
+ _eol
+ " table: \"TestLineBreaks\","
+ _eol
+ " keyColumn: \"Id\","
+ _eol
+ " keyValues: new object[]"
+ _eol
+ " {"
+ _eol
+ " 0,"
+ _eol
+ " 1,"
+ _eol
+ " 2"
+ _eol
+ " },"
+ _eol
+ " column: \"Description\","
+ _eol
+ " values: new object[]"
+ _eol
+ " {"
+ _eol
+ " \"Contains\\r\\na Windows linebreak\","
+ _eol
+ " \"Contains a\\nLinux linebreak\","
+ _eol
+ " \"Contains a single Backslash r,\\rjust in case\""
+ _eol
+ " });",
operation =>
{
Assert.Equal("dbo", operation.Schema);
Assert.Equal("TestLineBreaks", operation.Table);
Assert.Single(operation.KeyColumns);
Assert.Equal(3, operation.KeyValues.GetLength(0));
Assert.Equal(1, operation.KeyValues.GetLength(1));
Assert.Single(operation.Columns);
Assert.Equal(3, operation.Values.GetLength(0));
Assert.Equal(1, operation.Values.GetLength(1));
Assert.Equal("Contains\r\na Windows linebreak", operation.Values[0, 0]);
Assert.Equal("Contains a\nLinux linebreak", operation.Values[1, 0]);
Assert.Equal("Contains a single Backslash r,\rjust in case", operation.Values[2, 0]);
});
}

private void Test<T>(T operation, string expectedCode, Action<T> assert)
where T : MigrationOperation
{
Expand Down