Support models with key fields other than "Id"

[ClintGood]

Description

I don't want to be forced to create models where the key field is always "Id". I might choose to have "{EntityName}Id" as the key field
...

Environment

• JsonApiDotNetCore Version: latest trunk
• Other Relevant Package Versions:

[bart-degreed]

If all you need is that the database column name is different from "Id", you can do that using EF Core mappings.

From OnModelCreating:

modelBuilder.Entity<Article>(entity =>
{
    entity.HasKey(article => article.Id);
    entity.Property(article => article.Id).HasColumnName("ArticleId");
});

Or on the resource itself:

public class Article : Identifiable
{
    [Key]
    [Column("ArticleId")]
    public override int Id { get; set; }
}

Does this solve your problem?

[ClintGood]

This issue was really created to give the referenced PR an issue to solve

I know I could do it via ef mappings, but I see this as a step along the way to an end goal..

Ideally I would have a .net standard library containing my model. It might use System.ComponentModel.DataAnnotations attributes. The model classes would not be required to implement any IIdentifiable

I can use the ef fluent api to configure the database mapping in a .net core library that needs to use ef.
I an then use a jadnc fluent api to configure it in a .net core application that want to expose a json api.

I can have another .net core winforms app using the same model.

All nicely separated without having to unnecessarily introduce dependencies..

I will also say that when I first used jadnc, being forced to have an Id property on my class didn't feel particularly nice. If someone has an existing model that they are trying to expose via a json.api, this could put them off choosing jadnc, and it's such a minor change.

[bart-degreed]

The model classes would not be required to implement any IIdentifiable

I'm not a fan of that, because it would require us to implement a lot more runtime reflection and we'd exchange compile-time errors for runtime errors. In my opinion the strength of a statically typed language like C# is that it enables compile-time checking. While enforcing a base class can be constraining because C# does not support multiple inheritance, I think there is nothing wrong with using interfaces to express implementation requirements. The only reason that Id is not among them, is it complicates things internally today due to its generic nature.

If someone has an existing model that they are trying to expose via a json.api...

That is not possible at the moment, because it would need to have [Attr] annotations, so it must have a dependency on JADNC. I find it unlikely that someone wants to expose a model from a third-party library without wrapping or mapping it. Because if the third-party changes its signatures, it immediately results in breaking changes in your API.

To realize your end goal, a lot more work is needed before getting benefits from it:

• Define and implement a JADNC fluent API
• Split the nuget package into multiple packages, where one of them provides only annotation types like IIdentifyable, Attr, HasOne etc.

This is certainly something to consider for a future release, but right now we're trying to finish v4. Your change is minor indeed, but it is part of a larger vision that we won't be delivering in v4 and which is subject to change on details, which may affect your PR. It would be unfortunate if we introduce a new attribute now without real benefits, only to take a breaking change in the future when all things come together and we realize it doesn't fit like we thought. Therefore I'd like to postpone taking your PR until the related work is being done and we have a clearer understanding on how to implement that.

Right now, I would categorize this as a beatification for which working alternatives exist. We have a lot of open bugs and features to work on that fix/enable scenarios which users are depending on. Unfortunately our time is limited and we need to prioritize (reviewing, documentation, release notes, testing etc. takes time too).

[ClintGood]

Fair enough. The PR is closed.