Skip to content

Pluggable filter value conversions #1284

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.

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion benchmarks/QueryString/QueryStringParserBenchmarks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public QueryStringParserBenchmarks()
var resourceFactory = new ResourceFactory(new ServiceContainer());

var includeReader = new IncludeQueryStringParameterReader(request, resourceGraph, options);
var filterReader = new FilterQueryStringParameterReader(request, resourceGraph, resourceFactory, options);
var filterReader = new FilterQueryStringParameterReader(request, resourceGraph, resourceFactory, options, Enumerable.Empty<IFilterValueConverter>());
var sortReader = new SortQueryStringParameterReader(request, resourceGraph);
var sparseFieldSetReader = new SparseFieldSetQueryStringParameterReader(request, resourceGraph);
var paginationReader = new PaginationQueryStringParameterReader(request, resourceGraph, options);
Expand Down
178 changes: 133 additions & 45 deletions src/JsonApiDotNetCore/Queries/Internal/Parsing/FilterParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using JetBrains.Annotations;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Queries.Expressions;
using JsonApiDotNetCore.QueryStrings;
using JsonApiDotNetCore.Resources;
using JsonApiDotNetCore.Resources.Annotations;
using JsonApiDotNetCore.Resources.Internal;
Expand All @@ -13,15 +14,16 @@ namespace JsonApiDotNetCore.Queries.Internal.Parsing;
public class FilterParser : QueryExpressionParser
{
private readonly IResourceFactory _resourceFactory;
private readonly Action<ResourceFieldAttribute, ResourceType, string>? _validateSingleFieldCallback;
private readonly IEnumerable<IFilterValueConverter> _filterValueConverters;
private ResourceType? _resourceTypeInScope;

public FilterParser(IResourceFactory resourceFactory, Action<ResourceFieldAttribute, ResourceType, string>? validateSingleFieldCallback = null)
public FilterParser(IResourceFactory resourceFactory, IEnumerable<IFilterValueConverter> filterValueConverters)
{
ArgumentGuard.NotNull(resourceFactory);
ArgumentGuard.NotNull(filterValueConverters);

_resourceFactory = resourceFactory;
_validateSingleFieldCallback = validateSingleFieldCallback;
_filterValueConverters = filterValueConverters;
}

public FilterExpression Parse(string source, ResourceType resourceTypeInScope)
Expand Down Expand Up @@ -135,40 +137,34 @@ protected ComparisonExpression ParseComparison(string operatorName)
EatText(operatorName);
EatSingleCharacterToken(TokenKind.OpenParen);

// Allow equality comparison of a HasOne relationship with null.
// Allow equality comparison of a to-one relationship with null.
FieldChainRequirements leftChainRequirements = comparisonOperator == ComparisonOperator.Equals
? FieldChainRequirements.EndsInAttribute | FieldChainRequirements.EndsInToOne
: FieldChainRequirements.EndsInAttribute;

QueryExpression leftTerm = ParseCountOrField(leftChainRequirements);
Converter<string, object> rightConstantValueConverter;

EatSingleCharacterToken(TokenKind.Comma);

QueryExpression rightTerm;

if (leftTerm is CountExpression)
{
rightConstantValueConverter = GetConstantValueConverterForCount();
Converter<string, object> rightConstantValueConverter = GetConstantValueConverterForCount();
rightTerm = ParseCountOrConstantOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
}
else if (leftTerm is ResourceFieldChainExpression fieldChain && fieldChain.Fields[^1] is AttrAttribute attribute)
{
rightConstantValueConverter = GetConstantValueConverterForAttribute(attribute);
Converter<string, object> rightConstantValueConverter = GetConstantValueConverterForAttribute(attribute, typeof(ComparisonExpression));
rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
}
else
{
// This temporary value never survives; it gets discarded during the second pass below.
rightConstantValueConverter = _ => 0;
rightTerm = ParseNull();
}

EatSingleCharacterToken(TokenKind.Comma);

QueryExpression rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);

EatSingleCharacterToken(TokenKind.CloseParen);

if (leftTerm is ResourceFieldChainExpression leftChain && leftChain.Fields[^1] is RelationshipAttribute && rightTerm is not NullConstantExpression)
{
// Run another pass over left chain to produce an error.
OnResolveFieldChain(leftChain.ToString(), FieldChainRequirements.EndsInAttribute);
}

return new ComparisonExpression(comparisonOperator, leftTerm, rightTerm);
}

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

ResourceFieldChainExpression targetAttributeChain = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
Type targetAttributeType = ((AttrAttribute)targetAttributeChain.Fields[^1]).Property.PropertyType;

if (targetAttributeType != typeof(string))
{
throw new QueryParseException("Attribute of type 'String' expected.");
}
var targetAttribute = (AttrAttribute)targetAttributeChain.Fields[^1];

EatSingleCharacterToken(TokenKind.Comma);

Converter<string, object> constantValueConverter = stringValue => stringValue;
Converter<string, object> constantValueConverter = GetConstantValueConverterForAttribute(targetAttribute, typeof(MatchTextExpression));
LiteralConstantExpression constant = ParseConstant(constantValueConverter);

EatSingleCharacterToken(TokenKind.CloseParen);
Expand All @@ -201,13 +192,14 @@ protected AnyExpression ParseAny()
EatText(Keywords.Any);
EatSingleCharacterToken(TokenKind.OpenParen);

ResourceFieldChainExpression targetAttribute = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
Converter<string, object> constantValueConverter = GetConstantValueConverterForAttribute((AttrAttribute)targetAttribute.Fields[^1]);
ResourceFieldChainExpression targetAttributeChain = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
var targetAttribute = (AttrAttribute)targetAttributeChain.Fields[^1];

EatSingleCharacterToken(TokenKind.Comma);

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

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

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

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

return new AnyExpression(targetAttribute, constantSet);
return new AnyExpression(targetAttributeChain, constantSet);
}

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

protected QueryExpression ParseCountOrConstantOrField(FieldChainRequirements chainRequirements, Converter<string, object> constantValueConverter)
{
CountExpression? count = TryParseCount();

if (count != null)
{
return count;
}

LiteralConstantExpression? constant = TryParseConstant(constantValueConverter);

if (constant != null)
{
return constant;
}

return ParseFieldChain(chainRequirements, "Count function, value between quotes or field name expected.");
}

protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequirements chainRequirements, Converter<string, object> constantValueConverter)
{
CountExpression? count = TryParseCount();
Expand All @@ -368,6 +379,19 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
return ParseFieldChain(chainRequirements, "Count function, value between quotes, null or field name expected.");
}

protected LiteralConstantExpression? TryParseConstant(Converter<string, object> constantValueConverter)
{
if (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.QuotedText)
{
TokenStack.Pop();

object constantValue = constantValueConverter(nextToken.Value!);
return new LiteralConstantExpression(constantValue, nextToken.Value!);
}

return null;
}

protected IdentifierExpression? TryParseConstantOrNull(Converter<string, object> constantValueConverter)
{
if (TokenStack.TryPeek(out Token? nextToken))
Expand All @@ -392,37 +416,93 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen

protected LiteralConstantExpression ParseConstant(Converter<string, object> constantValueConverter)
{
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.QuotedText)
LiteralConstantExpression? constant = TryParseConstant(constantValueConverter);

if (constant == null)
{
throw new QueryParseException("Value between quotes expected.");
}

return constant;
}

protected NullConstantExpression ParseNull()
{
if (TokenStack.TryPop(out Token? token) && token is { Kind: TokenKind.Text, Value: Keywords.Null })
{
object constantValue = constantValueConverter(token.Value!);
return new LiteralConstantExpression(constantValue, token.Value!);
return NullConstantExpression.Instance;
}

throw new QueryParseException("Value between quotes expected.");
throw new QueryParseException("null expected.");
}

private Converter<string, object> GetConstantValueConverterForCount()
{
return stringValue => ConvertStringToType(stringValue, typeof(int));
}

private object ConvertStringToType(string value, Type type)
private static object ConvertStringToType(string value, Type type)
{
try
{
return RuntimeTypeConverter.ConvertType(value, type)!;
}
catch (FormatException)
catch (FormatException exception)
{
throw new QueryParseException($"Failed to convert '{value}' of type 'String' to type '{type.Name}'.");
throw new QueryParseException($"Failed to convert '{value}' of type 'String' to type '{type.Name}'.", exception);
}
}

private Converter<string, object> GetConstantValueConverterForAttribute(AttrAttribute attribute)
private Converter<string, object> GetConstantValueConverterForAttribute(AttrAttribute attribute, Type outerExpressionType)
{
return stringValue => attribute.Property.Name == nameof(Identifiable<object>.Id)
? DeObfuscateStringId(attribute.Type.ClrType, stringValue)
: ConvertStringToType(stringValue, attribute.Property.PropertyType);
return stringValue =>
{
object? value = TryConvertFromStringUsingFilterValueConverters(attribute, stringValue, outerExpressionType);

if (value != null)
{
return value;
}

if (outerExpressionType == typeof(MatchTextExpression))
{
if (attribute.Property.PropertyType != typeof(string))
{
throw new QueryParseException("Attribute of type 'String' expected.");
}
}
else
{
// Partial text matching on an obfuscated ID usually fails.
if (attribute.Property.Name == nameof(Identifiable<object>.Id))
{
return DeObfuscateStringId(attribute.Type.ClrType, stringValue);
}
}

return ConvertStringToType(stringValue, attribute.Property.PropertyType);
};
}

private object? TryConvertFromStringUsingFilterValueConverters(AttrAttribute attribute, string stringValue, Type outerExpressionType)
{
foreach (IFilterValueConverter converter in _filterValueConverters)
{
if (converter.CanConvert(attribute))
{
object result = converter.Convert(attribute, stringValue, outerExpressionType);

if (result == null)
{
throw new InvalidOperationException(
$"Converter '{converter.GetType().Name}' returned null for '{stringValue}' on attribute '{attribute.PublicName}'. Return a sentinel value instead.");
}

return result;
}
}

return null;
}

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

if (chainRequirements == FieldChainRequirements.EndsInAttribute)
{
return ChainResolver.ResolveToOneChainEndingInAttribute(_resourceTypeInScope!, path, FieldChainInheritanceRequirement.Disabled,
_validateSingleFieldCallback);
ValidateSingleField);
}

if (chainRequirements == FieldChainRequirements.EndsInToOne)
{
return ChainResolver.ResolveToOneChain(_resourceTypeInScope!, path, _validateSingleFieldCallback);
return ChainResolver.ResolveToOneChain(_resourceTypeInScope!, path, ValidateSingleField);
}

if (chainRequirements.HasFlag(FieldChainRequirements.EndsInAttribute) && chainRequirements.HasFlag(FieldChainRequirements.EndsInToOne))
{
return ChainResolver.ResolveToOneChainEndingInAttributeOrToOne(_resourceTypeInScope!, path, _validateSingleFieldCallback);
return ChainResolver.ResolveToOneChainEndingInAttributeOrToOne(_resourceTypeInScope!, path, ValidateSingleField);
}

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

protected override void ValidateSingleField(ResourceFieldAttribute field, ResourceType resourceType, string path)
{
if (field.IsFilterBlocked())
{
string kind = field is AttrAttribute ? "attribute" : "relationship";
throw new QueryParseException($"Filtering on {kind} '{field.PublicName}' is not allowed.");
}
}

private TResult InScopeOfResourceType<TResult>(ResourceType resourceType, Func<TResult> action)
{
ResourceType? backupType = _resourceTypeInScope;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private static void AssertAtLeastOneCanBeIncluded(ISet<RelationshipAttribute> re
? $"Including the relationship '{relationshipName}' on '{resourceType}' is not allowed."
: $"Including the relationship '{relationshipName}' in '{parentPath}.{relationshipName}' on '{resourceType}' is not allowed.";

throw new InvalidQueryStringParameterException("include", "Including the requested relationship is not allowed.", message);
throw new InvalidQueryStringParameterException("include", "The specified include is invalid.", message);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,8 @@ namespace JsonApiDotNetCore.Queries.Internal.Parsing;
[PublicAPI]
public class PaginationParser : QueryExpressionParser
{
private readonly Action<ResourceFieldAttribute, ResourceType, string>? _validateSingleFieldCallback;
private ResourceType? _resourceTypeInScope;

public PaginationParser(Action<ResourceFieldAttribute, ResourceType, string>? validateSingleFieldCallback = null)
{
_validateSingleFieldCallback = validateSingleFieldCallback;
}

public PaginationQueryStringValueExpression Parse(string source, ResourceType resourceTypeInScope)
{
ArgumentGuard.NotNull(resourceTypeInScope);
Expand Down Expand Up @@ -104,6 +98,6 @@ protected PaginationElementQueryStringValueExpression ParsePaginationElement()

protected override IImmutableList<ResourceFieldAttribute> OnResolveFieldChain(string path, FieldChainRequirements chainRequirements)
{
return ChainResolver.ResolveToManyChain(_resourceTypeInScope!, path, _validateSingleFieldCallback);
return ChainResolver.ResolveToManyChain(_resourceTypeInScope!, path);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System.Collections.Immutable;
using System.Text;
using JetBrains.Annotations;
using JsonApiDotNetCore.Configuration;
using JsonApiDotNetCore.Queries.Expressions;
using JsonApiDotNetCore.Resources.Annotations;

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

protected virtual void ValidateSingleField(ResourceFieldAttribute field, ResourceType resourceType, string path)
{
}

protected virtual void Tokenize(string source)
{
var tokenizer = new QueryTokenizer(source);
Expand All @@ -49,7 +54,7 @@ private void EatFieldChain(StringBuilder pathBuilder, string? alternativeErrorMe
{
while (true)
{
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text)
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.Text && token.Value != Keywords.Null)
{
pathBuilder.Append(token.Value);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,4 +9,9 @@ public QueryParseException(string message)
: base(message)
{
}

public QueryParseException(string message, Exception innerException)
: base(message, innerException)
{
}
}
Loading