From 3a87ab2902d6d763786267ba5a234d7853fcec11 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 18 Nov 2020 15:52:50 +0100 Subject: [PATCH 1/3] 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. --- .../Controllers/VisasController.cs | 18 - .../Data/AppDbContext.cs | 6 - .../Models/Passport.cs | 9 - .../JsonApiDotNetCoreExample/Models/Visa.cs | 18 - .../Queries/Internal/QueryLayerComposer.cs | 43 ++- .../EntityFrameworkCoreRepository.cs | 18 +- .../IResourceRepositoryAccessor.cs | 2 +- .../Repositories/IResourceWriteRepository.cs | 2 +- .../ResourceRepositoryAccessor.cs | 4 +- .../Services/JsonApiResourceService.cs | 21 +- .../Acceptance/InjectableResourceTests.cs | 11 - .../Acceptance/Spec/EagerLoadTests.cs | 220 ------------ .../CompositeKeys/CompositeKeyTests.cs | 9 + .../IntegrationTests/EagerLoading/Building.cs | 39 ++ .../EagerLoading/BuildingsController.cs | 16 + .../IntegrationTests/EagerLoading/City.cs | 15 + .../IntegrationTests/EagerLoading/Door.cs | 8 + .../EagerLoading/EagerLoadingDbContext.cs | 31 ++ .../EagerLoading/EagerLoadingFakers.cs | 46 +++ .../EagerLoading/EagerLoadingTests.cs | 337 ++++++++++++++++++ .../IntegrationTests/EagerLoading/State.cs | 15 + .../EagerLoading/StatesController.cs | 16 + .../IntegrationTests/EagerLoading/Street.cs | 25 ++ .../EagerLoading/StreetsController.cs | 16 + .../IntegrationTests/EagerLoading/Window.cs | 9 + .../IServiceCollectionExtensionsTests.cs | 4 +- 26 files changed, 636 insertions(+), 322 deletions(-) delete mode 100644 src/Examples/JsonApiDotNetCoreExample/Controllers/VisasController.cs delete mode 100644 src/Examples/JsonApiDotNetCoreExample/Models/Visa.cs delete mode 100644 test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/EagerLoadTests.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingsController.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/City.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Door.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingFakers.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/State.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StatesController.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StreetsController.cs create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Window.cs diff --git a/src/Examples/JsonApiDotNetCoreExample/Controllers/VisasController.cs b/src/Examples/JsonApiDotNetCoreExample/Controllers/VisasController.cs deleted file mode 100644 index 1cb637160e..0000000000 --- a/src/Examples/JsonApiDotNetCoreExample/Controllers/VisasController.cs +++ /dev/null @@ -1,18 +0,0 @@ -using JsonApiDotNetCore.Configuration; -using JsonApiDotNetCore.Controllers; -using JsonApiDotNetCore.Services; -using JsonApiDotNetCoreExample.Models; -using Microsoft.Extensions.Logging; - -namespace JsonApiDotNetCoreExample.Controllers -{ - public sealed class VisasController : JsonApiController - { - public VisasController( - IJsonApiOptions options, - ILoggerFactory loggerFactory, - IResourceService resourceService) - : base(options, loggerFactory, resourceService) - { } - } -} diff --git a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs index c951e412e6..7221b18492 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Data/AppDbContext.cs @@ -11,7 +11,6 @@ public sealed class AppDbContext : DbContext public DbSet TodoItems { get; set; } public DbSet Passports { get; set; } - public DbSet Visas { get; set; } public DbSet People { get; set; } public DbSet TodoItemCollections { get; set; } public DbSet KebabCasedModels { get; set; } @@ -69,11 +68,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) .HasForeignKey("PassportKey") .OnDelete(DeleteBehavior.SetNull); - modelBuilder.Entity() - .HasMany(passport => passport.GrantedVisas) - .WithOne() - .OnDelete(DeleteBehavior.Cascade); - modelBuilder.Entity() .HasOne(p => p.OneToOnePerson) .WithOne(p => p.OneToOneTodoItem) diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs index 58eaeb5f9f..0ab164810c 100644 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs +++ b/src/Examples/JsonApiDotNetCoreExample/Models/Passport.cs @@ -52,15 +52,6 @@ public string BirthCountryName [EagerLoad] public Country BirthCountry { get; set; } - [Attr(Capabilities = AttrCapabilities.All & ~(AttrCapabilities.AllowCreate | AttrCapabilities.AllowChange))] - [NotMapped] - public string GrantedVisaCountries => GrantedVisas == null || !GrantedVisas.Any() - ? null - : string.Join(", ", GrantedVisas.Select(v => v.TargetCountry.Name)); - - [EagerLoad] - public ICollection GrantedVisas { get; set; } - public Passport(AppDbContext appDbContext) { _systemClock = appDbContext.SystemClock; diff --git a/src/Examples/JsonApiDotNetCoreExample/Models/Visa.cs b/src/Examples/JsonApiDotNetCoreExample/Models/Visa.cs deleted file mode 100644 index 781a391a8e..0000000000 --- a/src/Examples/JsonApiDotNetCoreExample/Models/Visa.cs +++ /dev/null @@ -1,18 +0,0 @@ -using System; -using JsonApiDotNetCore.Resources; -using JsonApiDotNetCore.Resources.Annotations; - -namespace JsonApiDotNetCoreExample.Models -{ - public sealed class Visa : Identifiable - { - [Attr] - public DateTime ExpiresAt { get; set; } - - [Attr] - public string CountryName => TargetCountry.Name; - - [EagerLoad] - public Country TargetCountry { get; set; } - } -} diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs index 4353fbd7e7..633fb27874 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryLayerComposer.cs @@ -21,7 +21,7 @@ public class QueryLayerComposer : IQueryLayerComposer public QueryLayerComposer( IEnumerable constraintProviders, IResourceContextProvider resourceContextProvider, - IResourceDefinitionAccessor resourceDefinitionAccessor, + IResourceDefinitionAccessor resourceDefinitionAccessor, IJsonApiOptions options, IPaginationContext paginationContext, ITargetedFields targetedFields) @@ -93,7 +93,8 @@ private IncludeExpression ComposeChildren(QueryLayer topLayer, ICollection constraint.Scope == null) - .Select(constraint => constraint.Expression).OfType() + .Select(constraint => constraint.Expression) + .OfType() .FirstOrDefault() ?? IncludeExpression.Empty; var includeElements = @@ -104,7 +105,7 @@ private IncludeExpression ComposeChildren(QueryLayer topLayer, ICollection ProcessIncludeSet(IReadOnlyCollection includeElements, + private IReadOnlyCollection ProcessIncludeSet(IReadOnlyCollection includeElements, QueryLayer parentLayer, ICollection parentRelationshipChain, ICollection constraints) { includeElements = GetIncludeElements(includeElements, parentLayer.ResourceContext) ?? Array.Empty(); @@ -135,7 +136,7 @@ private IReadOnlyCollection ProcessIncludeSet(IReadOnl { Filter = GetFilter(expressionsInCurrentScope, resourceContext), Sort = GetSort(expressionsInCurrentScope, resourceContext), - Pagination = ((JsonApiOptions) _options).DisableChildrenPagination + Pagination = ((JsonApiOptions)_options).DisableChildrenPagination ? null : GetPagination(expressionsInCurrentScope, resourceContext), Projection = GetSparseFieldSetProjection(expressionsInCurrentScope, resourceContext) @@ -186,7 +187,10 @@ public QueryLayer ComposeForGetById(TId id, ResourceContext resourceContext if (fieldSelection == TopFieldSelection.OnlyIdAttribute) { - queryLayer.Projection = new Dictionary {{idAttribute, null}}; + queryLayer.Projection = new Dictionary + { + [idAttribute] = null + }; } else if (fieldSelection == TopFieldSelection.WithAllAttributes && queryLayer.Projection != null) { @@ -234,9 +238,9 @@ public QueryLayer WrapLayerForSecondaryEndpoint(QueryLayer secondaryLayer, secondaryLayer.Include = null; var primaryIdAttribute = GetIdAttribute(primaryResourceContext); - var sparseFieldSet = new SparseFieldSetExpression(new[] { primaryIdAttribute }); + var sparseFieldSet = new SparseFieldSetExpression(new[] {primaryIdAttribute}); - var primaryProjection = GetSparseFieldSetProjection(new[] { sparseFieldSet }, primaryResourceContext) ?? new Dictionary(); + var primaryProjection = GetSparseFieldSetProjection(new[] {sparseFieldSet}, primaryResourceContext) ?? new Dictionary(); primaryProjection[secondaryRelationship] = secondaryLayer; primaryProjection[primaryIdAttribute] = null; @@ -276,10 +280,10 @@ private FilterExpression CreateFilterByIds(ICollection ids, AttrAttrib filter = new EqualsAnyOfExpression(idChain, constants); } - return filter == null - ? existingFilter - : existingFilter == null - ? filter + return filter == null + ? existingFilter + : existingFilter == null + ? filter : new LogicalExpression(LogicalOperator.And, new[] {filter, existingFilter}); } @@ -294,7 +298,7 @@ public QueryLayer ComposeForUpdate(TId id, ResourceContext primaryResource) var primaryIdAttribute = GetIdAttribute(primaryResource); var primaryLayer = ComposeTopLayer(Array.Empty(), primaryResource); - primaryLayer.Include = includeElements.Any() ? new IncludeExpression(includeElements) : null; + primaryLayer.Include = includeElements.Any() ? new IncludeExpression(includeElements) : IncludeExpression.Empty; primaryLayer.Sort = null; primaryLayer.Pagination = null; primaryLayer.Filter = CreateFilterByIds(new[] {id}, primaryIdAttribute, primaryLayer.Filter); @@ -332,6 +336,7 @@ public QueryLayer ComposeForGetRelationshipRightIds(RelationshipAttribute relati return new QueryLayer(rightResourceContext) { + Include = IncludeExpression.Empty, Filter = filter, Projection = new Dictionary { @@ -386,10 +391,12 @@ protected virtual FilterExpression GetFilter(IReadOnlyCollection().ToArray(); - var filter = filters.Length > 1 ? new LogicalExpression(LogicalOperator.And, filters) : filters.FirstOrDefault(); - filter = _resourceDefinitionAccessor.OnApplyFilter(resourceContext.ResourceType, filter); - return filter; + var filter = filters.Length > 1 + ? new LogicalExpression(LogicalOperator.And, filters) + : filters.FirstOrDefault(); + + return _resourceDefinitionAccessor.OnApplyFilter(resourceContext.ResourceType, filter); } protected virtual SortExpression GetSort(IReadOnlyCollection expressionsInScope, ResourceContext resourceContext) @@ -398,7 +405,7 @@ protected virtual SortExpression GetSort(IReadOnlyCollection ex if (resourceContext == null) throw new ArgumentNullException(nameof(resourceContext)); var sort = expressionsInScope.OfType().FirstOrDefault(); - + sort = _resourceDefinitionAccessor.OnApplySort(resourceContext.ResourceType, sort); if (sort == null) @@ -416,7 +423,7 @@ protected virtual PaginationExpression GetPagination(IReadOnlyCollection().FirstOrDefault(); - + pagination = _resourceDefinitionAccessor.OnApplyPagination(resourceContext.ResourceType, pagination); pagination ??= new PaginationExpression(PageNumber.ValueOne, _options.DefaultPageSize); @@ -444,7 +451,7 @@ protected virtual IDictionary GetSparseField var idAttribute = GetIdAttribute(resourceContext); attributes.Add(idAttribute); - return attributes.Cast().ToDictionary(key => key, value => (QueryLayer) null); + return attributes.Cast().ToDictionary(key => key, value => (QueryLayer)null); } private static AttrAttribute GetIdAttribute(ResourceContext resourceContext) diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index f2c63998be..e05827242b 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -115,21 +115,27 @@ protected virtual IQueryable GetAll() } /// - public virtual async Task CreateAsync(TResource resource, CancellationToken cancellationToken) + public virtual async Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken) { - _traceWriter.LogMethodStart(new {resource}); - if (resource == null) throw new ArgumentNullException(nameof(resource)); + _traceWriter.LogMethodStart(new {resourceFromRequest, resourceForDatabase}); + if (resourceFromRequest == null) throw new ArgumentNullException(nameof(resourceFromRequest)); + if (resourceForDatabase == null) throw new ArgumentNullException(nameof(resourceForDatabase)); using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); foreach (var relationship in _targetedFields.Relationships) { - var rightValue = relationship.GetValue(resource); - await UpdateRelationshipAsync(relationship, resource, rightValue, collector, cancellationToken); + var rightResources = relationship.GetValue(resourceFromRequest); + await UpdateRelationshipAsync(relationship, resourceForDatabase, rightResources, collector, cancellationToken); + } + + foreach (var attribute in _targetedFields.Attributes) + { + attribute.SetValue(resourceForDatabase, attribute.GetValue(resourceFromRequest)); } var dbSet = _dbContext.Set(); - dbSet.Add(resource); + dbSet.Add(resourceForDatabase); await SaveChangesAsync(cancellationToken); } diff --git a/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs index ca37d9dcd0..3eea039525 100644 --- a/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs +++ b/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs @@ -33,7 +33,7 @@ Task CountAsync(FilterExpression topFilter, CancellationToken ca /// /// Invokes . /// - Task CreateAsync(TResource resource, CancellationToken cancellationToken) + Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken) where TResource : class, IIdentifiable; /// diff --git a/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs b/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs index 929631d3b7..b46e422f6b 100644 --- a/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs @@ -23,7 +23,7 @@ public interface IResourceWriteRepository /// /// Creates a new resource in the underlying data store. /// - Task CreateAsync(TResource resource, CancellationToken cancellationToken); + Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken); /// /// Adds resources to a to-many relationship in the underlying data store. diff --git a/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs index a4cb63d1a9..607773f131 100644 --- a/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs +++ b/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs @@ -48,11 +48,11 @@ public async Task CountAsync(FilterExpression topFilter, Cancell } /// - public async Task CreateAsync(TResource resource, CancellationToken cancellationToken) + public async Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken) where TResource : class, IIdentifiable { dynamic repository = GetWriteRepository(typeof(TResource)); - await repository.CreateAsync(resource, cancellationToken); + await repository.CreateAsync(resourceFromRequest, resourceForDatabase, cancellationToken); } /// diff --git a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs index c880c147f7..7e600d2c4e 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs @@ -171,32 +171,33 @@ public virtual async Task CreateAsync(TResource resource, Cancellatio _traceWriter.LogMethodStart(new {resource}); if (resource == null) throw new ArgumentNullException(nameof(resource)); - _resourceChangeTracker.SetRequestedAttributeValues(resource); + var resourceFromRequest = resource; + _resourceChangeTracker.SetRequestedAttributeValues(resourceFromRequest); - var defaultResource = _resourceFactory.CreateInstance(); - defaultResource.Id = resource.Id; + _hookExecutor.BeforeCreate(resourceFromRequest); - _resourceChangeTracker.SetInitiallyStoredAttributeValues(defaultResource); + var resourceForDatabase = _resourceFactory.CreateInstance(); + resourceForDatabase.Id = resourceFromRequest.Id; - _hookExecutor.BeforeCreate(resource); + _resourceChangeTracker.SetInitiallyStoredAttributeValues(resourceForDatabase); try { - await _repositoryAccessor.CreateAsync(resource, cancellationToken); + await _repositoryAccessor.CreateAsync(resourceFromRequest, resourceForDatabase, cancellationToken); } catch (DataStoreUpdateException) { - var existingResource = await TryGetPrimaryResourceByIdAsync(resource.Id, TopFieldSelection.OnlyIdAttribute, cancellationToken); + var existingResource = await TryGetPrimaryResourceByIdAsync(resourceFromRequest.Id, TopFieldSelection.OnlyIdAttribute, cancellationToken); if (existingResource != null) { - throw new ResourceAlreadyExistsException(resource.StringId, _request.PrimaryResource.PublicName); + throw new ResourceAlreadyExistsException(resourceFromRequest.StringId, _request.PrimaryResource.PublicName); } - await AssertResourcesToAssignInRelationshipsExistAsync(resource, cancellationToken); + await AssertResourcesToAssignInRelationshipsExistAsync(resourceFromRequest, cancellationToken); throw; } - var resourceFromDatabase = await TryGetPrimaryResourceByIdAsync(resource.Id, TopFieldSelection.WithAllAttributes, cancellationToken); + var resourceFromDatabase = await TryGetPrimaryResourceByIdAsync(resourceForDatabase.Id, TopFieldSelection.WithAllAttributes, cancellationToken); AssertPrimaryResourceExists(resourceFromDatabase); _hookExecutor.AfterCreate(resourceFromDatabase); diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/InjectableResourceTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/InjectableResourceTests.cs index 8ab3b67a0c..ce8d259d5f 100644 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/InjectableResourceTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/Acceptance/InjectableResourceTests.cs @@ -19,20 +19,14 @@ public class InjectableResourceTests private readonly TestFixture _fixture; private readonly AppDbContext _context; private readonly Faker _personFaker; - private readonly Faker _todoItemFaker; private readonly Faker _passportFaker; private readonly Faker _countryFaker; - private readonly Faker _visaFaker; public InjectableResourceTests(TestFixture fixture) { _fixture = fixture; _context = fixture.GetRequiredService(); - _todoItemFaker = new Faker() - .RuleFor(t => t.Description, f => f.Lorem.Sentence()) - .RuleFor(t => t.Ordinal, f => f.Random.Number()) - .RuleFor(t => t.CreatedDate, f => f.Date.Past()); _personFaker = new Faker() .RuleFor(t => t.FirstName, f => f.Name.FirstName()) .RuleFor(t => t.LastName, f => f.Name.LastName()); @@ -41,8 +35,6 @@ public InjectableResourceTests(TestFixture fixture) .RuleFor(t => t.SocialSecurityNumber, f => f.Random.Number(100, 10_000)); _countryFaker = new Faker() .RuleFor(c => c.Name, f => f.Address.Country()); - _visaFaker = new Faker() - .RuleFor(v => v.ExpiresAt, f => f.Date.Future()); } [Fact] @@ -75,7 +67,6 @@ public async Task Can_Get_Passports() { // Arrange await _context.ClearTableAsync(); - await _context.SaveChangesAsync(); var passports = _passportFaker.Generate(3); foreach (var passport in passports) @@ -112,7 +103,6 @@ public async Task Can_Get_Passports_With_Filter() { // Arrange await _context.ClearTableAsync(); - await _context.SaveChangesAsync(); var passports = _passportFaker.Generate(3); foreach (var passport in passports) @@ -152,7 +142,6 @@ public async Task Can_Get_Passports_With_Sparse_Fieldset() { // Arrange await _context.ClearTableAsync(); - await _context.SaveChangesAsync(); var passports = _passportFaker.Generate(2); foreach (var passport in passports) diff --git a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/EagerLoadTests.cs b/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/EagerLoadTests.cs deleted file mode 100644 index cc6a9532b6..0000000000 --- a/test/JsonApiDotNetCoreExampleTests/Acceptance/Spec/EagerLoadTests.cs +++ /dev/null @@ -1,220 +0,0 @@ -using System.Collections.Generic; -using System.Linq; -using System.Net; -using System.Threading.Tasks; -using Bogus; -using JsonApiDotNetCore.Serialization.Objects; -using JsonApiDotNetCoreExample.Data; -using JsonApiDotNetCoreExample.Models; -using Microsoft.Extensions.DependencyInjection; -using Newtonsoft.Json; -using Xunit; -using Person = JsonApiDotNetCoreExample.Models.Person; - -namespace JsonApiDotNetCoreExampleTests.Acceptance.Spec -{ - public class EagerLoadTests : FunctionalTestCollection - { - private readonly Faker _personFaker; - private readonly Faker _passportFaker; - private readonly Faker _countryFaker; - private readonly Faker _todoItemFaker; - private readonly Faker _visaFaker; - - public EagerLoadTests(StandardApplicationFactory factory) : base(factory) - { - var appDbContext = factory.ServiceProvider.GetRequiredService(); - - _todoItemFaker = new Faker() - .RuleFor(t => t.Description, f => f.Lorem.Sentence()) - .RuleFor(t => t.Ordinal, f => f.Random.Number()) - .RuleFor(t => t.CreatedDate, f => f.Date.Past()); - _personFaker = new Faker() - .RuleFor(t => t.FirstName, f => f.Name.FirstName()) - .RuleFor(t => t.LastName, f => f.Name.LastName()); - _passportFaker = new Faker() - .CustomInstantiator(f => new Passport(appDbContext)) - .RuleFor(t => t.SocialSecurityNumber, f => f.Random.Number(100, 10_000)); - _countryFaker = new Faker() - .RuleFor(c => c.Name, f => f.Address.Country()); - _visaFaker = new Faker() - .RuleFor(v => v.ExpiresAt, f => f.Date.Future()); - } - - [Fact] - public async Task GetSingleResource_TopLevel_AppliesEagerLoad() - { - // Arrange - var passport = _passportFaker.Generate(); - passport.BirthCountry = _countryFaker.Generate(); - - var visa1 = _visaFaker.Generate(); - visa1.TargetCountry = _countryFaker.Generate(); - - var visa2 = _visaFaker.Generate(); - visa2.TargetCountry = _countryFaker.Generate(); - - passport.GrantedVisas = new List { visa1, visa2 }; - - _dbContext.Add(passport); - await _dbContext.SaveChangesAsync(); - - // Act - var (body, response) = await Get($"/api/v1/passports/{passport.StringId}"); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotNull(document.SingleData); - Assert.Equal(passport.StringId, document.SingleData.Id); - Assert.Equal(passport.BirthCountry.Name, document.SingleData.Attributes["birthCountryName"]); - Assert.Equal(visa1.TargetCountry.Name + ", " + visa2.TargetCountry.Name, document.SingleData.Attributes["grantedVisaCountries"]); - } - - [Fact] - public async Task GetSingleResource_TopLevel_with_SparseFieldSet_AppliesEagerLoad() - { - // Arrange - var visa = _visaFaker.Generate(); - visa.TargetCountry = _countryFaker.Generate(); - - _dbContext.Visas.Add(visa); - await _dbContext.SaveChangesAsync(); - - // Act - var (body, response) = await Get($"/api/v1/visas/{visa.StringId}?fields=expiresAt,countryName"); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotNull(document.SingleData); - Assert.Equal(visa.StringId, document.SingleData.Id); - Assert.Equal(visa.TargetCountry.Name, document.SingleData.Attributes["countryName"]); - } - - [Fact] - public async Task GetMultiResource_Secondary_AppliesEagerLoad() - { - // Arrange - var person = _personFaker.Generate(); - person.Passport = _passportFaker.Generate(); - person.Passport.BirthCountry = _countryFaker.Generate(); - - var visa = _visaFaker.Generate(); - visa.TargetCountry = _countryFaker.Generate(); - person.Passport.GrantedVisas = new List {visa}; - - await _dbContext.ClearTableAsync(); - _dbContext.Add(person); - await _dbContext.SaveChangesAsync(); - - // Act - var (body, response) = await Get("/api/v1/people?include=passport"); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotEmpty(document.ManyData); - Assert.Equal(person.StringId, document.ManyData[0].Id); - - Assert.NotEmpty(document.Included); - Assert.Equal(person.Passport.StringId, document.Included[0].Id); - Assert.Equal(person.Passport.BirthCountry.Name, document.Included[0].Attributes["birthCountryName"]); - Assert.Equal(person.Passport.GrantedVisaCountries, document.Included[0].Attributes["grantedVisaCountries"]); - } - - [Fact] - public async Task GetMultiResource_DeeplyNested_AppliesEagerLoad() - { - // Arrange - var todo = _todoItemFaker.Generate(); - todo.Assignee = _personFaker.Generate(); - todo.Owner = _personFaker.Generate();; - todo.Owner.Passport = _passportFaker.Generate(); - todo.Owner.Passport.BirthCountry = _countryFaker.Generate(); - - var visa = _visaFaker.Generate(); - visa.TargetCountry = _countryFaker.Generate(); - todo.Owner.Passport.GrantedVisas = new List {visa}; - - _dbContext.Add(todo); - await _dbContext.SaveChangesAsync(); - - // Act - var (body, response) = await Get($"/api/v1/people/{todo.Assignee.Id}/assignedTodoItems?include=owner.passport"); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotEmpty(document.ManyData); - Assert.Equal(todo.StringId, document.ManyData[0].Id); - - Assert.Equal(2, document.Included.Count); - Assert.Equal(todo.Owner.StringId, document.Included[0].Id); - Assert.Equal(todo.Owner.Passport.StringId, document.Included[1].Id); - Assert.Equal(todo.Owner.Passport.BirthCountry.Name, document.Included[1].Attributes["birthCountryName"]); - Assert.Equal(todo.Owner.Passport.GrantedVisaCountries, document.Included[1].Attributes["grantedVisaCountries"]); - } - - [Fact] - public async Task PostSingleResource_TopLevel_AppliesEagerLoad() - { - // Arrange - var passport = _passportFaker.Generate(); - passport.BirthCountry = _countryFaker.Generate(); - - var serializer = GetSerializer(p => new { p.SocialSecurityNumber, p.BirthCountryName }); - var content = serializer.Serialize(passport); - - // Act - var (body, response) = await Post("/api/v1/passports", content); - - // Assert - AssertEqualStatusCode(HttpStatusCode.Created, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotNull(document.SingleData); - Assert.Equal((long?)passport.SocialSecurityNumber, document.SingleData.Attributes["socialSecurityNumber"]); - Assert.Equal(passport.BirthCountry.Name, document.SingleData.Attributes["birthCountryName"]); - Assert.Null(document.SingleData.Attributes["grantedVisaCountries"]); - } - - [Fact] - public async Task PatchResource_TopLevel_AppliesEagerLoad() - { - // Arrange - var passport = _passportFaker.Generate(); - passport.BirthCountry = _countryFaker.Generate(); - - var visa = _visaFaker.Generate(); - visa.TargetCountry = _countryFaker.Generate(); - passport.GrantedVisas = new List { visa }; - - _dbContext.Add(passport); - await _dbContext.SaveChangesAsync(); - - passport.SocialSecurityNumber = _passportFaker.Generate().SocialSecurityNumber; - passport.BirthCountry.Name = _countryFaker.Generate().Name; - - var serializer = GetSerializer(p => new { p.SocialSecurityNumber, p.BirthCountryName }); - var content = serializer.Serialize(passport); - - // Act - var (body, response) = await Patch($"/api/v1/passports/{passport.StringId}", content); - - // Assert - AssertEqualStatusCode(HttpStatusCode.OK, response); - - var document = JsonConvert.DeserializeObject(body); - Assert.NotNull(document.SingleData); - Assert.Equal(passport.StringId, document.SingleData.Id); - Assert.Equal((long?)passport.SocialSecurityNumber, document.SingleData.Attributes["socialSecurityNumber"]); - Assert.Equal(passport.BirthCountry.Name, document.SingleData.Attributes["birthCountryName"]); - Assert.Equal(passport.GrantedVisas.First().TargetCountry.Name, document.SingleData.Attributes["grantedVisaCountries"]); - } - } -} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs index 9f20310613..8cded61238 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/CompositeKeys/CompositeKeyTests.cs @@ -176,6 +176,15 @@ await _testContext.RunOnDatabaseAsync(async dbContext => httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); responseDocument.Should().BeEmpty(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + var carInDatabase = await dbContext.Cars + .FirstOrDefaultAsync(car => car.RegionId == 123 && car.LicensePlate == "AA-BB-11"); + + carInDatabase.Should().NotBeNull(); + carInDatabase.Id.Should().Be("123:AA-BB-11"); + }); } [Fact] diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs new file mode 100644 index 0000000000..61913ec152 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs @@ -0,0 +1,39 @@ +using System.Collections.Generic; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class Building : Identifiable + { + [Attr] + public string Number { get; set; } + + [Attr] + public int WindowCount => Windows?.Count ?? 0; + + [Attr] + public string PrimaryDoorColor + { + get + { + // Must ensure that an instance exists for this required relationship, so that POST succeeds. + PrimaryDoor ??= new Door(); + + return PrimaryDoor.Color; + } + } + + [Attr] + public string SecondaryDoorColor => SecondaryDoor?.Color; + + [EagerLoad] + public IList Windows { get; set; } + + [EagerLoad] + public Door PrimaryDoor { get; set; } + + [EagerLoad] + public Door SecondaryDoor { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingsController.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingsController.cs new file mode 100644 index 0000000000..4a0b9bb366 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingsController.cs @@ -0,0 +1,16 @@ +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class BuildingsController : JsonApiController + { + public BuildingsController(IJsonApiOptions options, ILoggerFactory loggerFactory, + IResourceService resourceService) + : base(options, loggerFactory, resourceService) + { + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/City.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/City.cs new file mode 100644 index 0000000000..2fbff2cf67 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/City.cs @@ -0,0 +1,15 @@ +using System.Collections.Generic; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class City : Identifiable + { + [Attr] + public string Name { get; set; } + + [HasMany] + public IList Streets { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Door.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Door.cs new file mode 100644 index 0000000000..919d0a1907 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Door.cs @@ -0,0 +1,8 @@ +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class Door + { + public int Id { get; set; } + public string Color { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs new file mode 100644 index 0000000000..fb60ddb51e --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingDbContext.cs @@ -0,0 +1,31 @@ +using Microsoft.EntityFrameworkCore; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class EagerLoadingDbContext : DbContext + { + public DbSet States { get; set; } + public DbSet Streets { get; set; } + public DbSet Buildings { get; set; } + + public EagerLoadingDbContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder builder) + { + builder.Entity() + .HasOne(building => building.PrimaryDoor) + .WithOne() + .HasForeignKey("PrimaryDoorKey") + .IsRequired(); + + builder.Entity() + .HasOne(building => building.SecondaryDoor) + .WithOne() + .HasForeignKey("SecondaryDoorKey") + .IsRequired(false); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingFakers.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingFakers.cs new file mode 100644 index 0000000000..6ca3157f55 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingFakers.cs @@ -0,0 +1,46 @@ +using System; +using Bogus; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + internal sealed class EagerLoadingFakers : FakerContainer + { + private readonly Lazy> _lazyStateFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(state => state.Name, f => f.Address.City())); + + private readonly Lazy> _lazyCityFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(city => city.Name, f => f.Address.City())); + + private readonly Lazy> _lazyStreetFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(street => street.Name, f => f.Address.StreetName())); + + private readonly Lazy> _lazyBuildingFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(building => building.Number, f => f.Address.BuildingNumber())); + + private readonly Lazy> _lazyWindowFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(window => window.HeightInCentimeters, f => f.Random.Number(30, 199)) + .RuleFor(window => window.WidthInCentimeters, f => f.Random.Number(30, 199))); + + private readonly Lazy> _lazyDoorFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(door => door.Color, f => f.Commerce.Color())); + + public Faker State => _lazyStateFaker.Value; + public Faker City => _lazyCityFaker.Value; + public Faker Street => _lazyStreetFaker.Value; + public Faker Building => _lazyBuildingFaker.Value; + public Faker Window => _lazyWindowFaker.Value; + public Faker Door => _lazyDoorFaker.Value; + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs new file mode 100644 index 0000000000..daf2c15eab --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs @@ -0,0 +1,337 @@ +using System.Collections.Generic; +using System.Net; +using System.Threading.Tasks; +using FluentAssertions; +using JsonApiDotNetCore.Serialization.Objects; +using Microsoft.EntityFrameworkCore; +using Xunit; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class EagerLoadingTests + : IClassFixture, EagerLoadingDbContext>> + { + private readonly IntegrationTestContext, EagerLoadingDbContext> _testContext; + private readonly EagerLoadingFakers _fakers = new EagerLoadingFakers(); + + public EagerLoadingTests(IntegrationTestContext, EagerLoadingDbContext> testContext) + { + _testContext = testContext; + } + + [Fact] + public async Task Can_get_primary_resource_with_eager_loads() + { + // Arrange + var building = _fakers.Building.Generate(); + building.Windows = _fakers.Window.Generate(4); + building.PrimaryDoor = _fakers.Door.Generate(); + building.SecondaryDoor = _fakers.Door.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Buildings.Add(building); + await dbContext.SaveChangesAsync(); + }); + + var route = "/buildings/" + building.StringId; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(building.StringId); + responseDocument.SingleData.Attributes["number"].Should().Be(building.Number); + responseDocument.SingleData.Attributes["windowCount"].Should().Be(4); + responseDocument.SingleData.Attributes["primaryDoorColor"].Should().Be(building.PrimaryDoor.Color); + responseDocument.SingleData.Attributes["secondaryDoorColor"].Should().Be(building.SecondaryDoor.Color); + } + + [Fact] + public async Task Can_get_primary_resource_with_nested_eager_loads() + { + // Arrange + var street = _fakers.Street.Generate(); + street.Buildings = _fakers.Building.Generate(2); + + street.Buildings[0].Windows = _fakers.Window.Generate(2); + street.Buildings[0].PrimaryDoor = _fakers.Door.Generate(); + + street.Buildings[1].Windows = _fakers.Window.Generate(3); + street.Buildings[1].PrimaryDoor = _fakers.Door.Generate(); + street.Buildings[1].SecondaryDoor = _fakers.Door.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Streets.Add(street); + await dbContext.SaveChangesAsync(); + }); + + var route = "/streets/" + street.StringId; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(street.StringId); + responseDocument.SingleData.Attributes["name"].Should().Be(street.Name); + responseDocument.SingleData.Attributes["buildingCount"].Should().Be(2); + responseDocument.SingleData.Attributes["doorTotalCount"].Should().Be(3); + responseDocument.SingleData.Attributes["windowTotalCount"].Should().Be(5); + } + + [Fact] + public async Task Can_get_primary_resource_with_fieldset() + { + // Arrange + var street = _fakers.Street.Generate(); + street.Buildings = _fakers.Building.Generate(1); + street.Buildings[0].Windows = _fakers.Window.Generate(3); + street.Buildings[0].PrimaryDoor = _fakers.Door.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Streets.Add(street); + await dbContext.SaveChangesAsync(); + }); + + var route = $"/streets/{street.StringId}?fields=windowTotalCount"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(street.StringId); + responseDocument.SingleData.Attributes.Should().HaveCount(1); + responseDocument.SingleData.Attributes["windowTotalCount"].Should().Be(3); + } + + [Fact] + public async Task Can_get_primary_resource_with_includes() + { + // Arrange + var state = _fakers.State.Generate(); + state.Cities = _fakers.City.Generate(1); + state.Cities[0].Streets = _fakers.Street.Generate(1); + state.Cities[0].Streets[0].Buildings = _fakers.Building.Generate(1); + state.Cities[0].Streets[0].Buildings[0].PrimaryDoor = _fakers.Door.Generate(); + state.Cities[0].Streets[0].Buildings[0].Windows = _fakers.Window.Generate(3); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.States.Add(state); + await dbContext.SaveChangesAsync(); + }); + + var route = $"/states/{state.StringId}?include=cities.streets"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(state.StringId); + responseDocument.SingleData.Attributes["name"].Should().Be(state.Name); + + responseDocument.Included.Should().HaveCount(2); + + responseDocument.Included[0].Type.Should().Be("cities"); + responseDocument.Included[0].Id.Should().Be(state.Cities[0].StringId); + responseDocument.Included[0].Attributes["name"].Should().Be(state.Cities[0].Name); + + responseDocument.Included[1].Type.Should().Be("streets"); + responseDocument.Included[1].Id.Should().Be(state.Cities[0].Streets[0].StringId); + responseDocument.Included[1].Attributes["buildingCount"].Should().Be(1); + responseDocument.Included[1].Attributes["doorTotalCount"].Should().Be(1); + responseDocument.Included[1].Attributes["windowTotalCount"].Should().Be(3); + } + + [Fact] + public async Task Can_get_secondary_resources_with_include_and_fieldsets() + { + // Arrange + var state = _fakers.State.Generate(); + state.Cities = _fakers.City.Generate(1); + state.Cities[0].Streets = _fakers.Street.Generate(1); + state.Cities[0].Streets[0].Buildings = _fakers.Building.Generate(1); + state.Cities[0].Streets[0].Buildings[0].PrimaryDoor = _fakers.Door.Generate(); + state.Cities[0].Streets[0].Buildings[0].SecondaryDoor = _fakers.Door.Generate(); + state.Cities[0].Streets[0].Buildings[0].Windows = _fakers.Window.Generate(1); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.States.Add(state); + await dbContext.SaveChangesAsync(); + }); + + var route = $"/states/{state.StringId}/cities?include=streets&fields=name&fields[streets]=doorTotalCount,windowTotalCount"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(1); + responseDocument.ManyData[0].Id.Should().Be(state.Cities[0].StringId); + responseDocument.ManyData[0].Attributes.Should().HaveCount(1); + responseDocument.ManyData[0].Attributes["name"].Should().Be(state.Cities[0].Name); + + responseDocument.Included.Should().HaveCount(1); + responseDocument.Included[0].Type.Should().Be("streets"); + responseDocument.Included[0].Id.Should().Be(state.Cities[0].Streets[0].StringId); + responseDocument.Included[0].Attributes.Should().HaveCount(2); + responseDocument.Included[0].Attributes["doorTotalCount"].Should().Be(2); + responseDocument.Included[0].Attributes["windowTotalCount"].Should().Be(1); + } + + [Fact] + public async Task Can_create_resource() + { + // Arrange + var newBuilding = _fakers.Building.Generate(); + + var requestBody = new + { + data = new + { + type = "buildings", + attributes = new + { + number = newBuilding.Number + } + } + }; + + var route = "/buildings"; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecutePostAsync(route, requestBody); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.Created); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Attributes["number"].Should().Be(newBuilding.Number); + responseDocument.SingleData.Attributes["windowCount"].Should().Be(0); + responseDocument.SingleData.Attributes["primaryDoorColor"].Should().BeNull(); + responseDocument.SingleData.Attributes["secondaryDoorColor"].Should().BeNull(); + + var newId = int.Parse(responseDocument.SingleData.Id); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + var buildingInDatabase = await dbContext.Buildings + .Include(building => building.PrimaryDoor) + .Include(building => building.SecondaryDoor) + .Include(building => building.Windows) + .FirstOrDefaultAsync(building => building.Id == newId); + + buildingInDatabase.Should().NotBeNull(); + buildingInDatabase.Number.Should().Be(newBuilding.Number); + buildingInDatabase.PrimaryDoor.Should().NotBeNull(); + buildingInDatabase.SecondaryDoor.Should().BeNull(); + buildingInDatabase.Windows.Should().BeEmpty(); + }); + } + + [Fact] + public async Task Can_update_resource() + { + // Arrange + var existingBuilding = _fakers.Building.Generate(); + existingBuilding.PrimaryDoor = _fakers.Door.Generate(); + existingBuilding.SecondaryDoor = _fakers.Door.Generate(); + existingBuilding.Windows = _fakers.Window.Generate(2); + + var newBuildingNumber = _fakers.Building.Generate().Number; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Buildings.Add(existingBuilding); + await dbContext.SaveChangesAsync(); + }); + + var requestBody = new + { + data = new + { + type = "buildings", + id = existingBuilding.StringId, + attributes = new + { + number = newBuildingNumber + } + } + }; + + var route = "/buildings/" + existingBuilding.StringId; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecutePatchAsync(route, requestBody); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); + + responseDocument.Should().BeEmpty(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + var buildingInDatabase = await dbContext.Buildings + .Include(building => building.PrimaryDoor) + .Include(building => building.SecondaryDoor) + .Include(building => building.Windows) + .FirstOrDefaultAsync(building => building.Id == existingBuilding.Id); + + buildingInDatabase.Should().NotBeNull(); + buildingInDatabase.Number.Should().Be(newBuildingNumber); + buildingInDatabase.PrimaryDoor.Should().NotBeNull(); + buildingInDatabase.SecondaryDoor.Should().NotBeNull(); + buildingInDatabase.Windows.Should().HaveCount(2); + }); + } + + [Fact] + public async Task Can_delete_resource() + { + // Arrange + var existingBuilding = _fakers.Building.Generate(); + existingBuilding.PrimaryDoor = _fakers.Door.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Buildings.Add(existingBuilding); + await dbContext.SaveChangesAsync(); + }); + + var route = "/buildings/" + existingBuilding.StringId; + + // Act + var (httpResponse, responseDocument) = await _testContext.ExecuteDeleteAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.NoContent); + + responseDocument.Should().BeEmpty(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + var buildingInDatabase = await dbContext.Buildings + .FirstOrDefaultAsync(building => building.Id == existingBuilding.Id); + + buildingInDatabase.Should().BeNull(); + }); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/State.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/State.cs new file mode 100644 index 0000000000..28fb189d05 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/State.cs @@ -0,0 +1,15 @@ +using System.Collections.Generic; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class State : Identifiable + { + [Attr] + public string Name { get; set; } + + [HasMany] + public IList Cities { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StatesController.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StatesController.cs new file mode 100644 index 0000000000..28c8b795b8 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StatesController.cs @@ -0,0 +1,16 @@ +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class StatesController : JsonApiController + { + public StatesController(IJsonApiOptions options, ILoggerFactory loggerFactory, + IResourceService resourceService) + : base(options, loggerFactory, resourceService) + { + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs new file mode 100644 index 0000000000..5096c7ea63 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs @@ -0,0 +1,25 @@ +using System.Collections.Generic; +using System.Linq; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class Street : Identifiable + { + [Attr] + public string Name { get; set; } + + [Attr] + public int BuildingCount => Buildings?.Count ?? 0; + + [Attr] + public int DoorTotalCount => Buildings?.Sum(building => building.SecondaryDoor == null ? 1 : 2) ?? 0; + + [Attr] + public int WindowTotalCount => Buildings?.Sum(building => building.WindowCount) ?? 0; + + [EagerLoad] + public IList Buildings { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StreetsController.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StreetsController.cs new file mode 100644 index 0000000000..e6b4eda7e1 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/StreetsController.cs @@ -0,0 +1,16 @@ +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class StreetsController : JsonApiController + { + public StreetsController(IJsonApiOptions options, ILoggerFactory loggerFactory, + IResourceService resourceService) + : base(options, loggerFactory, resourceService) + { + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Window.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Window.cs new file mode 100644 index 0000000000..85a10775e2 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Window.cs @@ -0,0 +1,9 @@ +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class Window + { + public int Id { get; set; } + public int HeightInCentimeters { get; set; } + public int WidthInCentimeters { get; set; } + } +} diff --git a/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs b/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs index faca3e6f78..fbae95b7c1 100644 --- a/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs +++ b/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs @@ -224,7 +224,7 @@ private sealed class IntResourceRepository : IResourceRepository { public Task> GetAsync(QueryLayer layer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CountAsync(FilterExpression topFilter, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task CreateAsync(IntResource resource, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task CreateAsync(IntResource resourceFromRequest, IntResource resourceForDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task AddToToManyRelationshipAsync(int primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task UpdateAsync(IntResource resourceFromRequest, IntResource resourceFromDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task SetRelationshipAsync(IntResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); @@ -237,7 +237,7 @@ private sealed class GuidResourceRepository : IResourceRepository> GetAsync(QueryLayer layer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CountAsync(FilterExpression topFilter, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task CreateAsync(GuidResource resource, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task CreateAsync(GuidResource resourceFromRequest, GuidResource resourceForDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task AddToToManyRelationshipAsync(Guid primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task UpdateAsync(GuidResource resourceFromRequest, GuidResource resourceFromDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task SetRelationshipAsync(GuidResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); From 23991d5ed0457ba531af85f6a004326313f8fecd Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Wed, 18 Nov 2020 17:55:13 +0100 Subject: [PATCH 2/3] Updated test to set calculated property that depends on required relationship --- .../IntegrationTests/EagerLoading/Building.cs | 15 ++++++++++++--- .../EagerLoading/EagerLoadingTests.cs | 5 ++++- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs index 61913ec152..ac32eeb9c5 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs @@ -17,11 +17,14 @@ public string PrimaryDoorColor { get { - // Must ensure that an instance exists for this required relationship, so that POST succeeds. - PrimaryDoor ??= new Door(); - + EnsureHasPrimaryDoor(); return PrimaryDoor.Color; } + set + { + EnsureHasPrimaryDoor(); + PrimaryDoor.Color = value; + } } [Attr] @@ -35,5 +38,11 @@ public string PrimaryDoorColor [EagerLoad] public Door SecondaryDoor { get; set; } + + private void EnsureHasPrimaryDoor() + { + // Must ensure that an instance exists for this required relationship, so that POST succeeds. + PrimaryDoor ??= new Door(); + } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs index daf2c15eab..9c49a34b84 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs @@ -256,6 +256,7 @@ public async Task Can_update_resource() existingBuilding.Windows = _fakers.Window.Generate(2); var newBuildingNumber = _fakers.Building.Generate().Number; + var newPrimaryDoorColor = _fakers.Door.Generate().Color; await _testContext.RunOnDatabaseAsync(async dbContext => { @@ -271,7 +272,8 @@ await _testContext.RunOnDatabaseAsync(async dbContext => id = existingBuilding.StringId, attributes = new { - number = newBuildingNumber + number = newBuildingNumber, + primaryDoorColor = newPrimaryDoorColor } } }; @@ -297,6 +299,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext => buildingInDatabase.Should().NotBeNull(); buildingInDatabase.Number.Should().Be(newBuildingNumber); buildingInDatabase.PrimaryDoor.Should().NotBeNull(); + buildingInDatabase.PrimaryDoor.Color.Should().Be(newPrimaryDoorColor); buildingInDatabase.SecondaryDoor.Should().NotBeNull(); buildingInDatabase.Windows.Should().HaveCount(2); }); From 5e82946f3b76e0605371da637de8434341a6ef10 Mon Sep 17 00:00:00 2001 From: Bart Koelman Date: Thu, 19 Nov 2020 11:42:07 +0100 Subject: [PATCH 3/3] 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). --- .../EntityFrameworkCoreRepository.cs | 175 +++++++++--------- .../IResourceRepositoryAccessor.cs | 28 +-- .../Repositories/IResourceWriteRepository.cs | 30 +-- .../ResourceRepositoryAccessor.cs | 34 ++-- .../Services/JsonApiResourceService.cs | 3 +- .../IntegrationTests/EagerLoading/Building.cs | 34 ++-- .../EagerLoading/BuildingRepository.cs | 31 ++++ .../EagerLoading/EagerLoadingTests.cs | 6 + .../IntegrationTests/EagerLoading/Street.cs | 10 +- .../IServiceCollectionExtensionsTests.cs | 14 +- 10 files changed, 221 insertions(+), 144 deletions(-) create mode 100644 test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingRepository.cs diff --git a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs index e05827242b..11c5f0ad2d 100644 --- a/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/EntityFrameworkCoreRepository.cs @@ -114,6 +114,15 @@ protected virtual IQueryable GetAll() return _dbContext.Set(); } + /// + public virtual Task GetForCreateAsync(TId id, CancellationToken cancellationToken) + { + var resource = _resourceFactory.CreateInstance(); + resource.Id = id; + + return Task.FromResult(resource); + } + /// public virtual async Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken) { @@ -141,37 +150,10 @@ public virtual async Task CreateAsync(TResource resourceFromRequest, TResource r } /// - public virtual async Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) - { - _traceWriter.LogMethodStart(new {primaryId, secondaryResourceIds}); - if (secondaryResourceIds == null) throw new ArgumentNullException(nameof(secondaryResourceIds)); - - var relationship = _targetedFields.Relationships.Single(); - - if (secondaryResourceIds.Any()) - { - using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); - var primaryResource = collector.CreateForId(primaryId); - - await UpdateRelationshipAsync(relationship, primaryResource, secondaryResourceIds, collector, cancellationToken); - - await SaveChangesAsync(cancellationToken); - } - } - - /// - public virtual async Task SetRelationshipAsync(TResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) + public virtual async Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) { - _traceWriter.LogMethodStart(new {primaryResource, secondaryResourceIds}); - - var relationship = _targetedFields.Relationships.Single(); - - AssertIsNotClearingRequiredRelationship(relationship, primaryResource, secondaryResourceIds); - - using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); - await UpdateRelationshipAsync(relationship, primaryResource, secondaryResourceIds, collector, cancellationToken); - - await SaveChangesAsync(cancellationToken); + var resources = await GetAsync(queryLayer, cancellationToken); + return resources.FirstOrDefault(); } /// @@ -200,6 +182,39 @@ public virtual async Task UpdateAsync(TResource resourceFromRequest, TResource r await SaveChangesAsync(cancellationToken); } + protected void AssertIsNotClearingRequiredRelationship(RelationshipAttribute relationship, TResource leftResource, object rightValue) + { + bool relationshipIsRequired = false; + + if (!(relationship is HasManyThroughAttribute)) + { + var navigation = TryGetNavigation(relationship); + relationshipIsRequired = navigation?.ForeignKey?.IsRequired ?? false; + } + + var relationshipIsBeingCleared = relationship is HasOneAttribute + ? rightValue == null + : IsRequiredToManyRelationshipBeingCleared(relationship, leftResource, rightValue); + + if (relationshipIsRequired && relationshipIsBeingCleared) + { + var resourceType = _resourceGraph.GetResourceContext().PublicName; + throw new CannotClearRequiredRelationshipException(relationship.PublicName, leftResource.StringId, resourceType); + } + } + + private static bool IsRequiredToManyRelationshipBeingCleared(RelationshipAttribute relationship, TResource leftResource, object valueToAssign) + { + ICollection newRightResourceIds = TypeHelper.ExtractResources(valueToAssign); + + var existingRightValue = relationship.GetValue(leftResource); + var existingRightResourceIds = TypeHelper.ExtractResources(existingRightValue).ToHashSet(IdentifiableComparer.Instance); + + existingRightResourceIds.ExceptWith(newRightResourceIds); + + return existingRightResourceIds.Any(); + } + /// public virtual async Task DeleteAsync(TId id, CancellationToken cancellationToken) { @@ -261,77 +276,70 @@ private INavigation TryGetNavigation(RelationshipAttribute relationship) return entityType?.FindNavigation(relationship.Property.Name); } - /// - public virtual async Task RemoveFromToManyRelationshipAsync(TResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken) + private bool HasForeignKeyAtLeftSide(RelationshipAttribute relationship) { - _traceWriter.LogMethodStart(new {primaryResource, secondaryResourceIds}); - if (secondaryResourceIds == null) throw new ArgumentNullException(nameof(secondaryResourceIds)); + if (relationship is HasOneAttribute) + { + var navigation = TryGetNavigation(relationship); + return navigation?.IsDependentToPrincipal() ?? false; + } - var relationship = (HasManyAttribute)_targetedFields.Relationships.Single(); + return false; + } - var rightValue = relationship.GetValue(primaryResource); + /// + public virtual async Task SetRelationshipAsync(TResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) + { + _traceWriter.LogMethodStart(new {primaryResource, secondaryResourceIds}); - var rightResourceIds= TypeHelper.ExtractResources(rightValue).ToHashSet(IdentifiableComparer.Instance); - rightResourceIds.ExceptWith(secondaryResourceIds); + var relationship = _targetedFields.Relationships.Single(); - AssertIsNotClearingRequiredRelationship(relationship, primaryResource, rightResourceIds); + AssertIsNotClearingRequiredRelationship(relationship, primaryResource, secondaryResourceIds); using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); - await UpdateRelationshipAsync(relationship, primaryResource, rightResourceIds, collector, cancellationToken); + await UpdateRelationshipAsync(relationship, primaryResource, secondaryResourceIds, collector, cancellationToken); await SaveChangesAsync(cancellationToken); } - protected void AssertIsNotClearingRequiredRelationship(RelationshipAttribute relationship, TResource leftResource, object rightValue) + /// + public virtual async Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) { - bool relationshipIsRequired = false; + _traceWriter.LogMethodStart(new {primaryId, secondaryResourceIds}); + if (secondaryResourceIds == null) throw new ArgumentNullException(nameof(secondaryResourceIds)); - if (!(relationship is HasManyThroughAttribute)) - { - var navigation = TryGetNavigation(relationship); - relationshipIsRequired = navigation?.ForeignKey?.IsRequired ?? false; - } + var relationship = _targetedFields.Relationships.Single(); - var relationshipIsBeingCleared = relationship is HasOneAttribute - ? rightValue == null - : IsRequiredToManyRelationshipBeingCleared(relationship, leftResource, rightValue); - - if (relationshipIsRequired && relationshipIsBeingCleared) + if (secondaryResourceIds.Any()) { - var resourceType = _resourceGraph.GetResourceContext().PublicName; - throw new CannotClearRequiredRelationshipException(relationship.PublicName, leftResource.StringId, resourceType); + using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); + var primaryResource = collector.CreateForId(primaryId); + + await UpdateRelationshipAsync(relationship, primaryResource, secondaryResourceIds, collector, cancellationToken); + + await SaveChangesAsync(cancellationToken); } } - private static bool IsRequiredToManyRelationshipBeingCleared(RelationshipAttribute relationship, TResource leftResource, object valueToAssign) + /// + public virtual async Task RemoveFromToManyRelationshipAsync(TResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken) { - ICollection newRightResourceIds = TypeHelper.ExtractResources(valueToAssign); + _traceWriter.LogMethodStart(new {primaryResource, secondaryResourceIds}); + if (secondaryResourceIds == null) throw new ArgumentNullException(nameof(secondaryResourceIds)); - var existingRightValue = relationship.GetValue(leftResource); - var existingRightResourceIds = TypeHelper.ExtractResources(existingRightValue).ToHashSet(IdentifiableComparer.Instance); + var relationship = (HasManyAttribute)_targetedFields.Relationships.Single(); - existingRightResourceIds.ExceptWith(newRightResourceIds); + var rightValue = relationship.GetValue(primaryResource); - return existingRightResourceIds.Any(); - } + var rightResourceIds= TypeHelper.ExtractResources(rightValue).ToHashSet(IdentifiableComparer.Instance); + rightResourceIds.ExceptWith(secondaryResourceIds); - /// - public virtual async Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) - { - var resources = await GetAsync(queryLayer, cancellationToken); - return resources.FirstOrDefault(); - } + AssertIsNotClearingRequiredRelationship(relationship, primaryResource, rightResourceIds); - protected virtual async Task SaveChangesAsync(CancellationToken cancellationToken) - { - try - { - await _dbContext.SaveChangesAsync(cancellationToken); - } - catch (DbUpdateException exception) - { - throw new DataStoreUpdateException(exception); - } + using var collector = new PlaceholderResourceCollector(_resourceFactory, _dbContext); + await UpdateRelationshipAsync(relationship, primaryResource, rightResourceIds, collector, cancellationToken); + + await SaveChangesAsync(cancellationToken); } protected async Task UpdateRelationshipAsync(RelationshipAttribute relationship, TResource leftResource, @@ -383,15 +391,16 @@ private static bool IsOneToOneRelationship(RelationshipAttribute relationship) return false; } - private bool HasForeignKeyAtLeftSide(RelationshipAttribute relationship) + protected virtual async Task SaveChangesAsync(CancellationToken cancellationToken) { - if (relationship is HasOneAttribute) + try { - var navigation = TryGetNavigation(relationship); - return navigation?.IsDependentToPrincipal() ?? false; + await _dbContext.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateException exception) + { + throw new DataStoreUpdateException(exception); } - - return false; } } diff --git a/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs index 3eea039525..2eafaa2f39 100644 --- a/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs +++ b/src/JsonApiDotNetCore/Repositories/IResourceRepositoryAccessor.cs @@ -30,6 +30,12 @@ Task> GetAsync(QueryLayer layer, Cance Task CountAsync(FilterExpression topFilter, CancellationToken cancellationToken) where TResource : class, IIdentifiable; + /// + /// Invokes . + /// + Task GetForCreateAsync(TId id, CancellationToken cancellationToken) + where TResource : class, IIdentifiable; + /// /// Invokes . /// @@ -37,10 +43,10 @@ Task CreateAsync(TResource resourceFromRequest, TResource resourceFor where TResource : class, IIdentifiable; /// - /// Invokes for the specified resource type. + /// Invokes . /// - Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) - where TResource : class, IIdentifiable; + Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) + where TResource : class, IIdentifiable; /// /// Invokes . @@ -48,6 +54,12 @@ Task AddToToManyRelationshipAsync(TId primaryId, ISet(TResource resourceFromRequest, TResource resourceFromDatabase, CancellationToken cancellationToken) where TResource : class, IIdentifiable; + /// + /// Invokes for the specified resource type. + /// + Task DeleteAsync(TId id, CancellationToken cancellationToken) + where TResource : class, IIdentifiable; + /// /// Invokes . /// @@ -55,9 +67,9 @@ Task SetRelationshipAsync(TResource primaryResource, object secondary where TResource : class, IIdentifiable; /// - /// Invokes for the specified resource type. + /// Invokes for the specified resource type. /// - Task DeleteAsync(TId id, CancellationToken cancellationToken) + Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) where TResource : class, IIdentifiable; /// @@ -65,11 +77,5 @@ Task DeleteAsync(TId id, CancellationToken cancellationToken) /// Task RemoveFromToManyRelationshipAsync(TResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken) where TResource : class, IIdentifiable; - - /// - /// Invokes . - /// - Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) - where TResource : class, IIdentifiable; } } diff --git a/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs b/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs index b46e422f6b..27244c5f17 100644 --- a/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs +++ b/src/JsonApiDotNetCore/Repositories/IResourceWriteRepository.cs @@ -20,39 +20,47 @@ public interface IResourceWriteRepository public interface IResourceWriteRepository where TResource : class, IIdentifiable { + /// + /// Creates a new resource instance, in preparation for . + /// + /// + /// This method can be overridden to assign resource-specific required relationships. + /// + Task GetForCreateAsync(TId id, CancellationToken cancellationToken); + /// /// Creates a new resource in the underlying data store. /// Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken); /// - /// Adds resources to a to-many relationship in the underlying data store. + /// Retrieves a resource with all of its attributes, including the set of targeted relationships, in preparation for . /// - Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken); + Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken); /// /// Updates the attributes and relationships of an existing resource in the underlying data store. /// Task UpdateAsync(TResource resourceFromRequest, TResource resourceFromDatabase, CancellationToken cancellationToken); + /// + /// Deletes an existing resource from the underlying data store. + /// + Task DeleteAsync(TId id, CancellationToken cancellationToken); + /// /// Performs a complete replacement of the relationship in the underlying data store. /// Task SetRelationshipAsync(TResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken); - + /// - /// Deletes an existing resource from the underlying data store. + /// Adds resources to a to-many relationship in the underlying data store. /// - Task DeleteAsync(TId id, CancellationToken cancellationToken); - + Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken); + /// /// Removes resources from a to-many relationship in the underlying data store. /// Task RemoveFromToManyRelationshipAsync(TResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken); - - /// - /// Retrieves a resource with all of its attributes, including the set of targeted relationships, in preparation for update. - /// - Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken); } } diff --git a/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs b/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs index 607773f131..11aaaf2a05 100644 --- a/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs +++ b/src/JsonApiDotNetCore/Repositories/ResourceRepositoryAccessor.cs @@ -47,6 +47,14 @@ public async Task CountAsync(FilterExpression topFilter, Cancell return (int) await repository.CountAsync(topFilter, cancellationToken); } + /// + public async Task GetForCreateAsync(TId id, CancellationToken cancellationToken) + where TResource : class, IIdentifiable + { + dynamic repository = GetWriteRepository(typeof(TResource)); + return await repository.GetForCreateAsync(id, cancellationToken); + } + /// public async Task CreateAsync(TResource resourceFromRequest, TResource resourceForDatabase, CancellationToken cancellationToken) where TResource : class, IIdentifiable @@ -56,11 +64,11 @@ public async Task CreateAsync(TResource resourceFromRequest, TResourc } /// - public async Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) - where TResource : class, IIdentifiable + public async Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) + where TResource : class, IIdentifiable { dynamic repository = GetWriteRepository(typeof(TResource)); - await repository.AddToToManyRelationshipAsync(primaryId, secondaryResourceIds, cancellationToken); + return await repository.GetForUpdateAsync(queryLayer, cancellationToken); } /// @@ -71,6 +79,14 @@ public async Task UpdateAsync(TResource resourceFromRequest, TResourc await repository.UpdateAsync(resourceFromRequest, resourceFromDatabase, cancellationToken); } + /// + public async Task DeleteAsync(TId id, CancellationToken cancellationToken) + where TResource : class, IIdentifiable + { + dynamic repository = GetWriteRepository(typeof(TResource)); + await repository.DeleteAsync(id, cancellationToken); + } + /// public async Task SetRelationshipAsync(TResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) where TResource : class, IIdentifiable @@ -80,11 +96,11 @@ public async Task SetRelationshipAsync(TResource primaryResource, obj } /// - public async Task DeleteAsync(TId id, CancellationToken cancellationToken) + public async Task AddToToManyRelationshipAsync(TId primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) where TResource : class, IIdentifiable { dynamic repository = GetWriteRepository(typeof(TResource)); - await repository.DeleteAsync(id, cancellationToken); + await repository.AddToToManyRelationshipAsync(primaryId, secondaryResourceIds, cancellationToken); } /// @@ -95,14 +111,6 @@ public async Task RemoveFromToManyRelationshipAsync(TResource primary await repository.RemoveFromToManyRelationshipAsync(primaryResource, secondaryResourceIds, cancellationToken); } - /// - public async Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) - where TResource : class, IIdentifiable - { - dynamic repository = GetWriteRepository(typeof(TResource)); - return await repository.GetForUpdateAsync(queryLayer, cancellationToken); - } - protected object GetReadRepository(Type resourceType) { var resourceContext = _resourceContextProvider.GetResourceContext(resourceType); diff --git a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs index 7e600d2c4e..f860d8a26c 100644 --- a/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs +++ b/src/JsonApiDotNetCore/Services/JsonApiResourceService.cs @@ -176,8 +176,7 @@ public virtual async Task CreateAsync(TResource resource, Cancellatio _hookExecutor.BeforeCreate(resourceFromRequest); - var resourceForDatabase = _resourceFactory.CreateInstance(); - resourceForDatabase.Id = resourceFromRequest.Id; + TResource resourceForDatabase = await _repositoryAccessor.GetForCreateAsync(resource.Id, cancellationToken); _resourceChangeTracker.SetInitiallyStoredAttributeValues(resourceForDatabase); diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs index ac32eeb9c5..ff68b9a0c5 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Building.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; @@ -6,28 +7,37 @@ namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading { public sealed class Building : Identifiable { + private string _tempPrimaryDoorColor; + [Attr] public string Number { get; set; } + [NotMapped] [Attr] public int WindowCount => Windows?.Count ?? 0; - [Attr] + [NotMapped] + [Attr(Capabilities = AttrCapabilities.AllowView | AttrCapabilities.AllowChange)] public string PrimaryDoorColor { - get - { - EnsureHasPrimaryDoor(); - return PrimaryDoor.Color; - } + get => _tempPrimaryDoorColor ?? PrimaryDoor.Color; set { - EnsureHasPrimaryDoor(); - PrimaryDoor.Color = value; + if (PrimaryDoor == null) + { + // A request body is being deserialized. At this time, related entities have not been loaded. + // We cache the assigned value in a private field, so it can be used later. + _tempPrimaryDoorColor = value; + } + else + { + PrimaryDoor.Color = value; + } } } - [Attr] + [NotMapped] + [Attr(Capabilities = AttrCapabilities.AllowView)] public string SecondaryDoorColor => SecondaryDoor?.Color; [EagerLoad] @@ -38,11 +48,5 @@ public string PrimaryDoorColor [EagerLoad] public Door SecondaryDoor { get; set; } - - private void EnsureHasPrimaryDoor() - { - // Must ensure that an instance exists for this required relationship, so that POST succeeds. - PrimaryDoor ??= new Door(); - } } } diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingRepository.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingRepository.cs new file mode 100644 index 0000000000..e8fd8644d5 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/BuildingRepository.cs @@ -0,0 +1,31 @@ +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Queries; +using JsonApiDotNetCore.Repositories; +using JsonApiDotNetCore.Resources; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.EagerLoading +{ + public sealed class BuildingRepository : EntityFrameworkCoreRepository + { + public BuildingRepository(ITargetedFields targetedFields, IDbContextResolver contextResolver, + IResourceGraph resourceGraph, IResourceFactory resourceFactory, + IEnumerable constraintProviders, ILoggerFactory loggerFactory) + : base(targetedFields, contextResolver, resourceGraph, resourceFactory, constraintProviders, loggerFactory) + { + } + + public override async Task GetForCreateAsync(int id, CancellationToken cancellationToken) + { + var building = await base.GetForCreateAsync(id, cancellationToken); + + // Must ensure that an instance exists for this required relationship, so that POST succeeds. + building.PrimaryDoor = new Door(); + + return building; + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs index 9c49a34b84..9d9c28e0a2 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/EagerLoadingTests.cs @@ -2,6 +2,7 @@ using System.Net; using System.Threading.Tasks; using FluentAssertions; +using JsonApiDotNetCore.Configuration; using JsonApiDotNetCore.Serialization.Objects; using Microsoft.EntityFrameworkCore; using Xunit; @@ -17,6 +18,11 @@ public sealed class EagerLoadingTests public EagerLoadingTests(IntegrationTestContext, EagerLoadingDbContext> testContext) { _testContext = testContext; + + testContext.ConfigureServicesAfterStartup(services => + { + services.AddResourceRepository(); + }); } [Fact] diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs index 5096c7ea63..7e275d03a2 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/EagerLoading/Street.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using JsonApiDotNetCore.Resources; using JsonApiDotNetCore.Resources.Annotations; @@ -10,13 +11,16 @@ public sealed class Street : Identifiable [Attr] public string Name { get; set; } - [Attr] + [NotMapped] + [Attr(Capabilities = AttrCapabilities.AllowView)] public int BuildingCount => Buildings?.Count ?? 0; - [Attr] + [NotMapped] + [Attr(Capabilities = AttrCapabilities.AllowView)] public int DoorTotalCount => Buildings?.Sum(building => building.SecondaryDoor == null ? 1 : 2) ?? 0; - [Attr] + [NotMapped] + [Attr(Capabilities = AttrCapabilities.AllowView)] public int WindowTotalCount => Buildings?.Sum(building => building.WindowCount) ?? 0; [EagerLoad] diff --git a/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs b/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs index fbae95b7c1..b42e16e701 100644 --- a/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs +++ b/test/UnitTests/Extensions/IServiceCollectionExtensionsTests.cs @@ -224,26 +224,28 @@ private sealed class IntResourceRepository : IResourceRepository { public Task> GetAsync(QueryLayer layer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CountAsync(FilterExpression topFilter, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task GetForCreateAsync(int id, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CreateAsync(IntResource resourceFromRequest, IntResource resourceForDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task AddToToManyRelationshipAsync(int primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task UpdateAsync(IntResource resourceFromRequest, IntResource resourceFromDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task SetRelationshipAsync(IntResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task DeleteAsync(int id, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task SetRelationshipAsync(IntResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task AddToToManyRelationshipAsync(int primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task RemoveFromToManyRelationshipAsync(IntResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) => throw new NotImplementedException(); } private sealed class GuidResourceRepository : IResourceRepository { public Task> GetAsync(QueryLayer layer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CountAsync(FilterExpression topFilter, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task GetForCreateAsync(Guid id, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task CreateAsync(GuidResource resourceFromRequest, GuidResource resourceForDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task AddToToManyRelationshipAsync(Guid primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task UpdateAsync(GuidResource resourceFromRequest, GuidResource resourceFromDatabase, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task SetRelationshipAsync(GuidResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task DeleteAsync(Guid id, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task SetRelationshipAsync(GuidResource primaryResource, object secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); + public Task AddToToManyRelationshipAsync(Guid primaryId, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); public Task RemoveFromToManyRelationshipAsync(GuidResource primaryResource, ISet secondaryResourceIds, CancellationToken cancellationToken) => throw new NotImplementedException(); - public Task GetForUpdateAsync(QueryLayer queryLayer, CancellationToken cancellationToken) => throw new NotImplementedException(); } public class TestContext : DbContext