Potential Bug Serializing Id's?

[RKennedy9064]

I've been messing around with JsonApiDotNetCore a lot recently and encountered something strange. For some reason the database I'm working with has some tables where the Primary Key starts at 0 and I'm noticing some strange results. For any row where the Id is 0, it looks like it's not serializing the Id with the response. Is this intended behavior, or am I doing something wrong? For example, here is my request and response for an id of 1.

http://localtest.me:5000/api/v1/case-status-classes/1
{"data":{"attributes":{"label":"Pending","i18n-tag":null,"order-by":2},"type":"case-status-classes","id":"1"}}

and here is my request and response for an id of 0.

http://localtest.me:5000/api/v1/case-status-classes/0
{"data":{"attributes":{"label":"No Action","i18n-tag":null,"order-by":1},"type":"case-status-classes","id":""}}

It's able to find the case-status-class in the database with an Id of 0, but for some reason it's not sending the Id back with the response. When trying to bind my models in Ember it's giving me issues since there's no id.

"Assertion Failed: You must include an 'id' for case-status-class in an object passed to 'push'"

Is this an actual issue with how JsonApiDotNetCore is sending data back, or is it a user error and I have something messed up in my configuration? Just figured I'd bring it up since I noticed it. Any help would be greatly appreciated.

[jaredcnance]

@RKennedy9064 thanks for reporting this. Does your model inherit from Identifiable? If so, this is the source of the problem:

JsonApiDotNetCore/src/JsonApiDotNetCore/Models/Identifiable.cs

Lines 32 to 34 in 74667eb

	return stringValue == "0"
	? string.Empty
	: stringValue;

If this is the case, then you can:

• implement IIdentifiable<T> yourself
• override Identifiable<T>.GetStringId() e.g.

protected override string GetStringId(object value)
{
    // since we're in the concrete type, we know the type of the Id
    // and can dispense with the base implementation type handling
    return value.ToString();
}

I'll need to do some more research into whether or not we actually need the empty string id. I remember there was a use case around how to handle the unset case (default(int)), but it may not be necessary anymore.

[RKennedy9064]

@jaredcnance Thanks for the quick response and taking time out your day to help me out. Currently my models all inherit from Identifiable and I've been doing Identifiable and such for keys that aren't int's. I figured public override short Id {get; set;} would have been enough. Switching to implementing IIDentifieable<T> should be pretty straight forward. I'll try and fit that in tomorrow and let you know if it fixes the issue I'm having. Thanks again.

[RKennedy9064]

@jaredcnance I was able to test overriding GetStringId and the response now sends 0 as the Id with the model. Just figured I'd give you a heads up, not sure if you want to keep this open or not.

[jaredcnance]

Leaving it open because I'd like to revisit this later.

[czemanek-arm]

I would like to add some extra information to this issue. I had the exact same problem and unfortunately did not see this issue in my search, but after solving it I figured I would leave my experience.

I started by removing the return (string.Value == "0" ? "" : string.Value)

This started out fine, but created an issue: whenever I submitted a POST request it failed because 0 was being submitted as the ID when it should have been empty (identity key column)

My solution was to have the model inherit from Identifiable<Nullable<long>>. This allowed the ID to be null when passed into the controller, which then correctly omitted it from the request.

Not sure what fix would be available other than requiring all Identifiable types to be nullable, but I figured I would leave this info here in the hopes someone else may stumble upon it.

To summarize my steps to resolving all issues:

To resolve GET requests correctly, change:

return stringValue == "0" ? string.Empty : stringValue;

To the following:

return stringValue;

To resolve POST requests:
Make model ID type nullable.

[jaredcnance]

@czemanek-arm you shouldn't have to make ids nullable. This sounds like an issue with your DbContext setup. I suspect EF is not aware this is an autogenerated identity column and as such is issuing requests like INSERT INTO Books (Id, Name) VALUES (@id, @name)

[czemanek-arm]

It's possible. I did have the column set to Identity in the EF model, but the black box of EF knows no master.

[jaredcnance]

I did have the column set to Identity in the EF model

Can you show me how you're doing this? Also, can you enable verbose logging on EF and paste the logs you get during the POST request?

[czemanek-arm]

As far as I know there is no "Identity" attribute, but EF is supposed to detect it from an integer key, so I'm just doing:
[Key, Column("ID_BOOK")] public override long Id { get; set; }

Oh yeah I'll look into doing that, will probably be some time before I can sit down and play with it again.

[jaredcnance]

Ah I see. Unfortunately, the [Key] attribute doesn't imply that the value is auto generated.

EF is supposed to detect it from an integer key

Was the database created by EF or was it pre-existing? This may be part of the issue.

I would recommend trying this:

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
  modelBuilder.Entity<Book>()
    .Property(book => book.Id)
    .ForSqlServer()
    .UseIdentity();
}

Or this:

public class Book
{
    [DatabaseGenerated(DatabaseGeneratedOption.Identity)]
    public long Id { get; set; }
}

I also recommend reading the docs on Value generated on add:

Depending on the database provider being used, values may be generated client side by EF or in the database. If the value is generated by the database, then EF may assign a temporary value when you add the entity to the context. This temporary value will then be replaced by the database generated value during SaveChanges().
If you add an entity to the context that has a value assigned to the property, then EF will attempt to insert that value rather than generating a new one. A property is considered to have a value assigned if it is not assigned the CLR default value (null for string, 0 for int, Guid.Empty for Guid, etc.). For more information, see Explicit values for generated properties.

[czemanek-arm]

Gotcha, I'll take a look at refactoring some of this next time I get the chance, thanks for the pointers!

[beonami]

How about this issue? I have a table with the primary (int --not identity) column. The data there has a value of 0 on ID column. But when executed, the error message "Value cannot be null. (Parameter 'id')" appears (something like that).

Environment

• JsonApiDotNetCore Version: 4.0.0-beta1

[bkoelman]

I think the quote from EF Core (mentioned above) says it all:

A property is considered to have a value assigned if it is not assigned the CLR default value (null for string, 0 for int, Guid.Empty for Guid, etc.).

So it you want 0 to be treated as an actual value, you need to make sure it differs from the default value. In other words, make it nullable.

The next special handling was removed months ago, so I expect that workarounds are no longer needed:

return stringValue == "0" ? string.Empty : stringValue;

I haven't tried, but would expect both GET and POST to work properly for 0 values when inheriting your resource from Identifyable<int?>.

If any additional issues remain, please post your model, along with the request/response data. And in case of errors, include the full stack trace (which you'll get by enabling it in options).

[beonami]

Thank you for the instructions. For the int data type, it can be handled using Identifiable<int?>. But what about string data types? I have a database structure which has a primary key (string) and only 1 row contains EMPTY values.

Example models:

[Table("Bank")]
    public class Bank : Identifiable<string>
    {
        [Key]
        [Column("Code")]
        public override string Id { get; set; }

        [Attr]
        public string Name { get; set; } = "";
    }

Once again, thank you for the instructions.

[bkoelman]

I would think the same rule applies. "" is not equal to null (the default value for string) so it should just work, unless EF Core itself treats strings differently. Are you running into any issues or unexpected behavior?

[beonami]

By this model:

public class Bank : Identifiable<string>
{
    [Key]
    [Column("Code")]
    public override string Id { get; set; }

    [Attr]
    public string Name { get; set; } = "";
}

With examples of data such as:

Code       Name
-------    -----------------
(empty)    (Not Name)
A101       Bank A1
A102       Bank A2
A103       Bank A3

I sent a request:

GET /banks HTTP/1.1
Content-Type: application/vnd.api+json
Accept: application/vnd.api+json

{
    "errors": [
        {
            "id": "5212a003-b41f-4972-b56e-427adc886711",
            "status": "500",
            "title": "An unhandled error occurred while processing this request.",
            "detail": "Value cannot be null. (Parameter 'id')"
        }
    ]
}

The empty string handling should be the same as the Identifiable<int?> handling right?

[bart-degreed]

Interesting. Can you include the stack trace? Enable it in startup options to include it in the response.

[bart-degreed]

And which database are you using?

[beonami]

The following is the stack trace I got:

fail: JsonApiDotNetCore.Middleware.ExceptionHandler[0]
      Value cannot be null. (Parameter 'id')
System.ArgumentNullException: Value cannot be null. (Parameter 'id')
   at ResourceLinks JsonApiDotNetCore.Serialization.Building.LinkBuilder.GetResourceLinks(string resourceName, string id) in C:/projects/jsonapidotnetcore/src/JsonApiDotNetCore/Serialization/Building/LinkBuilder.cs:line 147
   at string JsonApiDotNetCore.Serialization.ResponseSerializer<TResource>.SerializeMany(IReadOnlyCollection<IIdentifiable> resources) in C:/projects/jsonapidotnetcore/src/JsonApiDotNetCore/Serialization/ResponseSerializer.cs:line 122
   at string JsonApiDotNetCore.Serialization.ResponseSerializer<TResource>.Serialize(object data) in C:/projects/jsonapidotnetcore/src/JsonApiDotNetCore/Serialization/ResponseSerializer.cs:line 71
   at string JsonApiDotNetCore.Serialization.JsonApiWriter.SerializeResponse(object contextObject, HttpStatusCode statusCode) in C:/projects/jsonapidotnetcore/src/JsonApiDotNetCore/Serialization/JsonApiWriter.cs:line 97
   at async Task JsonApiDotNetCore.Serialization.JsonApiWriter.WriteAsync(OutputFormatterWriteContext context) in C:/projects/jsonapidotnetcore/src/JsonApiDotNetCore/Serialization/JsonApiWriter.cs:line 55

By startup config:

services.AddDbContext<FabrikamContext>(options =>
{
    options.UseSqlServer("Data Source=localhost;User ID=sa;Password=password;Initial Catalog=master");
});

services.AddJsonApi<FabrikamContext>(options: options => options.AllowClientGeneratedIds = true, discovery: discovery => discovery.AddCurrentAssembly());

and, the database I use is SQL Server 2017. Thanks for the quick response.

[beonami]

Has the empty string issue been fixed? When is the next beta scheduled?

[bart-degreed]

Has the empty string issue been fixed?

I did some StringId related changes in 1cc2278 (part of #874) while working on something else. These may unblock your empty string problem, but I haven't tried.

Note though that empty string IDs are unlikely to work properly in general. For example, in URLs there is no distinction between empty string and missing. So you cannot fetch a row by ID when that ID is an empty string. And when creating a resource using a client-generated ID that is an empty string, the returned Location header is wrong (it contains no ID value, so it points to a collection endpoint: /articles/ instead of /articles/1). I don't see how we can solve these.