diff --git a/Directory.Build.props b/Directory.Build.props index ae0468de7d..e1d2a04da6 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -2,8 +2,8 @@ netcoreapp3.1 3.1.* - 3.1.* - 3.1.* + 5.0.* + 5.0.* $(SolutionDir)CodingGuidelines.ruleset diff --git a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs index b6ddf29c9a..be618d0af2 100644 --- a/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs +++ b/src/JsonApiDotNetCore/Queries/Internal/QueryableBuilding/SelectClauseBuilder.cs @@ -133,7 +133,13 @@ private ICollection ToPropertySelectors(IDictionary + { + // Data + + [Attr] + public string Name { get; set; } + + // Navigation Properties + + [HasMany] + [EagerLoad] + public ICollection Parties { get; set; } //= new List(); + + [HasMany] + [NotMapped] + public ICollection FirstParties => + Parties.Where(party => party.Role == ModelConstants.FirstPartyRoleName).OrderBy(party => party.ShortName).ToList(); + + [HasMany] + [NotMapped] + public ICollection SecondParties => + Parties.Where(party => party.Role == ModelConstants.SecondPartyRoleName).OrderBy(party => party.ShortName).ToList(); + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartiesController.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartiesController.cs new file mode 100644 index 0000000000..8e7977c0ef --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartiesController.cs @@ -0,0 +1,16 @@ +using System; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + public sealed class EngagementPartiesController : JsonApiController + { + public EngagementPartiesController(IJsonApiOptions options, ILoggerFactory loggerFactory, IResourceService resourceService) + : base(options, loggerFactory, resourceService) + { + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementParty.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementParty.cs new file mode 100644 index 0000000000..3d7df26b5b --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementParty.cs @@ -0,0 +1,25 @@ +using System; +using JetBrains.Annotations; +using JsonApiDotNetCore.Resources.Annotations; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + [UsedImplicitly(ImplicitUseTargetFlags.Members)] + public sealed class EngagementParty : EntityBase + { + // Data (simplified) + + [Attr] + public string Role { get; set; } + + [Attr] + public string ShortName { get; set; } + + // Foreign Keys (simplified) + + [HasOne] + public Engagement Engagement { get; set; } + + //public Guid EngagementId { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartyResourceDefinition.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartyResourceDefinition.cs new file mode 100644 index 0000000000..f9cff0f024 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementPartyResourceDefinition.cs @@ -0,0 +1,34 @@ +using System; +using System.ComponentModel; +using JetBrains.Annotations; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Queries.Expressions; +using JsonApiDotNetCore.Resources; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + [UsedImplicitly(ImplicitUseKindFlags.InstantiatedNoFixedConstructorSignature)] + public sealed class EngagementPartyResourceDefinition : JsonApiResourceDefinition + { + /// + public EngagementPartyResourceDefinition(IResourceGraph resourceGraph) + : base(resourceGraph) + { + } + + /// + public override SortExpression OnApplySort(SortExpression existingSort) + { + if (existingSort != null) + { + return existingSort; + } + + return CreateSortExpressionFromLambda(new PropertySortOrder + { + (ep => ep.Role, ListSortDirection.Ascending), + (ep => ep.ShortName, ListSortDirection.Ascending) + }); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementsController.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementsController.cs new file mode 100644 index 0000000000..57d98abacc --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EngagementsController.cs @@ -0,0 +1,16 @@ +using System; +using JsonApiDotNetCore.Configuration; +using JsonApiDotNetCore.Controllers; +using JsonApiDotNetCore.Services; +using Microsoft.Extensions.Logging; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + public sealed class EngagementsController : JsonApiController + { + public EngagementsController(IJsonApiOptions options, ILoggerFactory loggerFactory, IResourceService resourceService) + : base(options, loggerFactory, resourceService) + { + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EntityBase.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EntityBase.cs new file mode 100644 index 0000000000..2bbdc72724 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/EntityBase.cs @@ -0,0 +1,18 @@ +using System; +using System.ComponentModel.DataAnnotations; +using JetBrains.Annotations; +using JsonApiDotNetCore.Resources; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + [UsedImplicitly(ImplicitUseTargetFlags.Members)] + public abstract class EntityBase : Identifiable + { + public DateTimeOffset? DateCreated { get; set; } + + public DateTimeOffset? DateModified { get; set; } + + [Timestamp] + public byte[] RowVersion { get; set; } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueDbContext.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueDbContext.cs new file mode 100644 index 0000000000..e9354dafe6 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueDbContext.cs @@ -0,0 +1,28 @@ +using JetBrains.Annotations; +using Microsoft.EntityFrameworkCore; + +// @formatter:wrap_chained_method_calls chop_always + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + [UsedImplicitly(ImplicitUseTargetFlags.Members)] + public sealed class IssueDbContext : DbContext + { + public DbSet Engagements { get; set; } + public DbSet EngagementParties { get; set; } + + public IssueDbContext(DbContextOptions options) + : base(options) + { + } + + protected override void OnModelCreating(ModelBuilder builder) + { + builder.Entity() + .HasOne(engagementParty => engagementParty.Engagement) + .WithMany(engagement => engagement.Parties) + .IsRequired() + .OnDelete(DeleteBehavior.Restrict); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueFakers.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueFakers.cs new file mode 100644 index 0000000000..2c0a9026f9 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueFakers.cs @@ -0,0 +1,26 @@ +using System; +using Bogus; +using TestBuildingBlocks; + +// @formatter:wrap_chained_method_calls chop_always +// @formatter:keep_existing_linebreaks true + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + internal sealed class IssueFakers : FakerContainer + { + private readonly Lazy> _lazyEngagementFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(engagement => engagement.Name, faker => faker.Lorem.Word())); + + private readonly Lazy> _lazyEngagementPartyFaker = new Lazy>(() => + new Faker() + .UseSeed(GetFakerSeed()) + .RuleFor(party => party.Role, faker => faker.Lorem.Word()) + .RuleFor(party => party.ShortName, faker => faker.Lorem.Word())); + + public Faker Engagement => _lazyEngagementFaker.Value; + public Faker EngagementParty => _lazyEngagementPartyFaker.Value; + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueTests.cs new file mode 100644 index 0000000000..adaa450930 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/IssueTests.cs @@ -0,0 +1,215 @@ +using System; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Threading.Tasks; +using FluentAssertions; +using JsonApiDotNetCore.Resources; +using JsonApiDotNetCore.Serialization.Objects; +using JsonApiDotNetCoreExampleTests.Startups; +using Microsoft.Extensions.DependencyInjection; +using TestBuildingBlocks; +using Xunit; + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + public sealed class IssueTests : IClassFixture, IssueDbContext>> + { + private readonly ExampleIntegrationTestContext, IssueDbContext> _testContext; + private readonly IssueFakers _fakers = new IssueFakers(); + + public IssueTests(ExampleIntegrationTestContext, IssueDbContext> testContext) + { + _testContext = testContext; + + testContext.UseController(); + testContext.UseController(); + + testContext.ConfigureServicesAfterStartup(services => + { + services.AddScoped, EngagementPartyResourceDefinition>(); + }); + } + + [Fact] + public async Task Can_get_primary_resource_by_ID() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = "/engagements/" + engagement.StringId; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(engagement.StringId); + + responseDocument.Included.Should().BeNull(); + } + + [Fact] + public async Task Can_get_primary_resource_by_ID_with_includes() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + engagement.Parties = _fakers.EngagementParty.Generate(3); + engagement.Parties.ElementAt(0).Role = ModelConstants.FirstPartyRoleName; + engagement.Parties.ElementAt(1).Role = ModelConstants.SecondPartyRoleName; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/engagements/{engagement.StringId}?include=firstParties,secondParties"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.SingleData.Should().NotBeNull(); + responseDocument.SingleData.Id.Should().Be(engagement.StringId); + + responseDocument.Included.Should().HaveCount(2); + responseDocument.Included.Should().ContainSingle(resourceObject => resourceObject.Id == engagement.Parties.ElementAt(0).StringId); + responseDocument.Included.Should().ContainSingle(resourceObject => resourceObject.Id == engagement.Parties.ElementAt(1).StringId); + } + + [Fact] + public async Task Can_get_secondary_resources_by_ID_without_sort() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + engagement.Parties = _fakers.EngagementParty.Generate(3); + engagement.Parties.ElementAt(0).Role = ModelConstants.SecondPartyRoleName; + engagement.Parties.ElementAt(1).Role = ModelConstants.FirstPartyRoleName; + engagement.Parties.ElementAt(1).ShortName = "B"; + engagement.Parties.ElementAt(2).Role = ModelConstants.FirstPartyRoleName; + engagement.Parties.ElementAt(2).ShortName = "A"; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/engagements/{engagement.StringId}/parties"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(3); + responseDocument.ManyData[0].Id.Should().Be(engagement.Parties.ElementAt(2).StringId); + responseDocument.ManyData[1].Id.Should().Be(engagement.Parties.ElementAt(1).StringId); + responseDocument.ManyData[2].Id.Should().Be(engagement.Parties.ElementAt(0).StringId); + + responseDocument.Included.Should().BeNull(); + } + + [Fact] + public async Task Can_get_secondary_resources_by_ID_with_sort() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + engagement.Parties = _fakers.EngagementParty.Generate(2); + engagement.Parties.ElementAt(0).ShortName = "B"; + engagement.Parties.ElementAt(1).ShortName = "A"; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/engagements/{engagement.StringId}/parties?sort=shortName"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(2); + responseDocument.ManyData[0].Id.Should().Be(engagement.Parties.ElementAt(1).StringId); + responseDocument.ManyData[1].Id.Should().Be(engagement.Parties.ElementAt(0).StringId); + + responseDocument.Included.Should().BeNull(); + } + + [Fact] + public async Task Can_get_secondary_resources_by_ID_with_descending_sort() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + engagement.Parties = _fakers.EngagementParty.Generate(2); + engagement.Parties.ElementAt(0).ShortName = "A"; + engagement.Parties.ElementAt(1).ShortName = "B"; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/engagements/{engagement.StringId}/parties?sort=-shortName"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(2); + responseDocument.ManyData[0].Id.Should().Be(engagement.Parties.ElementAt(1).StringId); + responseDocument.ManyData[1].Id.Should().Be(engagement.Parties.ElementAt(0).StringId); + + responseDocument.Included.Should().BeNull(); + } + + [Fact] + public async Task Can_get_unmapped_secondary_resources_by_ID() + { + // Arrange + Engagement engagement = _fakers.Engagement.Generate(); + engagement.Parties = _fakers.EngagementParty.Generate(3); + engagement.Parties.ElementAt(0).Role = ModelConstants.FirstPartyRoleName; + engagement.Parties.ElementAt(1).Role = ModelConstants.FirstPartyRoleName; + + await _testContext.RunOnDatabaseAsync(async dbContext => + { + dbContext.Engagements.Add(engagement); + await dbContext.SaveChangesAsync(); + }); + + string route = $"/engagements/{engagement.StringId}/firstParties"; + + // Act + (HttpResponseMessage httpResponse, Document responseDocument) = await _testContext.ExecuteGetAsync(route); + + // Assert + httpResponse.Should().HaveStatusCode(HttpStatusCode.OK); + + responseDocument.ManyData.Should().HaveCount(2); + responseDocument.ManyData.Should().ContainSingle(resourceObject => resourceObject.Id == engagement.Parties.ElementAt(0).StringId); + responseDocument.ManyData.Should().ContainSingle(resourceObject => resourceObject.Id == engagement.Parties.ElementAt(1).StringId); + + responseDocument.Included.Should().BeNull(); + } + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/ModelConstants.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/ModelConstants.cs new file mode 100644 index 0000000000..a9d732f635 --- /dev/null +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/Issue988/ModelConstants.cs @@ -0,0 +1,10 @@ +#pragma warning disable AV1008 // Class should not be static + +namespace JsonApiDotNetCoreExampleTests.IntegrationTests.Issue988 +{ + internal static class ModelConstants + { + public const string FirstPartyRoleName = "P1"; + public const string SecondPartyRoleName = "P2"; + } +} diff --git a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs index fb5e465329..085127f7b6 100644 --- a/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs +++ b/test/JsonApiDotNetCoreExampleTests/IntegrationTests/RequiredRelationships/DefaultBehaviorTests.cs @@ -94,7 +94,7 @@ public async Task Cannot_create_dependent_side_of_required_OneToOne_relationship Error error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.InternalServerError); error.Title.Should().Be("An unhandled error occurred while processing this request."); - error.Detail.Should().Be("Failed to persist changes in the underlying data store."); + error.Detail.Should().StartWith("The value of 'Shipment.Id' is unknown when attempting to save changes."); } [Fact] @@ -431,7 +431,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext => Error error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.InternalServerError); error.Title.Should().Be("An unhandled error occurred while processing this request."); - error.Detail.Should().StartWith("The property 'Id' on entity type 'Shipment' is part of a key and so cannot be modified or marked as modified."); + error.Detail.Should().StartWith("The property 'Shipment.Id' is part of a key and so cannot be modified or marked as modified."); } [Fact] @@ -473,7 +473,7 @@ await _testContext.RunOnDatabaseAsync(async dbContext => Error error = responseDocument.Errors[0]; error.StatusCode.Should().Be(HttpStatusCode.InternalServerError); error.Title.Should().Be("An unhandled error occurred while processing this request."); - error.Detail.Should().StartWith("The property 'Id' on entity type 'Shipment' is part of a key and so cannot be modified or marked as modified."); + error.Detail.Should().StartWith("The property 'Shipment.Id' is part of a key and so cannot be modified or marked as modified."); } } } diff --git a/test/TestBuildingBlocks/BaseIntegrationTestContext.cs b/test/TestBuildingBlocks/BaseIntegrationTestContext.cs index aba34ea755..37a5757ad9 100644 --- a/test/TestBuildingBlocks/BaseIntegrationTestContext.cs +++ b/test/TestBuildingBlocks/BaseIntegrationTestContext.cs @@ -5,6 +5,7 @@ using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Testing; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; @@ -74,6 +75,8 @@ private WebApplicationFactory CreateFactory() options.UseNpgsql(dbConnectionString); options.EnableSensitiveDataLogging(); options.EnableDetailedErrors(); + + options.ConfigureWarnings(builder => builder.Ignore(CoreEventId.InvalidIncludePathError)); }); }); diff --git a/test/TestBuildingBlocks/TestBuildingBlocks.csproj b/test/TestBuildingBlocks/TestBuildingBlocks.csproj index 3aa8cb646a..3af745c1f0 100644 --- a/test/TestBuildingBlocks/TestBuildingBlocks.csproj +++ b/test/TestBuildingBlocks/TestBuildingBlocks.csproj @@ -12,7 +12,7 @@ - +