Skip to content

Commit 221c33f

Browse files
bkoelmanbkoelman
committed
Changes LiteralConstantExpression to contain a typed constant instead of a string value. This effectively moves the type resolving/conversion logic from the EF query production phase to the filter parsing phase. By providing a richer QueryLayer model, it becomes easier to implement non-relational and non-EF Core-based repositories. And it enables to produce better errors earlier.
Note we still need to wrap nullable values (see WhereClauseBuilder.ResolveCommonType), due to https://bradwilson.typepad.com/blog/2008/07/creating-nullab.html.
1 parent ac7d3ae commit 221c33f

File tree

7 files changed

+156
-99
lines changed

7 files changed

+156
-99
lines changed

‎src/JsonApiDotNetCore/Queries/Expressions/LiteralConstantExpression.cs‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,22 @@ namespace JsonApiDotNetCore.Queries.Expressions;
88
[PublicAPI]
99
public class LiteralConstantExpression : IdentifierExpression
1010
{
11-
public string Value { get; }
11+
private readonly string _stringValue;
1212

13-
public LiteralConstantExpression(string text)
13+
public object TypedValue { get; }
14+
15+
public LiteralConstantExpression(object typedValue)
16+
: this(typedValue, typedValue.ToString()!)
17+
{
18+
}
19+
20+
public LiteralConstantExpression(object typedValue, string stringValue)
1421
{
15-
ArgumentGuard.NotNull(text);
22+
ArgumentGuard.NotNull(typedValue);
23+
ArgumentGuard.NotNull(stringValue);
1624

17-
Value = text;
25+
TypedValue = typedValue;
26+
_stringValue = stringValue;
1827
}
1928

2029
public override TResult Accept<TArgument, TResult>(QueryExpressionVisitor<TArgument, TResult> visitor, TArgument argument)
@@ -24,8 +33,8 @@ public override TResult Accept<TArgument, TResult>(QueryExpressionVisitor<TArgum
2433

2534
public override string ToString()
2635
{
27-
string value = Value.Replace("\'", "\'\'");
28-
return $"'{value}'";
36+
string escapedValue = _stringValue.Replace("\'", "\'\'");
37+
return $"'{escapedValue}'";
2938
}
3039

3140
public override string ToFullString()
@@ -47,11 +56,11 @@ public override bool Equals(object? obj)
4756

4857
var other = (LiteralConstantExpression)obj;
4958

50-
return Value == other.Value;
59+
return Equals(TypedValue, other.TypedValue) && _stringValue == other._stringValue;
5160
}
5261

5362
public override int GetHashCode()
5463
{
55-
return Value.GetHashCode();
64+
return HashCode.Combine(TypedValue, _stringValue);
5665
}
5766
}

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

Lines changed: 68 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
using System.Collections.Immutable;
2-
using System.Reflection;
32
using Humanizer;
43
using JetBrains.Annotations;
54
using JsonApiDotNetCore.Configuration;
65
using JsonApiDotNetCore.Queries.Expressions;
76
using JsonApiDotNetCore.Resources;
87
using JsonApiDotNetCore.Resources.Annotations;
8+
using JsonApiDotNetCore.Resources.Internal;
99

1010
namespace JsonApiDotNetCore.Queries.Internal.Parsing;
1111

@@ -141,28 +141,32 @@ protected ComparisonExpression ParseComparison(string operatorName)
141141
: FieldChainRequirements.EndsInAttribute;
142142

143143
QueryExpression leftTerm = ParseCountOrField(leftChainRequirements);
144+
Converter<string, object> rightConstantValueConverter;
145+
146+
if (leftTerm is CountExpression)
147+
{
148+
rightConstantValueConverter = GetConstantValueConverterForCount();
149+
}
150+
else if (leftTerm is ResourceFieldChainExpression fieldChain && fieldChain.Fields[^1] is AttrAttribute attribute)
151+
{
152+
rightConstantValueConverter = GetConstantValueConverterForAttribute(attribute);
153+
}
154+
else
155+
{
156+
// This temporary value never survives; it gets discarded during the second pass below.
157+
rightConstantValueConverter = _ => 0;
158+
}
144159

145160
EatSingleCharacterToken(TokenKind.Comma);
146161

147-
QueryExpression rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute);
162+
QueryExpression rightTerm = ParseCountOrConstantOrNullOrField(FieldChainRequirements.EndsInAttribute, rightConstantValueConverter);
148163

149164
EatSingleCharacterToken(TokenKind.CloseParen);
150165

151-
if (leftTerm is ResourceFieldChainExpression leftChain)
166+
if (leftTerm is ResourceFieldChainExpression leftChain && leftChain.Fields[^1] is RelationshipAttribute && rightTerm is not NullConstantExpression)
152167
{
153-
if (leftChainRequirements.HasFlag(FieldChainRequirements.EndsInToOne) && rightTerm is not NullConstantExpression)
154-
{
155-
// Run another pass over left chain to have it fail when chain ends in relationship.
156-
OnResolveFieldChain(leftChain.ToString(), FieldChainRequirements.EndsInAttribute);
157-
}
158-
159-
PropertyInfo leftProperty = leftChain.Fields[^1].Property;
160-
161-
if (leftProperty.Name == nameof(Identifiable<object>.Id) && rightTerm is LiteralConstantExpression rightConstant)
162-
{
163-
string id = DeObfuscateStringId(leftProperty.ReflectedType!, rightConstant.Value);
164-
rightTerm = new LiteralConstantExpression(id);
165-
}
168+
// Run another pass over left chain to produce an error.
169+
OnResolveFieldChain(leftChain.ToString(), FieldChainRequirements.EndsInAttribute);
166170
}
167171

168172
return new ComparisonExpression(comparisonOperator, leftTerm, rightTerm);
@@ -173,16 +177,23 @@ protected MatchTextExpression ParseTextMatch(string matchFunctionName)
173177
EatText(matchFunctionName);
174178
EatSingleCharacterToken(TokenKind.OpenParen);
175179

176-
ResourceFieldChainExpression targetAttribute = ParseFieldChain(FieldChainRequirements.EndsInAttribute, null);
180+
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+
}
177187

178188
EatSingleCharacterToken(TokenKind.Comma);
179189

180-
LiteralConstantExpression constant = ParseConstant();
190+
Converter<string, object> constantValueConverter = stringValue => stringValue;
191+
LiteralConstantExpression constant = ParseConstant(constantValueConverter);
181192

182193
EatSingleCharacterToken(TokenKind.CloseParen);
183194

184195
var matchKind = Enum.Parse<TextMatchKind>(matchFunctionName.Pascalize());
185-
return new MatchTextExpression(targetAttribute, constant, matchKind);
196+
return new MatchTextExpression(targetAttributeChain, constant, matchKind);
186197
}
187198

188199
protected AnyExpression ParseAny()
@@ -191,52 +202,30 @@ protected AnyExpression ParseAny()
191202
EatSingleCharacterToken(TokenKind.OpenParen);
192203

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

195207
EatSingleCharacterToken(TokenKind.Comma);
196208

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

199-
LiteralConstantExpression constant = ParseConstant();
211+
LiteralConstantExpression constant = ParseConstant(constantValueConverter);
200212
constantsBuilder.Add(constant);
201213

202214
while (TokenStack.TryPeek(out Token? nextToken) && nextToken.Kind == TokenKind.Comma)
203215
{
204216
EatSingleCharacterToken(TokenKind.Comma);
205217

206-
constant = ParseConstant();
218+
constant = ParseConstant(constantValueConverter);
207219
constantsBuilder.Add(constant);
208220
}
209221

210222
EatSingleCharacterToken(TokenKind.CloseParen);
211223

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

214-
PropertyInfo targetAttributeProperty = targetAttribute.Fields[^1].Property;
215-
216-
if (targetAttributeProperty.Name == nameof(Identifiable<object>.Id))
217-
{
218-
constantSet = DeObfuscateIdConstants(constantSet, targetAttributeProperty);
219-
}
220-
221226
return new AnyExpression(targetAttribute, constantSet);
222227
}
223228

224-
private IImmutableSet<LiteralConstantExpression> DeObfuscateIdConstants(IImmutableSet<LiteralConstantExpression> constantSet,
225-
PropertyInfo targetAttributeProperty)
226-
{
227-
ImmutableHashSet<LiteralConstantExpression>.Builder idConstantsBuilder = ImmutableHashSet.CreateBuilder<LiteralConstantExpression>();
228-
229-
foreach (LiteralConstantExpression idConstant in constantSet)
230-
{
231-
string stringId = idConstant.Value;
232-
string id = DeObfuscateStringId(targetAttributeProperty.ReflectedType!, stringId);
233-
234-
idConstantsBuilder.Add(new LiteralConstantExpression(id));
235-
}
236-
237-
return idConstantsBuilder.ToImmutable();
238-
}
239-
240229
protected HasExpression ParseHas()
241230
{
242231
EatText(Keywords.Has);
@@ -360,7 +349,7 @@ protected QueryExpression ParseCountOrField(FieldChainRequirements chainRequirem
360349
return ParseFieldChain(chainRequirements, "Count function or field name expected.");
361350
}
362351

363-
protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequirements chainRequirements)
352+
protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequirements chainRequirements, Converter<string, object> constantValueConverter)
364353
{
365354
CountExpression? count = TryParseCount();
366355

@@ -369,7 +358,7 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
369358
return count;
370359
}
371360

372-
IdentifierExpression? constantOrNull = TryParseConstantOrNull();
361+
IdentifierExpression? constantOrNull = TryParseConstantOrNull(constantValueConverter);
373362

374363
if (constantOrNull != null)
375364
{
@@ -379,7 +368,7 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
379368
return ParseFieldChain(chainRequirements, "Count function, value between quotes, null or field name expected.");
380369
}
381370

382-
protected IdentifierExpression? TryParseConstantOrNull()
371+
protected IdentifierExpression? TryParseConstantOrNull(Converter<string, object> constantValueConverter)
383372
{
384373
if (TokenStack.TryPeek(out Token? nextToken))
385374
{
@@ -392,28 +381,55 @@ protected QueryExpression ParseCountOrConstantOrNullOrField(FieldChainRequiremen
392381
if (nextToken.Kind == TokenKind.QuotedText)
393382
{
394383
TokenStack.Pop();
395-
return new LiteralConstantExpression(nextToken.Value!);
384+
385+
object constantValue = constantValueConverter(nextToken.Value!);
386+
return new LiteralConstantExpression(constantValue, nextToken.Value!);
396387
}
397388
}
398389

399390
return null;
400391
}
401392

402-
protected LiteralConstantExpression ParseConstant()
393+
protected LiteralConstantExpression ParseConstant(Converter<string, object> constantValueConverter)
403394
{
404395
if (TokenStack.TryPop(out Token? token) && token.Kind == TokenKind.QuotedText)
405396
{
406-
return new LiteralConstantExpression(token.Value!);
397+
object constantValue = constantValueConverter(token.Value!);
398+
return new LiteralConstantExpression(constantValue, token.Value!);
407399
}
408400

409401
throw new QueryParseException("Value between quotes expected.");
410402
}
411403

412-
private string DeObfuscateStringId(Type resourceClrType, string stringId)
404+
private Converter<string, object> GetConstantValueConverterForCount()
405+
{
406+
return stringValue => ConvertStringToType(stringValue, typeof(int));
407+
}
408+
409+
private object ConvertStringToType(string value, Type type)
410+
{
411+
try
412+
{
413+
return RuntimeTypeConverter.ConvertType(value, type)!;
414+
}
415+
catch (FormatException)
416+
{
417+
throw new QueryParseException($"Failed to convert '{value}' of type 'String' to type '{type.Name}'.");
418+
}
419+
}
420+
421+
private Converter<string, object> GetConstantValueConverterForAttribute(AttrAttribute attribute)
422+
{
423+
return stringValue => attribute.Property.Name == nameof(IIdentifiable<object>.Id)
424+
? DeObfuscateStringId(attribute.Type.ClrType, stringValue)
425+
: ConvertStringToType(stringValue, attribute.Property.PropertyType);
426+
}
427+
428+
private object DeObfuscateStringId(Type resourceClrType, string stringId)
413429
{
414430
IIdentifiable tempResource = _resourceFactory.CreateInstance(resourceClrType);
415431
tempResource.StringId = stringId;
416-
return tempResource.GetTypedId().ToString()!;
432+
return tempResource.GetTypedId();
417433
}
418434

419435
protected override IImmutableList<ResourceFieldAttribute> OnResolveFieldChain(string path, FieldChainRequirements chainRequirements)

‎src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,14 @@ private static FilterExpression GetInverseHasOneRelationshipFilter<TId>(TId prim
116116
AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
117117
var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(inverseRelationship, idAttribute));
118118

119-
return new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!.ToString()!));
119+
return new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!));
120120
}
121121

122122
private static FilterExpression GetInverseHasManyRelationshipFilter<TId>(TId primaryId, HasManyAttribute relationship, HasManyAttribute inverseRelationship)
123123
{
124124
AttrAttribute idAttribute = GetIdAttribute(relationship.LeftType);
125125
var idChain = new ResourceFieldChainExpression(ImmutableArray.Create<ResourceFieldAttribute>(idAttribute));
126-
var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!.ToString()!));
126+
var idComparison = new ComparisonExpression(ComparisonOperator.Equals, idChain, new LiteralConstantExpression(primaryId!));
127127

128128
return new HasExpression(new ResourceFieldChainExpression(inverseRelationship), idComparison);
129129
}
@@ -362,12 +362,12 @@ private IncludeExpression RewriteIncludeForSecondaryEndpoint(IncludeExpression?
362362

363363
if (ids.Count == 1)
364364
{
365-
var constant = new LiteralConstantExpression(ids.Single()!.ToString()!);
365+
var constant = new LiteralConstantExpression(ids.Single()!);
366366
filter = new ComparisonExpression(ComparisonOperator.Equals, idChain, constant);
367367
}
368368
else if (ids.Count > 1)
369369
{
370-
ImmutableHashSet<LiteralConstantExpression> constants = ids.Select(id => new LiteralConstantExpression(id!.ToString()!)).ToImmutableHashSet();
370+
ImmutableHashSet<LiteralConstantExpression> constants = ids.Select(id => new LiteralConstantExpression(id!)).ToImmutableHashSet();
371371
filter = new AnyExpression(idChain, constants);
372372
}
373373

0 commit comments

Comments
 (0)