From 2f2b170aeb2940c251802b3bb81d8ca913194c4d Mon Sep 17 00:00:00 2001 From: lajones Date: Wed, 22 Apr 2020 14:30:18 -0700 Subject: [PATCH] Fix for 20662. Linebreaks in HasData() in migration. (#20699) --- .../Design/Internal/CSharpHelper.cs | 5 +- .../Design/Internal/CSharpHelperTest.cs | 4 +- .../CSharpMigrationOperationGeneratorTest.cs | 159 ++++++++++++++++++ 3 files changed, 163 insertions(+), 5 deletions(-) diff --git a/src/EFCore.Design/Design/Internal/CSharpHelper.cs b/src/EFCore.Design/Design/Internal/CSharpHelper.cs index a4f7b39bdcd..1efc271c93d 100644 --- a/src/EFCore.Design/Design/Internal/CSharpHelper.cs +++ b/src/EFCore.Design/Design/Internal/CSharpHelper.cs @@ -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. /// 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") + "\""; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to diff --git a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs index 6c49f0b90cc..de048064196 100644 --- a/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs +++ b/test/EFCore.Design.Tests/Design/Internal/CSharpHelperTest.cs @@ -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")] diff --git a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs index 01a10a1f8f6..4d62624e7e5 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/CSharpMigrationOperationGeneratorTest.cs @@ -2728,6 +2728,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() { @@ -2871,6 +2919,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() { @@ -3276,6 +3368,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 operation, string expectedCode, Action assert) where T : MigrationOperation {