diff --git a/.run/Aviationexam.MoneyErp.Tests.run.xml b/.run/Aviationexam.MoneyErp.Tests.run.xml deleted file mode 100644 index 0b82fd4..0000000 --- a/.run/Aviationexam.MoneyErp.Tests.run.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - \ No newline at end of file diff --git a/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.ValueContainingOperatorCharacters.cs b/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.ValueContainingOperatorCharacters.cs new file mode 100644 index 0000000..8dda490 --- /dev/null +++ b/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.ValueContainingOperatorCharacters.cs @@ -0,0 +1,101 @@ +using Aviationexam.MoneyErp.Common.Filters; +using System; +using Xunit; + +namespace Aviationexam.MoneyErp.Common.Tests; + +public partial class FilterForTests +{ + /// + /// Reproduces a real-world failure where a street address value contains '#', + /// which is also the AND operator in the filter DSL. + /// + /// Original (anonymized) request had "Street 80A # 17-85" as FaktUlice value. + /// Without escaping, the '#' was misinterpreted as an AND operator. + /// + [Fact] + public void AndWithValueContainingHashIsEscaped() + { + var filter = FilterFor.And( + x => x.StartWith(m => m.AProperty, "ABC123"), + x => x.Equal(m => m.BProperty, "John Doe"), + x => x.Equal(m => m.CProperty, "Street 80A # 17-85"), + x => x.Equal(m => m.DProperty, "Prague"), + x => x.Equal(m => m.EProperty, "11000"), + x => x.Equal(m => m.BoolProperty, false), + x => x.Equal(m => m.AProperty, "b679009e-4d3c-40a0-9031-bc00b24ec9d6"), + x => x.Empty(m => m.AProperty) + ); + + Assert.Equal( + @"AProperty~sw~ABC123#BProperty~eq~John Doe#CProperty~eq~Street 80A \# 17-85#DProperty~eq~Prague#EProperty~eq~11000#BoolProperty~eq~false#AProperty~eq~b679009e-4d3c-40a0-9031-bc00b24ec9d6#AProperty~eq~", + filter + ); + } + + [Fact] + public void EqualWithValueContainingHash() + { + var filter = FilterFor.Equal(x => x.AProperty, "Street 80A # 17-85"); + + Assert.Equal(@"AProperty~eq~Street 80A \# 17-85", filter); + } + + [Fact] + public void EqualWithValueContainingTilde() + { + var filter = FilterFor.Equal(x => x.AProperty, "value~with~tildes"); + + Assert.Equal(@"AProperty~eq~value\~with\~tildes", filter); + } + + [Fact] + public void EqualWithValueContainingPipe() + { + var filter = FilterFor.Equal(x => x.AProperty, "option A|option B"); + + Assert.Equal(@"AProperty~eq~option A\|option B", filter); + } + + [Fact] + public void EqualWithValueContainingBackslash() + { + var filter = FilterFor.Equal(x => x.AProperty, @"path\to\file"); + + Assert.Equal(@"AProperty~eq~path\\to\\file", filter); + } + + [Fact] + public void EqualWithValueContainingMultipleSpecialCharacters() + { + var filter = FilterFor.Equal(x => x.AProperty, "a#b|c~d"); + + Assert.Equal(@"AProperty~eq~a\#b\|c\~d", filter); + } + + [Fact] + public void EqualWithValueWithoutSpecialCharactersIsUnchanged() + { + var filter = FilterFor.Equal(x => x.AProperty, "normal value 123"); + + Assert.Equal("AProperty~eq~normal value 123", filter); + } + + [Fact] + public void EscapeFilterValueReturnsInputWhenNoSpecialChars() + { + ReadOnlySpan input = "hello world"; + var result = FilterFor.EscapeFilterValue(input); + + Assert.Equal("hello world", result); + } + + [Fact] + public void EscapeFilterValueEscapesAllOperatorCharacters() + { + ReadOnlySpan input = @"a#b|c~d\e"; + var result = FilterFor.EscapeFilterValue(input); + + Assert.Equal(@"a\#b\|c\~d\\e", result); + } +} diff --git a/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.cs b/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.cs index d037b24..c44b722 100644 --- a/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.cs +++ b/src/Aviationexam.MoneyErp.Common.Tests/FilterForTests.cs @@ -20,6 +20,14 @@ private sealed class ApiModel { public required string AProperty { get; set; } + public string? BProperty { get; set; } + + public string? CProperty { get; set; } + + public string? DProperty { get; set; } + + public string? EProperty { get; set; } + public int IntProperty { get; set; } public int? NullableIntProperty { get; set; } @@ -43,5 +51,9 @@ private sealed class ApiModel public DateOnly DateOnlyProperty { get; set; } public DateOnly? NullableDateOnlyProperty { get; set; } + + public Guid GuidProperty { get; set; } + + public Guid? NullableGuidProperty { get; set; } } } diff --git a/src/Aviationexam.MoneyErp.Common/Filters/FilterFor.cs b/src/Aviationexam.MoneyErp.Common/Filters/FilterFor.cs index 578ec4b..d563d71 100644 --- a/src/Aviationexam.MoneyErp.Common/Filters/FilterFor.cs +++ b/src/Aviationexam.MoneyErp.Common/Filters/FilterFor.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq.Expressions; using System.Numerics; using System.Reflection; @@ -11,6 +12,16 @@ public partial class FilterFor where T : class { public const char AndOperator = '#'; public const char OrOperator = '|'; + private const char OperatorDelimiter = '~'; + + [SuppressMessage("ReSharper", "StaticMemberInGenericType")] + private static readonly IReadOnlyCollection<(char Character, string Replacement)> EscapeRules = + [ + ('\\', "\\\\"), + (AndOperator, $"\\{AndOperator}"), + (OrOperator, $"\\{OrOperator}"), + (OperatorDelimiter, $"\\{OperatorDelimiter}"), + ]; private static ReadOnlySpan CombineExpressions( char joiningCharacter, @@ -94,19 +105,68 @@ public static ReadOnlySpan GetFilterClause( EFilterOperator filterOperator, ReadOnlySpan property, ReadOnlySpan value - ) => filterOperator switch + ) { - EFilterOperator.Equal => $"{property}~eq~{value}", - EFilterOperator.NotEqual => $"{property}~ne~{value}", - EFilterOperator.LessThan => $"{property}~lt~{value}", - EFilterOperator.LessThanOrEqual => $"{property}~lte~{value}", - EFilterOperator.GreaterThan => $"{property}~gt~{value}", - EFilterOperator.GreaterThanOrEqual => $"{property}~gte~{value}", - EFilterOperator.StartWith => $"{property}~sw~{value}", - EFilterOperator.Contains => $"{property}~ct~{value}", - EFilterOperator.EndWith => $"{property}~ew~{value}", - _ => throw new ArgumentOutOfRangeException(nameof(filterOperator), filterOperator, null), - }; + var escapedValue = EscapeFilterValue(value); + + return filterOperator switch + { + EFilterOperator.Equal => $"{property}~eq~{escapedValue}", + EFilterOperator.NotEqual => $"{property}~ne~{escapedValue}", + EFilterOperator.LessThan => $"{property}~lt~{escapedValue}", + EFilterOperator.LessThanOrEqual => $"{property}~lte~{escapedValue}", + EFilterOperator.GreaterThan => $"{property}~gt~{escapedValue}", + EFilterOperator.GreaterThanOrEqual => $"{property}~gte~{escapedValue}", + EFilterOperator.StartWith => $"{property}~sw~{escapedValue}", + EFilterOperator.Contains => $"{property}~ct~{escapedValue}", + EFilterOperator.EndWith => $"{property}~ew~{escapedValue}", + _ => throw new ArgumentOutOfRangeException(nameof(filterOperator), filterOperator, null), + }; + } + + internal static ReadOnlySpan EscapeFilterValue(ReadOnlySpan value) + { + if (!ContainsAnyEscapableCharacter(value)) + { + return value; + } + + var sb = new StringBuilder(value.Length + 4); + + foreach (var ch in value) + { + var replaced = false; + foreach (var (character, replacement) in EscapeRules) + { + if (ch == character) + { + sb.Append(replacement); + replaced = true; + break; + } + } + + if (!replaced) + { + sb.Append(ch); + } + } + + return sb.ToString(); + } + + private static bool ContainsAnyEscapableCharacter(ReadOnlySpan value) + { + foreach (var (character, _) in EscapeRules) + { + if (value.Contains(character)) + { + return true; + } + } + + return false; + } public static ReadOnlySpan GetFilterClause( EFilterOperator filterOperator, diff --git a/src/Aviationexam.MoneyErp.Graphql.Tests/MoneyErpCompanyFilterTests.cs b/src/Aviationexam.MoneyErp.Graphql.Tests/MoneyErpCompanyFilterTests.cs new file mode 100644 index 0000000..35a0fa9 --- /dev/null +++ b/src/Aviationexam.MoneyErp.Graphql.Tests/MoneyErpCompanyFilterTests.cs @@ -0,0 +1,221 @@ +using Aviationexam.MoneyErp.Common.Filters; +using Aviationexam.MoneyErp.Graphql.Client; +using Aviationexam.MoneyErp.Graphql.Extensions; +using Aviationexam.MoneyErp.Graphql.Tests.Infrastructure; +using Microsoft.Extensions.DependencyInjection; +using System; +using System.Threading.Tasks; +using Xunit; +using ZLinq; + +namespace Aviationexam.MoneyErp.Graphql.Tests; + +/// +/// Integration test verifying that company filter queries work correctly +/// when filter values contain special characters (#, ~, |) that are also +/// used as operators in the MoneyERP filter DSL. +/// +public class MoneyErpCompanyFilterTests( + ITestOutputHelper testOutputHelper +) +{ + [Theory] + [ClassData(typeof(MoneyErpAuthenticationsClassData), Explicit = false)] + public Task QueryCompanyWithHashInFilterValueWorks( + MoneyErpAuthenticationsClassData.AuthenticationData? authenticationData + ) => QueryCompanyWithSpecialCharInFilterValue(authenticationData, "TEST_HASH", "Street 80A # 17-85", '#'); + + [Theory] + [ClassData(typeof(MoneyErpAuthenticationsClassData), Explicit = false)] + public Task QueryCompanyWithTildeInFilterValueWorks( + MoneyErpAuthenticationsClassData.AuthenticationData? authenticationData + ) => QueryCompanyWithSpecialCharInFilterValue(authenticationData, "TEST_TILDE", "Street~80A~17", '~'); + + [Theory] + [ClassData(typeof(MoneyErpAuthenticationsClassData), Explicit = false)] + public Task QueryCompanyWithPipeInFilterValueWorks( + MoneyErpAuthenticationsClassData.AuthenticationData? authenticationData + ) => QueryCompanyWithSpecialCharInFilterValue(authenticationData, "TEST_PIPE", "Street 80A|17-85", '|'); + + private async Task QueryCompanyWithSpecialCharInFilterValue( + MoneyErpAuthenticationsClassData.AuthenticationData? authenticationData, + string testCompanyKod, + string testCompanyStreet, + char specialChar + ) + { + await using var serviceProvider = ServiceProviderFactory.Create( + authenticationData!, testOutputHelper, shouldRedactHeaderValue: true + ); + + var graphqlClient = serviceProvider.GetRequiredService(); + + // Step 1: Verify connectivity + var version = await graphqlClient.Query( + x => x.Version, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.NotNull(version.Data); + Assert.NotEmpty(version.Data); + + // Step 2: Resolve country ID for CZ and numerical series + var metadataFilter = new + { + countryFilter = FilterFor.Equal(m => m.Kod, "CZ").ToString(), + numericalSeriesFilter = FilterFor.Equal(m => m.Kod, "USER_ID").ToString(), + }; + var metadataResponse = await graphqlClient.Query( + metadataFilter, + static (f, x) => new + { + Countries = x.Countries( + Filter: f.countryFilter, + selector: c => new { c.ID, c.Deleted, c.Kod } + ), + NumericalSeries = x.NumericalSeries( + Filter: f.numericalSeriesFilter, + selector: c => new { c.ID, c.Deleted, c.Kod } + ), + }, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.Empty(metadataResponse.Errors ?? []); + + var country = Assert.Single(metadataResponse.Data!.Countries! + .AsValueEnumerable() + .Where(c => c!.Deleted is false) + .ToArray()); + var countryId = country!.ID.AsGuid()!.Value; + + var numericalSerie = Assert.Single(metadataResponse.Data!.NumericalSeries! + .AsValueEnumerable() + .Where(c => c!.Deleted is false) + .ToArray()); + var numericalSerieId = numericalSerie!.ID.AsGuid()!.Value; + + // Step 3: Query company with special char in FaktUlice value + var companyWithSpecialCharFilter = new + { + companyFilter = FilterFor.And( + x => x.StartWith(m => m.Kod, testCompanyKod), + x => x.Equal(m => m.FaktUlice, testCompanyStreet) + ) + .ToString(), + }; + var companyResponse = await graphqlClient.Query( + companyWithSpecialCharFilter, + static (f, x) => new + { + Companies = x.Companies( + Filter: f.companyFilter, + selector: c => new { c.ID, c.Deleted, c.Kod, c.FaktUlice } + ), + }, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.Empty(companyResponse.Errors ?? []); + + var existingCompany = companyResponse.Data?.Companies? + .AsValueEnumerable() + .Where(c => c!.Deleted is false) + .FirstOrDefault(); + + if (existingCompany is not null) + { + Assert.Equal(testCompanyStreet, existingCompany.FaktUlice); + return; + } + + // Step 4: Query without the special char field to distinguish + // "filter broken by special char" vs "company doesn't exist" + var fallbackFilter = new + { + companyFilter = FilterFor.StartWith(m => m.Kod, testCompanyKod).ToString(), + }; + var fallbackResponse = await graphqlClient.Query( + fallbackFilter, + static (f, x) => new + { + Companies = x.Companies( + Filter: f.companyFilter, + selector: c => new { c.ID, c.Deleted, c.Kod, c.FaktUlice } + ), + }, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.Empty(fallbackResponse.Errors ?? []); + + var fallbackCompany = fallbackResponse.Data?.Companies? + .AsValueEnumerable() + .Where(c => c!.Deleted is false) + .FirstOrDefault(); + + if (fallbackCompany is not null) + { + Assert.Fail( + $"Company '{fallbackCompany.Kod}' with FaktUlice='{fallbackCompany.FaktUlice}' exists " + + $"but was NOT found when '{specialChar}' was included in the filter value. " + + $"This confirms the '{specialChar}' character in filter values is misinterpreted as an operator." + ); + } + + // Step 5: Company doesn't exist — create it + var testCompanyName = $"Test Company {specialChar}"; + var companyInput = new + { + companyInput = new CompanyInput + { + CiselnaRada_ID = numericalSerieId, + Kod = testCompanyKod, + Nazev = testCompanyName, + PlatceDPH = false, + FaktNazev = testCompanyName, + FaktUlice = testCompanyStreet, + FaktMisto = "Prague", + FaktPsc = "11000", + FaktStat_ID = countryId, + ObchNazev = testCompanyName, + ObchUlice = testCompanyStreet, + ObchMisto = "Prague", + ObchPsc = "11000", + ObchStat_ID = countryId, + }, + }; + var createResponse = await graphqlClient.Mutation( + companyInput, + static (f, x) => new + { + Company = x.CreateCompany( + f.companyInput, + selector: c => new { c.ID, c.Deleted, c.Kod, c.FaktUlice } + ), + }, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.Empty(createResponse.Errors ?? []); + Assert.NotNull(createResponse.Data?.Company); + Assert.Equal(testCompanyStreet, createResponse.Data!.Company!.FaktUlice); + + // Step 6: Re-query with the special char filter + var verifyResponse = await graphqlClient.Query( + companyWithSpecialCharFilter, + static (f, x) => new + { + Companies = x.Companies( + Filter: f.companyFilter, + selector: c => new { c.ID, c.Deleted, c.Kod, c.FaktUlice } + ), + }, + cancellationToken: TestContext.Current.CancellationToken + ); + Assert.Empty(verifyResponse.Errors ?? []); + + var foundCompany = verifyResponse.Data?.Companies? + .AsValueEnumerable() + .Where(c => c!.Deleted is false) + .FirstOrDefault(); + + Assert.NotNull(foundCompany); + Assert.Equal(testCompanyStreet, foundCompany.FaktUlice); + } +} diff --git a/src/Aviationexam.MoneyErp.Graphql/packages.lock.json b/src/Aviationexam.MoneyErp.Graphql/packages.lock.json index 0d4cc91..2d7b07e 100644 --- a/src/Aviationexam.MoneyErp.Graphql/packages.lock.json +++ b/src/Aviationexam.MoneyErp.Graphql/packages.lock.json @@ -421,12 +421,12 @@ }, "OpenTelemetry.Api.ProviderBuilderExtensions": { "type": "Direct", - "requested": "[1.15.1, )", - "resolved": "1.15.1", - "contentHash": "aZedpOfXtHmVSWlebxJBeJg2DCdzds86mMowBTS6l+sjwV9LvQuZa0JDU9+S7FQvta4hnauxlCEYplbiDiYGeg==", + "requested": "[1.13.1, )", + "resolved": "1.13.1", + "contentHash": "x8QXMsrIyp+XzDUFQAM+C4upAPNbwaBIPjTWEoonziWAav6weS8OxsMKrE4wz7Zly8ATlsoxk0mWZ+PHO3Wg0w==", "dependencies": { "Microsoft.Extensions.DependencyInjection.Abstractions": "9.0.0", - "OpenTelemetry.Api": "1.15.1" + "OpenTelemetry.Api": "1.13.1" } }, "ZeroQL": { @@ -516,16 +516,8 @@ }, "OpenTelemetry.Api": { "type": "Transitive", - "resolved": "1.15.1", - "contentHash": "+LJP0YBrysh4kPCRZhEyTUTcd+FFP0NPDvV4AzULBmiInGt6fp+RgBieRhUzVX/yyVEyshg3s82RWFYZJIkeGQ==", - "dependencies": { - "System.Diagnostics.DiagnosticSource": "10.0.0" - } - }, - "System.Diagnostics.DiagnosticSource": { - "type": "Transitive", - "resolved": "10.0.0", - "contentHash": "0KdBK+h7G13PuOSC2R/DalAoFMvdYMznvGRuICtkdcUMXgl/gYXsG6z4yUvTxHSMACorWgHCU1Faq0KUHU6yAQ==" + "resolved": "1.13.1", + "contentHash": "tieglRERo7Rgu8oE8aamnuXCMPEW5fXIqO5ngTMCNk9pOEXanc0SdQ86ZAD1goNiGcjWHn+P3WMZp0FZSJgCoQ==" }, "aviationexam.moneyerp.common": { "type": "Project", diff --git a/src/Directory.Packages.props b/src/Directory.Packages.props index fcf5a16..b3cf8e6 100644 --- a/src/Directory.Packages.props +++ b/src/Directory.Packages.props @@ -26,7 +26,7 @@ - +