Skip to content

Commit 7a8d3eb

Browse files
author
Bart Koelman
authored
Restructured eager loading tests and fixed two bugs (#882)
* Restructured eager loading tests and fixed two bugs: 1] During update, eager loads without parse fieldsets were not loaded at all places, which would make the resource change tracker incorrectly detect changes and return 200 instead of 204. 2] Creating a resource with a required EagerLoad (not exposed as relationship) was not possible, as there was no place for user code to create the related entity. Fixed this by making the create logic more similar to update logic. The change tracker scans through the initally created resource, invoking all property getters, enabling them to create related entities on the fly. See usage in Building.cs. * Updated test to set calculated property that depends on required relationship * Improved handling for non-loaded related entities. Yesterday I was trying to solve the wrong problem. The problem is not how to prevent a crash when creating a new resource that has a required EagerLoad relationship. Instead, the problem is that api developers need a way to assign relationships (EagerLoads or not) on resource creation. Whether such a relationship is required or not is irrelevant, it depends on business logic which relationships must be assigned. And that may be an already-existing entity or a new one. If the goal is to assign an already-existing entity, the api developer needs an async method to load that entity and assign it to the resource being created. This commit adds `IResourceWriteRepository.GetForCreateAsync`, which is analog to `GetForUpdateAsync`. This makes the resource creation logic in resource service very similar to update. This also fixes the tests, where [NotMapped] was missing on calculated properties. Except for request body deserialization, there is no more need to handle nulls in calculated properties that access a required related EagerLoad entity. And by treating the deserialized resource object just as an input source (which is never saved), we get by with just storing the deserialized data for later use, knowing it will be read and copied into the resource instance we're going to save. Also reordered repository members in a more logical order (used to be verb-based).
1 parent 369c4fd commit 7a8d3eb

27 files changed

+848
-445
lines changed

src/Examples/JsonApiDotNetCoreExample/Controllers/VisasController.cs

-18
This file was deleted.

src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs

-6
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ public sealed class AppDbContext : DbContext
1111

1212
public DbSet<TodoItem> TodoItems { get; set; }
1313
public DbSet<Passport> Passports { get; set; }
14-
public DbSet<Visa> Visas { get; set; }
1514
public DbSet<Person> People { get; set; }
1615
public DbSet<TodoItemCollection> TodoItemCollections { get; set; }
1716
public DbSet<KebabCasedModel> KebabCasedModels { get; set; }
@@ -69,11 +68,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder)
6968
.HasForeignKey<Person>("PassportKey")
7069
.OnDelete(DeleteBehavior.SetNull);
7170

72-
modelBuilder.Entity<Passport>()
73-
.HasMany(passport => passport.GrantedVisas)
74-
.WithOne()
75-
.OnDelete(DeleteBehavior.Cascade);
76-
7771
modelBuilder.Entity<TodoItem>()
7872
.HasOne(p => p.OneToOnePerson)
7973
.WithOne(p => p.OneToOneTodoItem)

src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs

-9
Original file line numberDiff line numberDiff line change
@@ -52,15 +52,6 @@ public string BirthCountryName
5252
[EagerLoad]
5353
public Country BirthCountry { get; set; }
5454

55-
[Attr(Capabilities = AttrCapabilities.All & ~(AttrCapabilities.AllowCreate | AttrCapabilities.AllowChange))]
56-
[NotMapped]
57-
public string GrantedVisaCountries => GrantedVisas == null || !GrantedVisas.Any()
58-
? null
59-
: string.Join(", ", GrantedVisas.Select(v => v.TargetCountry.Name));
60-
61-
[EagerLoad]
62-
public ICollection<Visa> GrantedVisas { get; set; }
63-
6455
public Passport(AppDbContext appDbContext)
6556
{
6657
_systemClock = appDbContext.SystemClock;

src/Examples/JsonApiDotNetCoreExample/Models/Visa.cs

-18
This file was deleted.

src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs

+25-18
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public class QueryLayerComposer : IQueryLayerComposer
2121
public QueryLayerComposer(
2222
IEnumerable<IQueryConstraintProvider> constraintProviders,
2323
IResourceContextProvider resourceContextProvider,
24-
IResourceDefinitionAccessor resourceDefinitionAccessor,
24+
IResourceDefinitionAccessor resourceDefinitionAccessor,
2525
IJsonApiOptions options,
2626
IPaginationContext paginationContext,
2727
ITargetedFields targetedFields)
@@ -93,7 +93,8 @@ private IncludeExpression ComposeChildren(QueryLayer topLayer, ICollection<Expre
9393
{
9494
var include = constraints
9595
.Where(constraint => constraint.Scope == null)
96-
.Select(constraint => constraint.Expression).OfType<IncludeExpression>()
96+
.Select(constraint => constraint.Expression)
97+
.OfType<IncludeExpression>()
9798
.FirstOrDefault() ?? IncludeExpression.Empty;
9899

99100
var includeElements =
@@ -104,7 +105,7 @@ private IncludeExpression ComposeChildren(QueryLayer topLayer, ICollection<Expre
104105
: include;
105106
}
106107

107-
private IReadOnlyCollection<IncludeElementExpression> ProcessIncludeSet(IReadOnlyCollection<IncludeElementExpression> includeElements,
108+
private IReadOnlyCollection<IncludeElementExpression> ProcessIncludeSet(IReadOnlyCollection<IncludeElementExpression> includeElements,
108109
QueryLayer parentLayer, ICollection<RelationshipAttribute> parentRelationshipChain, ICollection<ExpressionInScope> constraints)
109110
{
110111
includeElements = GetIncludeElements(includeElements, parentLayer.ResourceContext) ?? Array.Empty<IncludeElementExpression>();
@@ -135,7 +136,7 @@ private IReadOnlyCollection<IncludeElementExpression> ProcessIncludeSet(IReadOnl
135136
{
136137
Filter = GetFilter(expressionsInCurrentScope, resourceContext),
137138
Sort = GetSort(expressionsInCurrentScope, resourceContext),
138-
Pagination = ((JsonApiOptions) _options).DisableChildrenPagination
139+
Pagination = ((JsonApiOptions)_options).DisableChildrenPagination
139140
? null
140141
: GetPagination(expressionsInCurrentScope, resourceContext),
141142
Projection = GetSparseFieldSetProjection(expressionsInCurrentScope, resourceContext)
@@ -186,7 +187,10 @@ public QueryLayer ComposeForGetById<TId>(TId id, ResourceContext resourceContext
186187

187188
if (fieldSelection == TopFieldSelection.OnlyIdAttribute)
188189
{
189-
queryLayer.Projection = new Dictionary<ResourceFieldAttribute, QueryLayer> {{idAttribute, null}};
190+
queryLayer.Projection = new Dictionary<ResourceFieldAttribute, QueryLayer>
191+
{
192+
[idAttribute] = null
193+
};
190194
}
191195
else if (fieldSelection == TopFieldSelection.WithAllAttributes && queryLayer.Projection != null)
192196
{
@@ -234,9 +238,9 @@ public QueryLayer WrapLayerForSecondaryEndpoint<TId>(QueryLayer secondaryLayer,
234238
secondaryLayer.Include = null;
235239

236240
var primaryIdAttribute = GetIdAttribute(primaryResourceContext);
237-
var sparseFieldSet = new SparseFieldSetExpression(new[] { primaryIdAttribute });
241+
var sparseFieldSet = new SparseFieldSetExpression(new[] {primaryIdAttribute});
238242

239-
var primaryProjection = GetSparseFieldSetProjection(new[] { sparseFieldSet }, primaryResourceContext) ?? new Dictionary<ResourceFieldAttribute, QueryLayer>();
243+
var primaryProjection = GetSparseFieldSetProjection(new[] {sparseFieldSet}, primaryResourceContext) ?? new Dictionary<ResourceFieldAttribute, QueryLayer>();
240244
primaryProjection[secondaryRelationship] = secondaryLayer;
241245
primaryProjection[primaryIdAttribute] = null;
242246

@@ -276,10 +280,10 @@ private FilterExpression CreateFilterByIds<TId>(ICollection<TId> ids, AttrAttrib
276280
filter = new EqualsAnyOfExpression(idChain, constants);
277281
}
278282

279-
return filter == null
280-
? existingFilter
281-
: existingFilter == null
282-
? filter
283+
return filter == null
284+
? existingFilter
285+
: existingFilter == null
286+
? filter
283287
: new LogicalExpression(LogicalOperator.And, new[] {filter, existingFilter});
284288
}
285289

@@ -294,7 +298,7 @@ public QueryLayer ComposeForUpdate<TId>(TId id, ResourceContext primaryResource)
294298
var primaryIdAttribute = GetIdAttribute(primaryResource);
295299

296300
var primaryLayer = ComposeTopLayer(Array.Empty<ExpressionInScope>(), primaryResource);
297-
primaryLayer.Include = includeElements.Any() ? new IncludeExpression(includeElements) : null;
301+
primaryLayer.Include = includeElements.Any() ? new IncludeExpression(includeElements) : IncludeExpression.Empty;
298302
primaryLayer.Sort = null;
299303
primaryLayer.Pagination = null;
300304
primaryLayer.Filter = CreateFilterByIds(new[] {id}, primaryIdAttribute, primaryLayer.Filter);
@@ -332,6 +336,7 @@ public QueryLayer ComposeForGetRelationshipRightIds(RelationshipAttribute relati
332336

333337
return new QueryLayer(rightResourceContext)
334338
{
339+
Include = IncludeExpression.Empty,
335340
Filter = filter,
336341
Projection = new Dictionary<ResourceFieldAttribute, QueryLayer>
337342
{
@@ -386,10 +391,12 @@ protected virtual FilterExpression GetFilter(IReadOnlyCollection<QueryExpression
386391
if (resourceContext == null) throw new ArgumentNullException(nameof(resourceContext));
387392

388393
var filters = expressionsInScope.OfType<FilterExpression>().ToArray();
389-
var filter = filters.Length > 1 ? new LogicalExpression(LogicalOperator.And, filters) : filters.FirstOrDefault();
390394

391-
filter = _resourceDefinitionAccessor.OnApplyFilter(resourceContext.ResourceType, filter);
392-
return filter;
395+
var filter = filters.Length > 1
396+
? new LogicalExpression(LogicalOperator.And, filters)
397+
: filters.FirstOrDefault();
398+
399+
return _resourceDefinitionAccessor.OnApplyFilter(resourceContext.ResourceType, filter);
393400
}
394401

395402
protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> expressionsInScope, ResourceContext resourceContext)
@@ -398,7 +405,7 @@ protected virtual SortExpression GetSort(IReadOnlyCollection<QueryExpression> ex
398405
if (resourceContext == null) throw new ArgumentNullException(nameof(resourceContext));
399406

400407
var sort = expressionsInScope.OfType<SortExpression>().FirstOrDefault();
401-
408+
402409
sort = _resourceDefinitionAccessor.OnApplySort(resourceContext.ResourceType, sort);
403410

404411
if (sort == null)
@@ -416,7 +423,7 @@ protected virtual PaginationExpression GetPagination(IReadOnlyCollection<QueryEx
416423
if (resourceContext == null) throw new ArgumentNullException(nameof(resourceContext));
417424

418425
var pagination = expressionsInScope.OfType<PaginationExpression>().FirstOrDefault();
419-
426+
420427
pagination = _resourceDefinitionAccessor.OnApplyPagination(resourceContext.ResourceType, pagination);
421428

422429
pagination ??= new PaginationExpression(PageNumber.ValueOne, _options.DefaultPageSize);
@@ -444,7 +451,7 @@ protected virtual IDictionary<ResourceFieldAttribute, QueryLayer> GetSparseField
444451
var idAttribute = GetIdAttribute(resourceContext);
445452
attributes.Add(idAttribute);
446453

447-
return attributes.Cast<ResourceFieldAttribute>().ToDictionary(key => key, value => (QueryLayer) null);
454+
return attributes.Cast<ResourceFieldAttribute>().ToDictionary(key => key, value => (QueryLayer)null);
448455
}
449456

450457
private static AttrAttribute GetIdAttribute(ResourceContext resourceContext)

0 commit comments

Comments
 (0)