From e7c8cb2117e5711eff86899a2d20a6dcffcba7c5 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Fri, 15 Nov 2024 20:16:31 +0300 Subject: [PATCH 1/5] Add Square Bracket when constructing collectionNode. Also add some tests --- .../UriParser/Binders/InBinder.cs | 32 +++++----- .../FilterAndOrderByBuilderTests.cs | 10 +++ .../UriBuilder/SelectExpandBuilderTests.cs | 12 ++-- .../FilterAndOrderByFunctionalTests.cs | 64 +++++++++++++++++++ 4 files changed, 99 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 2d4ee8d02d..7f8ee5838d 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -97,21 +97,23 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm LiteralToken literalToken = queryToken as LiteralToken; if (literalToken != null) { - string originalLiteralText = literalToken.OriginalText; - // Parentheses-based collections are not standard JSON but bracket-based ones are. // Temporarily switch our collection to bracket-based so that the JSON reader will // correctly parse the collection. Then pass the original literal text to the token. - string bracketLiteralText = originalLiteralText; - if (bracketLiteralText[0] == '(') + ReadOnlySpan bracketLiteralText = literalToken.OriginalText.AsSpan(); + + if (bracketLiteralText[0] == '(' || bracketLiteralText[0] == '[') { - Debug.Assert(bracketLiteralText[bracketLiteralText.Length - 1] == ')', - "Collection with opening '(' should have corresponding ')'"); + Debug.Assert((bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') || (bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'), + $"Collection with opening '{bracketLiteralText[0]}' should have corresponding '{bracketLiteralText[^1]}'"); - StringBuilder replacedText = new StringBuilder(bracketLiteralText); - replacedText[0] = '['; - replacedText[replacedText.Length - 1] = ']'; - bracketLiteralText = replacedText.ToString(); + if(bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') + { + Span replacedText = new Span(bracketLiteralText.ToArray()); + replacedText[0] = '['; + replacedText[replacedText.Length - 1] = ']'; + bracketLiteralText = replacedText; + } Debug.Assert(expectedType.IsCollection()); string expectedTypeFullName = expectedType.Definition.AsElementType().FullTypeName(); @@ -122,7 +124,7 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // and also, per ABNF, a single quote within a string literal is "encoded" as two consecutive single quotes in either // literal or percent - encoded representation. // Sample: ['a''bc','''def','xyz'''] ==> ["a'bc","'def","xyz'"], which is legitimate Json format. - bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText); + bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText.ToString()); } else if (expectedTypeFullName.Equals("Edm.Guid", StringComparison.Ordinal)) { @@ -130,7 +132,7 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // with the Json reader used for deserialization. // Sample: [D01663CF-EB21-4A0E-88E0-361C10ACE7FD, 492CF54A-84C9-490C-A7A4-B5010FAD8104] // ==> ['D01663CF-EB21-4A0E-88E0-361C10ACE7FD', '492CF54A-84C9-490C-A7A4-B5010FAD8104'] - bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText); + bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText.ToString()); } else if (expectedTypeFullName.Equals("Edm.DateTimeOffset", StringComparison.Ordinal) || expectedTypeFullName.Equals("Edm.Date", StringComparison.Ordinal) || @@ -141,12 +143,12 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // with the Json reader used for deserialization. // Sample: [1970-01-01T00:00:00Z, 1980-01-01T01:01:01+01:00] // ==> ['1970-01-01T00:00:00Z', '1980-01-01T01:01:01+01:00'] - bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText); + bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText.ToString()); } } - object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText, model, expectedType); - LiteralToken collectionLiteralToken = new LiteralToken(collection, originalLiteralText, expectedType); + object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText.ToString(), model, expectedType); + LiteralToken collectionLiteralToken = new LiteralToken(collection, literalToken.OriginalText, expectedType); operand = this.bindMethod(collectionLiteralToken) as CollectionConstantNode; } else diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/FilterAndOrderByBuilderTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/FilterAndOrderByBuilderTests.cs index f8c689e3a3..44ce91c42c 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/FilterAndOrderByBuilderTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/FilterAndOrderByBuilderTests.cs @@ -450,6 +450,16 @@ public void BuildFilterWithInOperatorUsingBracketedCollectionConstant() Assert.Equal(new Uri("http://gobbledygook/People?$filter=ID%20in%20[1%2C2%2C3]"), actualUri, new UriComparer()); } + [Theory] + [InlineData("People?$filter=Name in ('')", "http://gobbledygook/People?$filter=Name%20in%20(%27%27)")] + [InlineData("People?$filter=Name in ['']", "http://gobbledygook/People?$filter=Name%20in%20[%27%27]")] + public void BuildFilterWithInOperatorUsingCollectionWithEmptyString(string filterQuery, string expectedQuery) + { + Uri queryUri = new Uri(filterQuery, UriKind.Relative); + Uri actualUri = UriBuilder(queryUri, ODataUrlKeyDelimiter.Parentheses, settings); + Assert.Equal(new Uri(expectedQuery), actualUri, new UriComparer()); + } + [Fact] public void BuildFilterWithInOperatorUsingSingleConstant() { diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/SelectExpandBuilderTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/SelectExpandBuilderTests.cs index 566c227bd4..2aa62b6860 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/SelectExpandBuilderTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriBuilder/SelectExpandBuilderTests.cs @@ -668,12 +668,16 @@ public void ExpandWithDollarItInFilterComplexBinaryOperatorShouldWork() Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString("MyDog($filter=$it/MyAddress/City eq 'Seattle')"), actualUri.OriginalString); } - [Fact] - public void ExpandWithDollarItInFilterInOperatorShouldWork() + [Theory] + [InlineData("People?$expand=MyDog($filter=$it/ID in ['1', '2', '3'])", "MyDog($filter=$it/ID in ['1', '2', '3'])")] + [InlineData("People?$expand=MyDog($filter=$it/ID in ('1', '2', '3'))", "MyDog($filter=$it/ID in ('1', '2', '3'))")] + [InlineData("People?$expand=MyDog($filter=$it/Name in (''))", "MyDog($filter=$it/Name in (''))")] + [InlineData("People?$expand=MyDog($filter=$it/Name in [''])", "MyDog($filter=$it/Name in [''])")] + public void ExpandWithDollarItInFilterInOperatorShouldWork(string filterQuery, string expectedSubQuery) { - Uri queryUri = new Uri("People?$expand=MyDog($filter=$it/ID in ['1', '2', '3'])", UriKind.Relative); + Uri queryUri = new Uri(filterQuery, UriKind.Relative); Uri actualUri = UriBuilder(queryUri, ODataUrlKeyDelimiter.Parentheses, settings); - Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString("MyDog($filter=$it/ID in ['1', '2', '3'])"), actualUri.OriginalString); + Assert.Equal("http://gobbledygook/People?$expand=" + Uri.EscapeDataString(expectedSubQuery), actualUri.OriginalString); } [Fact] diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index b7b28ad93a..3fce887f69 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2717,6 +2717,24 @@ public void FilterWithInOperationWithEmptyString(string filterClause) Assert.Equal("\"\"", constantNode.LiteralText); } + [Theory] + [InlineData("SSN in ['']")] // Edm.String + [InlineData("SSN in [ '' ]")] // Edm.String + [InlineData("SSN in [\"\"]")] // Edm.String + [InlineData("SSN in [ \"\" ]")] // Edm.String + public void FilterWithInOperationWithEmptyStringInSquareBrackets(string filterClause) + { + FilterClause filter = ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + var inNode = Assert.IsType(filter.Expression); + + CollectionConstantNode collectionNode = Assert.IsType(inNode.Right); + Assert.Equal(1, collectionNode.Collection.Count); + + ConstantNode constantNode = collectionNode.Collection.First(); + Assert.Equal("\"\"", constantNode.LiteralText); + } + [Theory] [InlineData("SSN in ( ' ' )", " ")] // 1 space [InlineData("SSN in ( ' ' )", " ")] // 3 spaces @@ -2737,6 +2755,26 @@ public void FilterWithInOperationWithWhitespace(string filterClause, string expe Assert.Equal(expectedLiteralText, constantNode.LiteralText); } + [Theory] + [InlineData("SSN in [ ' ' ]", " ")] // 1 space + [InlineData("SSN in [ ' ' ]", " ")] // 3 spaces + [InlineData("SSN in [ \" \" ]", " ")] // 2 spaces + [InlineData("SSN in [ \" \" ]", " ")] // 4 spaces + public void FilterWithInOperationWithWhitespaceInSquareBrackets(string filterClause, string expectedLiteralText) + { + FilterClause filter = ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + var inNode = Assert.IsType(filter.Expression); + + CollectionConstantNode collectionNode = Assert.IsType(inNode.Right); + + // A single whitespace or multiple whitespaces are valid literals + Assert.Equal(1, collectionNode.Collection.Count); + + ConstantNode constantNode = collectionNode.Collection.First(); + Assert.Equal(expectedLiteralText, constantNode.LiteralText); + } + [Theory] [InlineData("SSN in ( '', ' ' )")] // Edm.String [InlineData("SSN in ( \"\", \" \" )")] // Edm.String @@ -2754,10 +2792,30 @@ public void FilterWithInOperationWithEmptyStringAndWhitespace(string filterClaus Assert.Equal(2, collectionNode.Collection.Count); } + [Theory] + [InlineData("SSN in [ '', ' ' ]")] // Edm.String + [InlineData("SSN in [ \"\", \" \" ]")] // Edm.String + [InlineData("SSN in [ '', \" \" ]")] // Edm.String + [InlineData("SSN in [ \"\", ' ' ]")] // Edm.String + public void FilterWithInOperationWithEmptyStringAndWhitespaceInSquareBrackets(string filterClause) + { + FilterClause filter = ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); + + var inNode = Assert.IsType(filter.Expression); + + CollectionConstantNode collectionNode = Assert.IsType(inNode.Right); + + // A single whitespace or multiple whitespaces are valid literals + Assert.Equal(2, collectionNode.Collection.Count); + } + [Theory] [InlineData("MyGuid in ( '' )", "")] // Edm.Guid [InlineData("MyGuid in ( ' ' )", " ")] // Edm.Guid [InlineData("MyGuid in ( \" \" )", " ")] // Edm.Guid + [InlineData("MyGuid in [ '' ]", "")] // Edm.Guid + [InlineData("MyGuid in [ ' ' ]", " ")] // Edm.Guid + [InlineData("MyGuid in [ \" \" ]", " ")] // Edm.Guid public void FilterWithInOperationGuidWithEmptyQuotesThrows(string filterClause, string quotedString) { Action parse = () => ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); @@ -2768,6 +2826,9 @@ public void FilterWithInOperationGuidWithEmptyQuotesThrows(string filterClause, [InlineData("Birthdate in ( '' )", "")] // Edm.DateTimeOffset [InlineData("Birthdate in ( \" \" )", " ")] // Edm.DateTimeOffset [InlineData("Birthdate in (' ')", " ")] // Edm.DateTimeOffset + [InlineData("Birthdate in [ '' ]", "")] // Edm.DateTimeOffset + [InlineData("Birthdate in [ \" \" ]", " ")] // Edm.DateTimeOffset + [InlineData("Birthdate in [' ']", " ")] // Edm.DateTimeOffset public void FilterWithInOperationDateTimeOffsetWithEmptyQuotesThrows(string filterClause, string quotedString) { Action parse = () => ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); @@ -2778,6 +2839,9 @@ public void FilterWithInOperationDateTimeOffsetWithEmptyQuotesThrows(string filt [InlineData("MyDate in ( '' )", "")] // Edm.Date [InlineData("MyDate in ( \" \" )", " ")] // Edm.Date [InlineData("MyDate in (' ')", " ")] // Edm.Date + [InlineData("MyDate in [ '' ]", "")] // Edm.Date + [InlineData("MyDate in [ \" \" ]", " ")] // Edm.Date + [InlineData("MyDate in [' ']", " ")] // Edm.Date public void FilterWithInOperationDateWithEmptyQuotesThrows(string filterClause, string quotedString) { Action parse = () => ParseFilter(filterClause, HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); From f4caa0c162f48201e99b3fe4e7bdd0aefb9d5ab6 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Fri, 15 Nov 2024 21:12:44 +0300 Subject: [PATCH 2/5] Added E2E tests --- .../ClientTests/Tests/ClientQueryTests.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs b/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs index b41e314a9d..3e4752386e 100644 --- a/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs +++ b/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs @@ -63,6 +63,37 @@ public async Task DollarFilter_UsingContains_ExecutesSuccessfully(string query, Assert.Equal(expectedCount, result.Length); } + [Theory] + [InlineData("People?$filter=Name in ('')", 0)] + [InlineData("People?$filter=Name in ['']", 0)] + [InlineData("People?$filter=Name in ( '' )", 0)] + [InlineData("People?$filter=Name in [ '' ]", 0)] + [InlineData("People?$filter=Name in (\"\")", 0)] + [InlineData("People?$filter=Name in [\"\"]", 0)] + [InlineData("People?$filter=Name in ( \"\" )", 0)] + [InlineData("People?$filter=Name in [ \"\" ]", 0)] + [InlineData("People?$filter=Name in ( ' ' )", 0)] + [InlineData("People?$filter=Name in [ ' ' ]", 0)] + [InlineData("People?$filter=Name in ( \" \" )", 0)] + [InlineData("People?$filter=Name in [ \" \"]", 0)] + [InlineData("People?$filter=Name in ( '', ' ' )", 0)] + [InlineData("People?$filter=Name in [ '', ' ' ]", 0)] + [InlineData("People?$filter=Name in ( \"\", \" \" )", 0)] + [InlineData("People?$filter=Name in [ \"\", \" \" ]", 0)] + [InlineData("People?$filter=Name in ( '', \" \" )", 0)] + [InlineData("People?$filter=Name in [ '', \" \" ]", 0)] + [InlineData("People?$filter=Name in ( \"\", ' ' )", 0)] + [InlineData("People?$filter=Name in [ \"\", ' ' ]", 0)] + [InlineData("People?$filter=Name in [ 'null', 'null' ]", 0)] + public async Task DollarFilter_WithCollectionWithEmptyString_ExecutesSuccessfully(string query, int expectedCount) + { + // Act + var response = await _context.ExecuteAsync(new Uri(_baseUri.OriginalString + query)); + + // Assert + Assert.Equal(expectedCount, response.ToArray().Length); + } + [Fact] public async Task Using_LinqContains_ExecutesSuccessfully() { From b75161fb5f723eda689b6573ab89736ac8292ce7 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Mon, 18 Nov 2024 19:24:06 +0300 Subject: [PATCH 3/5] Add more test cases and refactor Assert --- .../ClientTests/Tests/ClientQueryTests.cs | 46 +++++++++---------- .../FilterAndOrderByFunctionalTests.cs | 45 +++++++++++++++++- 2 files changed, 66 insertions(+), 25 deletions(-) diff --git a/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs b/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs index 3e4752386e..33baa9652c 100644 --- a/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs +++ b/test/EndToEndTests/Tests/Client/Microsoft.OData.Client.E2E.Tests/ClientTests/Tests/ClientQueryTests.cs @@ -64,34 +64,34 @@ public async Task DollarFilter_UsingContains_ExecutesSuccessfully(string query, } [Theory] - [InlineData("People?$filter=Name in ('')", 0)] - [InlineData("People?$filter=Name in ['']", 0)] - [InlineData("People?$filter=Name in ( '' )", 0)] - [InlineData("People?$filter=Name in [ '' ]", 0)] - [InlineData("People?$filter=Name in (\"\")", 0)] - [InlineData("People?$filter=Name in [\"\"]", 0)] - [InlineData("People?$filter=Name in ( \"\" )", 0)] - [InlineData("People?$filter=Name in [ \"\" ]", 0)] - [InlineData("People?$filter=Name in ( ' ' )", 0)] - [InlineData("People?$filter=Name in [ ' ' ]", 0)] - [InlineData("People?$filter=Name in ( \" \" )", 0)] - [InlineData("People?$filter=Name in [ \" \"]", 0)] - [InlineData("People?$filter=Name in ( '', ' ' )", 0)] - [InlineData("People?$filter=Name in [ '', ' ' ]", 0)] - [InlineData("People?$filter=Name in ( \"\", \" \" )", 0)] - [InlineData("People?$filter=Name in [ \"\", \" \" ]", 0)] - [InlineData("People?$filter=Name in ( '', \" \" )", 0)] - [InlineData("People?$filter=Name in [ '', \" \" ]", 0)] - [InlineData("People?$filter=Name in ( \"\", ' ' )", 0)] - [InlineData("People?$filter=Name in [ \"\", ' ' ]", 0)] - [InlineData("People?$filter=Name in [ 'null', 'null' ]", 0)] - public async Task DollarFilter_WithCollectionWithEmptyString_ExecutesSuccessfully(string query, int expectedCount) + [InlineData("People?$filter=Name in ('')")] + [InlineData("People?$filter=Name in ['']")] + [InlineData("People?$filter=Name in ( '' )")] + [InlineData("People?$filter=Name in [ '' ]")] + [InlineData("People?$filter=Name in (\"\")")] + [InlineData("People?$filter=Name in [\"\"]")] + [InlineData("People?$filter=Name in ( \"\" )")] + [InlineData("People?$filter=Name in [ \"\" ]")] + [InlineData("People?$filter=Name in ( ' ' )")] + [InlineData("People?$filter=Name in [ ' ' ]")] + [InlineData("People?$filter=Name in ( \" \" )")] + [InlineData("People?$filter=Name in [ \" \"]")] + [InlineData("People?$filter=Name in ( '', ' ' )")] + [InlineData("People?$filter=Name in [ '', ' ' ]")] + [InlineData("People?$filter=Name in ( \"\", \" \" )")] + [InlineData("People?$filter=Name in [ \"\", \" \" ]")] + [InlineData("People?$filter=Name in ( '', \" \" )")] + [InlineData("People?$filter=Name in [ '', \" \" ]")] + [InlineData("People?$filter=Name in ( \"\", ' ' )")] + [InlineData("People?$filter=Name in [ \"\", ' ' ]")] + [InlineData("People?$filter=Name in [ 'null', 'null' ]")] + public async Task DollarFilter_WithCollectionWithEmptyString_ExecutesSuccessfully(string query) { // Act var response = await _context.ExecuteAsync(new Uri(_baseUri.OriginalString + query)); // Assert - Assert.Equal(expectedCount, response.ToArray().Length); + Assert.Empty(response.ToArray()); } [Fact] diff --git a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs index 3fce887f69..d271ba866b 100644 --- a/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs +++ b/test/FunctionalTests/Microsoft.OData.Core.Tests/ScenarioTests/UriParser/FilterAndOrderByFunctionalTests.cs @@ -2413,45 +2413,86 @@ public void FilterWithInOperationWithParensStringCollection_EscapedSingleQuote() [Theory] [InlineData("('a''bc')", "('a''bc')", 1)] + [InlineData("['a''bc']", "['a''bc']", 1)] [InlineData("('''def')", "('''def')", 1)] + [InlineData("['''def']", "['''def']", 1)] [InlineData("('xyz''')", "('xyz''')", 1)] + [InlineData("['xyz''']", "['xyz''']", 1)] [InlineData("('''pqr''')", "('''pqr''')", 1)] + [InlineData("['''pqr''']", "['''pqr''']", 1)] [InlineData("('a''bc','''def')", "('a''bc','''def')", 2)] + [InlineData("['a''bc','''def']", "['a''bc','''def']", 2)] [InlineData("('a''bc','xyz''')", "('a''bc','xyz''')", 2)] + [InlineData("['a''bc','xyz''']", "['a''bc','xyz''']", 2)] [InlineData("('a''bc','''pqr''')", "('a''bc','''pqr''')", 2)] + [InlineData("['a''bc','''pqr''']", "['a''bc','''pqr''']", 2)] [InlineData("('''def','a''bc')", "('''def','a''bc')", 2)] + [InlineData("['''def','a''bc']", "['''def','a''bc']", 2)] [InlineData("('''def','xyz''')", "('''def','xyz''')", 2)] + [InlineData("['''def','xyz''']", "['''def','xyz''']", 2)] [InlineData("('''def','''pqr''')", "('''def','''pqr''')", 2)] + [InlineData("['''def','''pqr''']", "['''def','''pqr''']", 2)] [InlineData("('xyz''','a''bc')", "('xyz''','a''bc')", 2)] + [InlineData("['xyz''','a''bc']", "['xyz''','a''bc']", 2)] [InlineData("('xyz''','''def')", "('xyz''','''def')", 2)] + [InlineData("['xyz''','''def']", "['xyz''','''def']", 2)] [InlineData("('xyz''','''pqr''')", "('xyz''','''pqr''')", 2)] + [InlineData("['xyz''','''pqr''']", "['xyz''','''pqr''']", 2)] [InlineData("('''pqr''','a''bc')", "('''pqr''','a''bc')", 2)] + [InlineData("['''pqr''','a''bc']", "['''pqr''','a''bc']", 2)] [InlineData("('''pqr''','''def')", "('''pqr''','''def')", 2)] + [InlineData("['''pqr''','''def']", "['''pqr''','''def']", 2)] [InlineData("('''pqr''','xyz''')", "('''pqr''','xyz''')", 2)] + [InlineData("['''pqr''','xyz''']", "['''pqr''','xyz''']", 2)] [InlineData("('a''bc','''def','xyz''')", "('a''bc','''def','xyz''')", 3)] + [InlineData("['a''bc','''def','xyz''']", "['a''bc','''def','xyz''']", 3)] [InlineData("('a''bc','''def','''pqr''')", "('a''bc','''def','''pqr''')", 3)] + [InlineData("['a''bc','''def','''pqr''']", "['a''bc','''def','''pqr''']", 3)] [InlineData("('a''bc','xyz''','''def')", "('a''bc','xyz''','''def')", 3)] + [InlineData("['a''bc','xyz''','''def']", "['a''bc','xyz''','''def']", 3)] [InlineData("('a''bc','xyz''','''pqr''')", "('a''bc','xyz''','''pqr''')", 3)] + [InlineData("['a''bc','xyz''','''pqr''']", "['a''bc','xyz''','''pqr''']", 3)] [InlineData("('a''bc','''pqr''','''def')", "('a''bc','''pqr''','''def')", 3)] + [InlineData("['a''bc','''pqr''','''def']", "['a''bc','''pqr''','''def']", 3)] [InlineData("('a''bc','''pqr''','xyz''')", "('a''bc','''pqr''','xyz''')", 3)] + [InlineData("['a''bc','''pqr''','xyz''']", "['a''bc','''pqr''','xyz''']", 3)] [InlineData("('''def','a''bc','xyz''')", "('''def','a''bc','xyz''')", 3)] + [InlineData("['''def','a''bc','xyz''']", "['''def','a''bc','xyz''']", 3)] [InlineData("('''def','a''bc','''pqr''')", "('''def','a''bc','''pqr''')", 3)] + [InlineData("['''def','a''bc','''pqr''']", "['''def','a''bc','''pqr''']", 3)] [InlineData("('''def','xyz''','a''bc')", "('''def','xyz''','a''bc')", 3)] + [InlineData("['''def','xyz''','a''bc']", "['''def','xyz''','a''bc']", 3)] [InlineData("('''def','xyz''','''pqr''')", "('''def','xyz''','''pqr''')", 3)] + [InlineData("['''def','xyz''','''pqr''']", "['''def','xyz''','''pqr''']", 3)] [InlineData("('''def','''pqr''','a''bc')", "('''def','''pqr''','a''bc')", 3)] + [InlineData("['''def','''pqr''','a''bc']", "['''def','''pqr''','a''bc']", 3)] [InlineData("('''def','''pqr''','xyz''')", "('''def','''pqr''','xyz''')", 3)] + [InlineData("['''def','''pqr''','xyz''']", "['''def','''pqr''','xyz''']", 3)] [InlineData("('xyz''','a''bc','''def')", "('xyz''','a''bc','''def')", 3)] + [InlineData("['xyz''','a''bc','''def']", "['xyz''','a''bc','''def']", 3)] [InlineData("('xyz''','a''bc','''pqr''')", "('xyz''','a''bc','''pqr''')", 3)] + [InlineData("['xyz''','a''bc','''pqr''']", "['xyz''','a''bc','''pqr''']", 3)] [InlineData("('xyz''','''def','''pqr''')", "('xyz''','''def','''pqr''')", 3)] + [InlineData("['xyz''','''def','''pqr''']", "['xyz''','''def','''pqr''']", 3)] [InlineData("('xyz''','''def','a''bc')", "('xyz''','''def','a''bc')", 3)] + [InlineData("['xyz''','''def','a''bc']", "['xyz''','''def','a''bc']", 3)] [InlineData("('xyz''','''pqr''','a''bc')", "('xyz''','''pqr''','a''bc')", 3)] + [InlineData("['xyz''','''pqr''','a''bc']", "['xyz''','''pqr''','a''bc']", 3)] [InlineData("('xyz''','''pqr''','''def')", "('xyz''','''pqr''','''def')", 3)] + [InlineData("['xyz''','''pqr''','''def']", "['xyz''','''pqr''','''def']", 3)] [InlineData("('''pqr''','a''bc','''def')", "('''pqr''','a''bc','''def')", 3)] + [InlineData("['''pqr''','a''bc','''def']", "['''pqr''','a''bc','''def']", 3)] [InlineData("('''pqr''','a''bc','xyz''')", "('''pqr''','a''bc','xyz''')", 3)] + [InlineData("['''pqr''','a''bc','xyz''']", "['''pqr''','a''bc','xyz''']", 3)] [InlineData("('''pqr''','''def','a''bc')", "('''pqr''','''def','a''bc')", 3)] + [InlineData("['''pqr''','''def','a''bc']", "['''pqr''','''def','a''bc']", 3)] [InlineData("('''pqr''','''def','xyz''')", "('''pqr''','''def','xyz''')", 3)] + [InlineData("['''pqr''','''def','xyz''']", "['''pqr''','''def','xyz''']", 3)] [InlineData("('''pqr''','xyz''','a''bc')", "('''pqr''','xyz''','a''bc')", 3)] + [InlineData("['''pqr''','xyz''','a''bc']", "['''pqr''','xyz''','a''bc']", 3)] [InlineData("('''pqr''','xyz''','''def')", "('''pqr''','xyz''','''def')", 3)] + [InlineData("['''pqr''','xyz''','''def']", "['''pqr''','xyz''','''def']", 3)] + public void FilterWithInExpressionContainingEscapedSingleQuotes(string inExpr, string parsedExpr, int count) { FilterClause filter = ParseFilter($"SSN in {inExpr}", HardCodedTestModel.TestModel, HardCodedTestModel.GetPersonType()); @@ -2749,7 +2790,7 @@ public void FilterWithInOperationWithWhitespace(string filterClause, string expe CollectionConstantNode collectionNode = Assert.IsType(inNode.Right); // A single whitespace or multiple whitespaces are valid literals - Assert.Equal(1, collectionNode.Collection.Count); + Assert.Single(collectionNode.Collection); ConstantNode constantNode = collectionNode.Collection.First(); Assert.Equal(expectedLiteralText, constantNode.LiteralText); @@ -2769,7 +2810,7 @@ public void FilterWithInOperationWithWhitespaceInSquareBrackets(string filterCla CollectionConstantNode collectionNode = Assert.IsType(inNode.Right); // A single whitespace or multiple whitespaces are valid literals - Assert.Equal(1, collectionNode.Collection.Count); + Assert.Single(collectionNode.Collection); ConstantNode constantNode = collectionNode.Collection.First(); Assert.Equal(expectedLiteralText, constantNode.LiteralText); From cae19e1e45503a6ec52c4d06fbdbc291ebc4feec Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Mon, 25 Nov 2024 15:02:59 +0300 Subject: [PATCH 4/5] Use string create instead of allocate extra memory --- .../UriParser/Binders/InBinder.cs | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 7f8ee5838d..9cb29458a3 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -100,19 +100,24 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // Parentheses-based collections are not standard JSON but bracket-based ones are. // Temporarily switch our collection to bracket-based so that the JSON reader will // correctly parse the collection. Then pass the original literal text to the token. - ReadOnlySpan bracketLiteralText = literalToken.OriginalText.AsSpan(); + string bracketLiteralText = literalToken.OriginalText; if (bracketLiteralText[0] == '(' || bracketLiteralText[0] == '[') { - Debug.Assert((bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') || (bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'), + bool isParenthesesBased = bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')'; + bool isBracketBased = bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'; + + Debug.Assert(isParenthesesBased || isBracketBased, $"Collection with opening '{bracketLiteralText[0]}' should have corresponding '{bracketLiteralText[^1]}'"); - if(bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') + if(isParenthesesBased) { - Span replacedText = new Span(bracketLiteralText.ToArray()); - replacedText[0] = '['; - replacedText[replacedText.Length - 1] = ']'; - bracketLiteralText = replacedText; + bracketLiteralText = string.Create(bracketLiteralText.Length, bracketLiteralText, (span, state) => + { + state.AsSpan().CopyTo(span); + span[0] = '['; + span[^1] = ']'; + }); } Debug.Assert(expectedType.IsCollection()); @@ -124,7 +129,7 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // and also, per ABNF, a single quote within a string literal is "encoded" as two consecutive single quotes in either // literal or percent - encoded representation. // Sample: ['a''bc','''def','xyz'''] ==> ["a'bc","'def","xyz'"], which is legitimate Json format. - bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText.ToString()); + bracketLiteralText = NormalizeStringCollectionItems(bracketLiteralText); } else if (expectedTypeFullName.Equals("Edm.Guid", StringComparison.Ordinal)) { @@ -132,7 +137,7 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // with the Json reader used for deserialization. // Sample: [D01663CF-EB21-4A0E-88E0-361C10ACE7FD, 492CF54A-84C9-490C-A7A4-B5010FAD8104] // ==> ['D01663CF-EB21-4A0E-88E0-361C10ACE7FD', '492CF54A-84C9-490C-A7A4-B5010FAD8104'] - bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText.ToString()); + bracketLiteralText = NormalizeGuidCollectionItems(bracketLiteralText); } else if (expectedTypeFullName.Equals("Edm.DateTimeOffset", StringComparison.Ordinal) || expectedTypeFullName.Equals("Edm.Date", StringComparison.Ordinal) || @@ -143,11 +148,11 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm // with the Json reader used for deserialization. // Sample: [1970-01-01T00:00:00Z, 1980-01-01T01:01:01+01:00] // ==> ['1970-01-01T00:00:00Z', '1980-01-01T01:01:01+01:00'] - bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText.ToString()); + bracketLiteralText = NormalizeDateTimeCollectionItems(bracketLiteralText); } } - object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText.ToString(), model, expectedType); + object collection = ODataUriConversionUtils.ConvertFromCollectionValue(bracketLiteralText, model, expectedType); LiteralToken collectionLiteralToken = new LiteralToken(collection, literalToken.OriginalText, expectedType); operand = this.bindMethod(collectionLiteralToken) as CollectionConstantNode; } From e140298361bd3a91c0e7c2c25887c1215a34d2d6 Mon Sep 17 00:00:00 2001 From: Samuel Wanjohi Date: Wed, 27 Nov 2024 13:40:58 +0300 Subject: [PATCH 5/5] Remove unnecessary variables --- src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs index 9cb29458a3..44672307c5 100644 --- a/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs +++ b/src/Microsoft.OData.Core/UriParser/Binders/InBinder.cs @@ -104,13 +104,10 @@ private CollectionNode GetCollectionOperandFromToken(QueryToken queryToken, IEdm if (bracketLiteralText[0] == '(' || bracketLiteralText[0] == '[') { - bool isParenthesesBased = bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')'; - bool isBracketBased = bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'; + Debug.Assert((bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') || (bracketLiteralText[0] == '[' && bracketLiteralText[^1] == ']'), + $"Collection with opening '{bracketLiteralText[0]}' should have corresponding '{(bracketLiteralText[0] == '(' ? ')' : ']')}'"); - Debug.Assert(isParenthesesBased || isBracketBased, - $"Collection with opening '{bracketLiteralText[0]}' should have corresponding '{bracketLiteralText[^1]}'"); - - if(isParenthesesBased) + if (bracketLiteralText[0] == '(' && bracketLiteralText[^1] == ')') { bracketLiteralText = string.Create(bracketLiteralText.Length, bracketLiteralText, (span, state) => {