Skip to content

JADNC does not support read-only navigation properties with backing fields. #1092

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ThomasBarnekow opened this issue Sep 29, 2021 · 6 comments

Comments

@ThomasBarnekow
Copy link

DESCRIPTION

Using backing fields, EF Core supports read-only navigation properties (e.g., Children) as follows (using a variation of the TestResource class already used in issue #1090 to demonstrate the constructor binding issue):

public class TestResource : Identifiable
{
    // Added backing field for navigation property.
    private readonly HashSet<TestResource> _children = new();
    
    // Same as before.
    [Attr]
    public string Name { get; set; }
    
    // Same as before.
    public int? ParentId { get; set; }
    
    // Same as before.
    [HasOne]
    public TestResource Parent { get; set; }
    
    // Changed type, made read-only and return _children.
    [HasMany]
    public IReadOnlyCollection<TestResource> Children
    {
        get => _children;
    
        // Setter required by JADNC (but not EF Core per se). Uncomment to fix the issue.
        // private init => _children = (HashSet<TestResource>) value;
    }
       
    // Added method to add items to _children backing field.
    public void Add(TestResource item)
    {
        _children.Add(item);
        item.Parent = this;
        item.ParentId = Id;
    }
}

Using EF core directly, it is possible to use the Children navigation property to retrieve the children of a "Root" resource, for example (see also the unit test in #1090):

public class RelationalDbIntegrationTests
{
    [Fact]
    public async Task TestRelationalDatabaseAsync()
    {
        await using (var context = new RelationalDbContext())
        {
            await context.Database.EnsureDeletedAsync().ConfigureAwait(false);
            await context.Database.MigrateAsync().ConfigureAwait(false);

            // This is the change required for the new API
            var root = new TestResource { Name = "Root" };
            root.Add(new TestResource { Name = "First" });
            root.Add(new TestResource { Name = "Second" });    
            context.Add(root);

            await context.SaveChangesAsync().ConfigureAwait(false);
        }

        await using (var context = new RelationalDbContext())
        {
            // Establish that we can read the resource using EF core.
            TestResource resource = await context.TestResources
                .Include(testResource => testResource.Children)
                .SingleAsync(testResource => testResource.Name == "Root");

            Assert.NotEmpty(resource.Children);
        }
    }
}

However, when using JADNC, the following request will not include any children:

  • https://localhost:5001/TestResources?include=children

When uncommenting private init => _children = (HashSet<TestResource>) value; as indicated above, everything works as desired (children are returned).

STEPS TO REPRODUCE

  1. Change the TestResource class as indicated above (without uncommenting the setter/initializer).
  2. Run the unit test described above to confirm children are indeed returned by EF Core.
  3. Issue the above request (https://localhost:5001/TestResources?include=children) and note that no children are included.
  4. Uncomment the setter/initializer. Run the unit test to confirm children are still returned by EF Core. Issue the above request again and note that children are now returned as expected.

EXPECTED BEHAVIOR

JADNC supports read-only navigation properties with backing fields.

ACTUAL BEHAVIOR

JADNC supports requires at least a private initializer or setter.

VERSIONS USED

  • JsonApiDotNetCore version: 4.2.0
  • ASP.NET Core version: 5.0.10
  • Entity Framework Core version: 5.0.10
  • Database provider: SQL Server Express LocalDb
@bart-degreed
Copy link
Contributor

EF Core contains logic to directly write to backing fields, based on naming conventions, when no setter is available. In your example, renaming _children to _somethingElse would break it. It was added because users complained that EF Core didn't work well with DDD, where it's good practice to protect business rule invariants by allowing writes exclusively through methods on the aggregate root.

None of that applies to EF Core, which is more similar to CRUD operations, as opposed to the functional RPC calls that write changes in DDD. I imagine we could add an extra property on relationship attributes to make them read-only for the outside world, yet it would still require a setter so JADNC can write the results of a database query into it.

The reason you're not getting any results for read-only relationship properties is because we actively skip them. Read-only attributes are usually calculated, based on other property values, so there is no point in crashing moments later because no setter exists.

What are you trying to accomplish that makes you think read-only collection properties with backing fields are the right solution?

@ThomasBarnekow
Copy link
Author

In this simplified example, the read-only property IReadOnlyCollection<TestResource> Children provides read access to the children while the method void Add(TestResource item) provides write access. The simple business logic here is the need to make sure that the Parent and ParentId properties of the item added to the collection are set properly. Other cases might require more business logic (e.g., checking whether the item has already been added to another collection, which might not be allowed).

When using EF Core directly, without JADNC in the middle, everything works just fine. However, JADNC seems to make additional assumptions and, therefore, requires additional access paths that are not required by EF Core. Therefore, I can't use models that would otherwise work just fine with EF Core if I also want to use JADNC. However, I would certainly love to be able to use models that comply with EF Core conventions while also using JADNC.

@bart-degreed
Copy link
Contributor

In reply to this issue and related to #1090 (comment): JADNC follows a different paradigm than what you're trying to do.

In a DDD world, entities are rich objects that guard business rule invariants through aggregate roots, such as: "this customer must not have more than 3 unpaid invoices at the same time unless his employer is a gold partner of ours". Of course, this can only work properly when you manually (eagerly or lazily) load the affected related entities upfront using a hand-written query. A web API to expose that would be RPC style, such as: /customers/1/create-invoice.

In contrast, JSON:API is a resource-oriented REST API. It provides operations to modify a single resource in isolation, as opposed to applying changes over a subtree from a root object (effectively executing remote procedures). If that's what you need, JSON:API is not a good fit.

JsonApiDotNetCore provides various declarative annotations and extensibility points to ease the pain from this fundamental paradigm mismatch, just don't expect it to go away. Instead of rich entities with hand-written logic, immutable data structures, self-validating constructor parameters, event-based notifications, etc, it relies on the use of annotations (.NET attributes), as well as ASP.NET ModelState validation. So to make sure User.IsAdmin is read-only through JADNC, you'd put AttrCapabilities on the IsAdmin property (which has a getter and a setter in code). In JADNC, resources are more like DTOs, while the business rules live in resource definition callbacks (see the Microservices example). This pattern violates all OO principles from DDD, which makes it a bad idea trying to combine that.

The fact that JADNC uses EF Core for data access, doesn't mean it supports everything that can be done with EF Core when writing custom LINQ queries. The simplest example would be projecting into an anonymous type: JADNC cannot do that. This means summations, aggregations, and groupings aren't possible too. Because all that exists in JSON:API are resources and the relationships between them. Expression trees are the interface that EF Core provides to JADNC, and so we must generate LINQ queries that project into resources.

I can't think of a query that is able to project a value into a resource with a read-only property. So unless you can provide such a query, I don't think we can implement what you're asking for. Even if we'd generate a query that uses reflection to write to the backing field, I doubt if EF Core would recognize that pattern and produce SQL out of it.

@ThomasBarnekow
Copy link
Author

@bart-degreed, thanks a lot for your explanation, which helps clarify some important design considerations.

Your explanation and some information on the features of EF Core that are not supported for good reasons would be a valuable addition to the documentation (unless I missed something and it is already there).

@bart-degreed
Copy link
Contributor

I don't think the JADNC docs are the best place to describe architectural concepts like what REST or HATEOAS means, which is already widely blogged about.

However, it's good to have some of these concepts written down in this issue, which users can search on, we can link to, etc.

@bart-degreed
Copy link
Contributor

FYI: I've added some guidance on using nullable reference types in v5 at https://www.jsonapi.net/usage/resources/nullability.html.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants