Skip to content

Commit 8fab87c

Browse files
bkoelmanbkoelman
authored
Merge a8b3ecf into c0eb108 
2 parents c0eb108 + a8b3ecf commit 8fab87c

30 files changed

+662
-141
lines changed

‎benchmarks/QueryString/QueryStringParserBenchmarks.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ public QueryStringParserBenchmarks()
3838
var resourceFactory = new ResourceFactory(new ServiceContainer());
3939

4040
var includeReader = new IncludeQueryStringParameterReader(request, resourceGraph, options);
41-
var filterReader = new FilterQueryStringParameterReader(request, resourceGraph, resourceFactory, options);
41+
var filterReader = new FilterQueryStringParameterReader(request, resourceGraph, resourceFactory, options, Enumerable.Empty<IFilterValueConverter>());
4242
var sortReader = new SortQueryStringParameterReader(request, resourceGraph);
4343
var sparseFieldSetReader = new SparseFieldSetQueryStringParameterReader(request, resourceGraph);
4444
var paginationReader = new PaginationQueryStringParameterReader(request, resourceGraph, options);

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs‎

Lines changed: 133 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using JetBrains.Annotations;
44
using JsonApiDotNetCore.Configuration;
55
using JsonApiDotNetCore.Queries.Expressions;
6+
using JsonApiDotNetCore.QueryStrings;
67
using JsonApiDotNetCore.Resources;
78
using JsonApiDotNetCore.Resources.Annotations;
89
using JsonApiDotNetCore.Resources.Internal;
@@ -13,15 +14,16 @@ namespace JsonApiDotNetCore.Queries.Internal.Parsing;
1314
public class FilterParser : QueryExpressionParser
1415
{
1516
private readonly IResourceFactory _resourceFactory;
16-
private readonly Action<ResourceFieldAttribute, ResourceType, string>? _validateSingleFieldCallback;
17+
private readonly IEnumerable<IFilterValueConverter> _filterValueConverters;
1718
private ResourceType? _resourceTypeInScope;
1819

19-
public FilterParser(IResourceFactory resourceFactory, Action<ResourceFieldAttribute, ResourceType, string>? validateSingleFieldCallback = null)
20+
public FilterParser(IResourceFactory resourceFactory, IEnumerable<IFilterValueConverter> filterValueConverters)
2021
{
2122
ArgumentGuard.NotNull(resourceFactory);
23+
ArgumentGuard.NotNull(filterValueConverters);
2224

2325
_resourceFactory = resourceFactory;
24-
_validateSingleFieldCallback = validateSingleFieldCallback;
26+
_filterValueConverters = filterValueConverters;
2527
}
2628

2729
public FilterExpression Parse(string source, ResourceType resourceTypeInScope)
@@ -135,40 +137,34 @@ protected ComparisonExpression ParseComparison(string operatorName)
135137
EatText(operatorName);
136138
EatSingleCharacterToken(TokenKind.OpenParen);
137139

138-
// Allow equality comparison of a HasOne relationship with null.
140+
// Allow equality comparison of a to-one relationship with null.
139141
FieldChainRequirements leftChainRequirements = comparisonOperator == ComparisonOperator.Equals
140142
? FieldChainRequirements.EndsInAttribute | FieldChainRequirements.EndsInToOne
141143
: FieldChainRequirements.EndsInAttribute;
142144

143145
QueryExpression leftTerm = ParseCountOrField(leftChainRequirements);
144-
Converter<string, object> rightConstantValueConverter;
146+
147+
EatSingleCharacterToken(TokenKind.Comma);
148+
149+
QueryExpression rightTerm;
145150

146151
if (leftTerm is CountExpression)
147152
{
148-
rightConstantValueConverter = GetConstantValueConverterForCount();
153+
Converter<string, object> rightConstantValueConverter = GetConstantValueConverterForCount();
154+
rightTerm = ParseCountOrConstantOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
149155
}
150156
else if (leftTerm is ResourceFieldChainExpression fieldChain && fieldChain.Fields[^1] is AttrAttribute attribute)
151157
{
152-
rightConstantValueConverter = GetConstantValueConverterForAttribute(attribute);
158+
Converter<string, object> rightConstantValueConverter = GetConstantValueConverterForAttribute(attribute, typeof(ComparisonExpression));
159+
rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
153160
}
154161
else
155162
{
156-
// This temporary value never survives; it gets discarded during the second pass below.
157-
rightConstantValueConverter = _ => 0;
163+
rightTerm = ParseNull();
158164
}
159165

160-
EatSingleCharacterToken(TokenKind.Comma);
161-
162-
QueryExpression rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
163-
164166
EatSingleCharacterToken(TokenKind.CloseParen);
165167

166-
if (leftTerm is ResourceFieldChainExpression leftChain && leftChain.Fields[^1] is RelationshipAttribute && rightTerm is not NullConstantExpression)
167-
{
168-
// Run another pass over left chain to produce an error.
169-
OnResolveFieldChain(leftChain.ToString(), FieldChainRequirements.EndsInAttribute);
170-
}
171-
172168
return new ComparisonExpression(comparisonOperator, leftTerm, rightTerm);
173169
}
174170

@@ -178,16 +174,11 @@ protected MatchTextExpression ParseTextMatch(string matchFunctionName)
178174
EatSingleCharacterToken(TokenKind.OpenParen);
179175

180176
ResourceFieldChainExpression targetAttributeChain = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
181-
Type targetAttributeType = ((AttrAttribute)targetAttributeChain.Fields[^1]).Property.PropertyType;
182-
183-
if (targetAttributeType != typeof(string))
184-
{
185-
throw new QueryParseException("Attribute of type 'String' expected.");
186-
}
177+
var targetAttribute = (AttrAttribute)targetAttributeChain.Fields[^1];
187178

188179
EatSingleCharacterToken(TokenKind.Comma);
189180

190-
Converter<string, object> constantValueConverter = stringValue => stringValue;
181+
Converter<string, object> constantValueConverter = GetConstantValueConverterForAttribute(targetAttribute, typeof(MatchTextExpression));
191182
LiteralConstantExpression constant = ParseConstant(constantValueConverter);
192183

193184
EatSingleCharacterToken(TokenKind.CloseParen);
@@ -201,13 +192,14 @@ protected AnyExpression ParseAny()
201192
EatText(Keywords.Any);
202193
EatSingleCharacterToken(TokenKind.OpenParen);
203194

204-
ResourceFieldChainExpression targetAttribute = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
205-
Converter<string, object> constantValueConverter = GetConstantValueConverterForAttribute((AttrAttribute)targetAttribute.Fields[^1]);
195+
ResourceFieldChainExpression targetAttributeChain = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
196+
var targetAttribute = (AttrAttribute)targetAttributeChain.Fields[^1];
206197

207198
EatSingleCharacterToken(TokenKind.Comma);
208199

209200
ImmutableHashSet<LiteralConstantExpression>.Builder constantsBuilder = ImmutableHashSet.CreateBuilder<LiteralConstantExpression>();
210201

202+
Converter<string, object> constantValueConverter = GetConstantValueConverterForAttribute(targetAttribute, typeof(AnyExpression));
211203
LiteralConstantExpression constant = ParseConstant(constantValueConverter);
212204
constantsBuilder.Add(constant);
213205

@@ -223,7 +215,7 @@ protected AnyExpression ParseAny()
223215

224216
IImmutableSet<LiteralConstantExpression> constantSet = constantsBuilder.ToImmutable();
225217

226-
return new AnyExpression(targetAttribute, constantSet);
218+
return new AnyExpression(targetAttributeChain, constantSet);
227219
}
228220

229221
protected HasExpression ParseHas()
@@ -349,6 +341,25 @@ protected QueryExpression ParseCountOrField(FieldChainRequirements chainRequirem
349341
return ParseFieldChain(chainRequirements, "Count function or field name expected.");
350342
}
351343

344+
protected QueryExpression ParseCountOrConstantOrField(FieldChainRequirements chainRequirements, Converter<string, object> constantValueConverter)
345+
{
346+
CountExpression? count = TryParseCount();
347+
348+
if (count != null)
349+
{
350+
return count;
351+
}
352+
353+
LiteralConstantExpression? constant = TryParseConstant(constantValueConverter);
354+
355+
if (constant != null)
356+
{
357+
return constant;
358+
}
359+
360+
return ParseFieldChain(chainRequirements, "Count function, value between quotes or field name expected.");
361+
}
362+
352363
protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequirements chainRequirements, Converter<string, object> constantValueConverter)
353364
{
354365
CountExpression? count = TryParseCount();
@@ -368,6 +379,19 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
368379
return ParseFieldChain(chainRequirements, "Count function, value between quotes, null or field name expected.");
369380
}
370381

382+
protected LiteralConstantExpression? TryParseConstant(Converter<string, object> constantValueConverter)
383+
{
384+
if (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.QuotedText)
385+
{
386+
TokenStack.Pop();
387+
388+
object constantValue = constantValueConverter(nextToken.Value!);
389+
return new LiteralConstantExpression(constantValue, nextToken.Value!);
390+
}
391+
392+
return null;
393+
}
394+
371395
protected IdentifierExpression? TryParseConstantOrNull(Converter<string, object> constantValueConverter)
372396
{
373397
if (TokenStack.TryPeek(out Token? nextToken))
@@ -392,37 +416,93 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
392416

393417
protected LiteralConstantExpression ParseConstant(Converter<string, object> constantValueConverter)
394418
{
395-
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.QuotedText)
419+
LiteralConstantExpression? constant = TryParseConstant(constantValueConverter);
420+
421+
if (constant == null)
422+
{
423+
throw new QueryParseException("Value between quotes expected.");
424+
}
425+
426+
return constant;
427+
}
428+
429+
protected NullConstantExpression ParseNull()
430+
{
431+
if (TokenStack.TryPop(out Token? token) && token is { Kind: TokenKind.Text, Value: Keywords.Null })
396432
{
397-
object constantValue = constantValueConverter(token.Value!);
398-
return new LiteralConstantExpression(constantValue, token.Value!);
433+
return NullConstantExpression.Instance;
399434
}
400435

401-
throw new QueryParseException("Value between quotes expected.");
436+
throw new QueryParseException("null expected.");
402437
}
403438

404439
private Converter<string, object> GetConstantValueConverterForCount()
405440
{
406441
return stringValue => ConvertStringToType(stringValue, typeof(int));
407442
}
408443

409-
private object ConvertStringToType(string value, Type type)
444+
private static object ConvertStringToType(string value, Type type)
410445
{
411446
try
412447
{
413448
return RuntimeTypeConverter.ConvertType(value, type)!;
414449
}
415-
catch (FormatException)
450+
catch (FormatException exception)
416451
{
417-
throw new QueryParseException($"Failed to convert '{value}' of type 'String' to type '{type.Name}'.");
452+
throw new QueryParseException($"Failed to convert '{value}' of type 'String' to type '{type.Name}'.", exception);
418453
}
419454
}
420455

421-
private Converter<string, object> GetConstantValueConverterForAttribute(AttrAttribute attribute)
456+
private Converter<string, object> GetConstantValueConverterForAttribute(AttrAttribute attribute, Type outerExpressionType)
422457
{
423-
return stringValue => attribute.Property.Name == nameof(Identifiable<object>.Id)
424-
? DeObfuscateStringId(attribute.Type.ClrType, stringValue)
425-
: ConvertStringToType(stringValue, attribute.Property.PropertyType);
458+
return stringValue =>
459+
{
460+
object? value = TryConvertFromStringUsingFilterValueConverters(attribute, stringValue, outerExpressionType);
461+
462+
if (value != null)
463+
{
464+
return value;
465+
}
466+
467+
if (outerExpressionType == typeof(MatchTextExpression))
468+
{
469+
if (attribute.Property.PropertyType != typeof(string))
470+
{
471+
throw new QueryParseException("Attribute of type 'String' expected.");
472+
}
473+
}
474+
else
475+
{
476+
// Partial text matching on an obfuscated ID usually fails.
477+
if (attribute.Property.Name == nameof(Identifiable<object>.Id))
478+
{
479+
return DeObfuscateStringId(attribute.Type.ClrType, stringValue);
480+
}
481+
}
482+
483+
return ConvertStringToType(stringValue, attribute.Property.PropertyType);
484+
};
485+
}
486+
487+
private object? TryConvertFromStringUsingFilterValueConverters(AttrAttribute attribute, string stringValue, Type outerExpressionType)
488+
{
489+
foreach (IFilterValueConverter converter in _filterValueConverters)
490+
{
491+
if (converter.CanConvert(attribute))
492+
{
493+
object result = converter.Convert(attribute, stringValue, outerExpressionType);
494+
495+
if (result == null)
496+
{
497+
throw new InvalidOperationException(
498+
$"Converter '{converter.GetType().Name}' returned null for '{stringValue}' on attribute '{attribute.PublicName}'. Return a sentinel value instead.");
499+
}
500+
501+
return result;
502+
}
503+
}
504+
505+
return null;
426506
}
427507

428508
private object DeObfuscateStringId(Type resourceClrType, string stringId)
@@ -436,29 +516,37 @@ protected override IImmutableList<ResourceFieldAttribute> OnResolveFieldChain(st
436516
{
437517
if (chainRequirements == FieldChainRequirements.EndsInToMany)
438518
{
439-
return ChainResolver.ResolveToOneChainEndingInToMany(_resourceTypeInScope!, path, FieldChainInheritanceRequirement.Disabled,
440-
_validateSingleFieldCallback);
519+
return ChainResolver.ResolveToOneChainEndingInToMany(_resourceTypeInScope!, path, FieldChainInheritanceRequirement.Disabled, ValidateSingleField);
441520
}
442521

443522
if (chainRequirements == FieldChainRequirements.EndsInAttribute)
444523
{
445524
return ChainResolver.ResolveToOneChainEndingInAttribute(_resourceTypeInScope!, path, FieldChainInheritanceRequirement.Disabled,
446-
_validateSingleFieldCallback);
525+
ValidateSingleField);
447526
}
448527

449528
if (chainRequirements == FieldChainRequirements.EndsInToOne)
450529
{
451-
return ChainResolver.ResolveToOneChain(_resourceTypeInScope!, path, _validateSingleFieldCallback);
530+
return ChainResolver.ResolveToOneChain(_resourceTypeInScope!, path, ValidateSingleField);
452531
}
453532

454533
if (chainRequirements.HasFlag(FieldChainRequirements.EndsInAttribute) && chainRequirements.HasFlag(FieldChainRequirements.EndsInToOne))
455534
{
456-
return ChainResolver.ResolveToOneChainEndingInAttributeOrToOne(_resourceTypeInScope!, path, _validateSingleFieldCallback);
535+
return ChainResolver.ResolveToOneChainEndingInAttributeOrToOne(_resourceTypeInScope!, path, ValidateSingleField);
457536
}
458537

459538
throw new InvalidOperationException($"Unexpected combination of chain requirement flags '{chainRequirements}'.");
460539
}
461540

541+
protected override void ValidateSingleField(ResourceFieldAttribute field, ResourceType resourceType, string path)
542+
{
543+
if (field.IsFilterBlocked())
544+
{
545+
string kind = field is AttrAttribute ? "attribute" : "relationship";
546+
throw new QueryParseException($"Filtering on {kind} '{field.PublicName}' is not allowed.");
547+
}
548+
}
549+
462550
private TResult InScopeOfResourceType<TResult>(ResourceType resourceType, Func<TResult> action)
463551
{
464552
ResourceType? backupType = _resourceTypeInScope;

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/IncludeParser.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private static void AssertAtLeastOneCanBeIncluded(ISet<RelationshipAttribute> re
154154
? $"Including the relationship '{relationshipName}' on '{resourceType}' is not allowed."
155155
: $"Including the relationship '{relationshipName}' in '{parentPath}.{relationshipName}' on '{resourceType}' is not allowed.";
156156

157-
throw new InvalidQueryStringParameterException("include", "Including the requested relationship is not allowed.", message);
157+
throw new InvalidQueryStringParameterException("include", "The specified include is invalid.", message);
158158
}
159159
}
160160

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/PaginationParser.cs‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,8 @@ namespace JsonApiDotNetCore.Queries.Internal.Parsing;
99
[PublicAPI]
1010
public class PaginationParser : QueryExpressionParser
1111
{
12-
private readonly Action<ResourceFieldAttribute, ResourceType, string>? _validateSingleFieldCallback;
1312
private ResourceType? _resourceTypeInScope;
1413

15-
public PaginationParser(Action<ResourceFieldAttribute, ResourceType, string>? validateSingleFieldCallback = null)
16-
{
17-
_validateSingleFieldCallback = validateSingleFieldCallback;
18-
}
19-
2014
public PaginationQueryStringValueExpression Parse(string source, ResourceType resourceTypeInScope)
2115
{
2216
ArgumentGuard.NotNull(resourceTypeInScope);
@@ -104,6 +98,6 @@ protected PaginationElementQueryStringValueExpression ParsePaginationElement()
10498

10599
protected override IImmutableList<ResourceFieldAttribute> OnResolveFieldChain(string path, FieldChainRequirements chainRequirements)
106100
{
107-
return ChainResolver.ResolveToManyChain(_resourceTypeInScope!, path, _validateSingleFieldCallback);
101+
return ChainResolver.ResolveToManyChain(_resourceTypeInScope!, path);
108102
}
109103
}

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/QueryExpressionParser.cs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System.Collections.Immutable;
22
using System.Text;
33
using JetBrains.Annotations;
4+
using JsonApiDotNetCore.Configuration;
45
using JsonApiDotNetCore.Queries.Expressions;
56
using JsonApiDotNetCore.Resources.Annotations;
67

@@ -24,6 +25,10 @@ public abstract class QueryExpressionParser
2425
/// </summary>
2526
protected abstract IImmutableList<ResourceFieldAttribute> OnResolveFieldChain(string path, FieldChainRequirements chainRequirements);
2627

28+
protected virtual void ValidateSingleField(ResourceFieldAttribute field, ResourceType resourceType, string path)
29+
{
30+
}
31+
2732
protected virtual void Tokenize(string source)
2833
{
2934
var tokenizer = new QueryTokenizer(source);
@@ -49,7 +54,7 @@ private void EatFieldChain(StringBuilder pathBuilder, string? alternativeErrorMe
4954
{
5055
while (true)
5156
{
52-
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text)
57+
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text && token.Value != Keywords.Null)
5358
{
5459
pathBuilder.Append(token.Value);
5560

‎src/JsonApiDotNetCore/Queries/Internal/Parsing/QueryParseException.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,9 @@ public QueryParseException(string message)
99
: base(message)
1010
{
1111
}
12+
13+
public QueryParseException(string message, Exception innerException)
14+
: base(message, innerException)
15+
{
16+
}
1217
}

0 commit comments

Comments
 (0)