supabase-community / realtime-csharp

A C# client library for supabase/realtime.
https://supabase-community.github.io/realtime-csharp/api/Supabase.Realtime.Client.html
MIT License
70 stars 12 forks source link

Timestamps for `Payload.Model<T>()` are not parsed correctly. #6

Closed acupofjose closed 3 years ago

acupofjose commented 3 years ago

While attempting to fix, noticed this as an issue: https://github.com/supabase/realtime/issues/114

commit_timestamps will be deserialized properly however Model timestamps will not. There seems to be a slight difference in what realtime returns over what postgrest would.

elrhomariyounes commented 3 years ago

Hello @acupofjose, I think the issue is from the C# property InsertedAt in the Model.

In the database schema the inserted_at column has a default value if it's not null in our case when we perform an Insert the property InsertedAt will not be null it will have this value {01/01/0001 00:00:00}.

So the user should be aware of that if some c# objects have default values like DateTime he should add the corresponding default value that he wants on the Model for his column, in our example case I solved by adding :

[Column("inserted_at")]
public DateTime InsertedAt { get; set; } = DateTime.UtcNow;

and after calling the Model() method on the SocketResponse the timestamp value is deserialized correctly.

acupofjose commented 3 years ago

@elrhomariyounes If I'm understanding correctly, it seems you end up with a default DateTime.UtcNow regardless.

Take the following test case:

[TestMethod("Channel: Payload Model parses a proper timestamp")]
public async Task ChannelPayloadModelParsesTimestamp()
{
    var tsc = new TaskCompletionSource<bool>();

    var timestamp = DateTime.UtcNow;

    var channel = SocketClient.Channel("realtime", "public", "todos");

    channel.OnInsert += (object sender, SocketResponseEventArgs e) =>
    {
        var model = e.Response.Model<Todo>();
        Debug.WriteLine($"Are {timestamp.ToLongTimeString()} and {model.InsertedAt.ToLongTimeString()} equal?");
        tsc.SetResult(timestamp.Equals(model.InsertedAt));
    };

    await channel.Subscribe();

    await Task.Delay(5000);

    await RestClient.Table<Todo>().Insert(new Todo { 
      UserId = 1, 
      Details = "Client Models a timestamp response? ✅", 
      InsertedAt = timestamp 
    });

    var result = await tsc.Task;
    Assert.IsTrue(result);
}

Using the model:

namespace RealtimeTests.Models
{
    [Table("todos")]
    public class Todo : BaseModel
    {
        [PrimaryKey("id", false)]
        public int Id { get; set; }

        [Column("details")]
        public string Details { get; set; }

        [Column("user_id")]
        public int UserId { get; set; }

        [Column("inserted_at")]
        public DateTime InsertedAt { get; set; } = DateTime.UtcNow;
    }
}

The application will output:

Are 20:35:54 and 20:35:59 equal?

and Postgrest (http://localhost:3000/todos) outputs:

{
"id": 7,
"details": "Client Models a timestamp  response? ✅",
"user_id": 1,
"inserted_at": "2021-02-07T20:35:54.261575"
}

They have a difference of exactly 5s from the Task.Delay(5000) which means it's pulling the default value rather than parsing it.

To me, this seems to be from an error parsing the timestamp?

elrhomariyounes commented 3 years ago

@acupofjose I think this test will always fail because there is a time difference between this two timestamps the task take some time to complete then Ticks property will be different even without the Delay. In my local project the InsertedAt property is parsed correctly when I added the default value DateTime.UtcNow

acupofjose commented 3 years ago

@elrhomariyounes hm. I think you're right, the Equals is the wrong operator here. However, the issue (unless I'm missing something) is that the timestamp declared as the data for Todo.InsertedAt does not match when it is sent back by realtime. Instead it resolves to the default DateTime.UtcNow when the Model is constructed.

Are you saying it does not do that locally for you? If not, I'm at a loss as to what the issue is 🤕

elrhomariyounes commented 3 years ago

@acupofjose I made some changes in the previous test case and I added DateTime.UtcNow as default value for the property InsertedAt after debugging I got the results below for SocketResponse and the Model after building it from Record, the are not matching I'm sorry you are right.

As you said when calling the method Model() at the construction of the object it takes the default value that we gave, in this case is DateTime.UtcNow.

[Table("todos")]
    public class Todo : BaseModel
    {
        [PrimaryKey("id", false)]
        public int Id { get; set; }

        [Column("details")]
        public string Details { get; set; }

        [Column("user_id")]
        public int UserId { get; set; }

        [Column("inserted_at")]
        public DateTime InsertedAt { get; set; } = DateTime.UtcNow;
[TestMethod("Channel: Payload Model parses a proper timestamp")]
        public async Task ChannelPayloadModelParsesTimestamp()
        {
            var tsc = new TaskCompletionSource<bool>();

            var channel = SocketClient.Channel("realtime", "public", "todos");

            channel.OnInsert += (object sender, SocketResponseEventArgs e) =>
            {
                var model = e.Response.Model<Todo>();
                //var timestamp = DateTime.UtcNow;
                //Debug.WriteLine($"Are {timestamp.ToLongTimeString()} and {model.InsertedAt.ToLongTimeString()} equal?");
                //var equal = e.Response.Payload.CommitTimestamp.UtcDateTime == model.InsertedAt;
                tsc.SetResult(model is Todo);
            };

            await channel.Subscribe();
            //await Task.Delay(5000);

            await RestClient.Table<Todo>().Insert(new Todo { UserId = 1, Details = "Client Models a timestamp response? ✅" });

            var result = await tsc.Task;
            Assert.IsTrue(result);
        }

After debugging the value of Response object received from the server :

{
  "topic": "realtime:public:todos",
  "event": "INSERT",
  "payload": {
    "columns": [
      {
        "flags": [
          "key"
        ],
        "name": "id",
        "type": "int8",
        "type_modifier": 4294967295
      },
      {
        "flags": [],
        "name": "details",
        "type": "text",
        "type_modifier": 4294967295
      },
      {
        "flags": [],
        "name": "user_id",
        "type": "int8",
        "type_modifier": 4294967295
      },
      {
        "flags": [],
        "name": "inserted_at",
        "type": "timestamp",
        "type_modifier": 4294967295
      }
    ],
    "commit_timestamp": "2021-02-08T02:50:18+00:00",
    "record": {
      "details": "Client Models a timestamp response? ✅",
      "id": "47",
      "inserted_at": "2021-02-08 02:50:17.901382",
      "user_id": "1"
    },
    "schema": "public",
    "table": "todos",
    "type": "INSERT",
    "status": null,
    "response": null
  },
  "ref": null
}

And the modelJson :

{
  "Id": 47,
  "Details": "Client Models a timestamp response? ✅",
  "UserId": 0,
  "InsertedAt": "2021-02-08T02:50:18.1239832Z"
}
acupofjose commented 3 years ago

@elrhomariyounes perfect! No worries. That’s exactly the problem I’m having. At the very least we’ve confirmed it! Waiting to hear from the parent repo

acupofjose commented 3 years ago

Closed by https://github.com/supabase/realtime/pull/124