mmvergara / supadart

Typesafe queries in Supabase Flutter! Generate Flutter / Dart 🎯 classes from your Supabase schema.
https://supadart.vercel.app
MIT License
43 stars 5 forks source link

toJson method compatibility issue with the fromJson method #66

Closed Achraf-cyber closed 2 months ago

Achraf-cyber commented 2 months ago

The toJson method that has been added is very good. But I think it will be better if the output of the toJson was the expected input of the fromJson method. For example the toJson does not convert DateTime fields into String (It just keep it as it is), but the fromJson expects Dates to be a String. The toJson method does not convert enums into String but the fromJson method expects enums type to be a String and then uses the Enum.values.byName method to convert the String into an enum. So what I am saying is that it will be nicer if the output of the toJson method was exactly the same as the insert or 'update` method.

mmvergara commented 2 months ago

i see, thanks for these issue's your opening btw it makes this tool much better. will work on it asap, got some school stuff atm

mmvergara commented 2 months ago

@Achraf-cyber Would love your feedback on this progress it currently generates these Books

  Map<String, dynamic> toJson() {
// Promotion doesn't work well with public fields due to the possibility of the field being modified elsewhere.
    final id = this.id;
    final name = this.name;
    final description = this.description;
    final price = this.price;
    final createdAt = this.createdAt;
    return {
      if (id != null) 'id': id.toString(),
      if (name != null) 'name': name.toString(),
      if (description != null) 'description': description.toString(),
      if (price != null) 'price': price.toString(),
      if (createdAt != null) 'created_at': createdAt.toUtc().toString()
    };
  }

This one is with enums

 Map<String, dynamic> toJson() {
// Promotion doesn't work well with public fields due to the possibility of the field being modified elsewhere.
    final id = this.id;
    final colMood = this.colMood;
    return {
      if (id != null) 'id': id.toString(),
      if (colMood != null) 'col_mood': colMood.toString().split('.').last
    };
  }

DateTime types

   Map<String, dynamic> toJson() {
// Promotion doesn't work well with public fields due to the possibility of the field being modified elsewhere.
    final id = this.id;
    final colDate = this.colDate;
    final colDateArray = this.colDateArray;
    final colTime = this.colTime;
    final colTimeArray = this.colTimeArray;
    final colTimetz = this.colTimetz;
    final colTimetzArray = this.colTimetzArray;
    final colTimestamp = this.colTimestamp;
    final colTimestampArray = this.colTimestampArray;
    final colTimestamptz = this.colTimestamptz;
    final colTimestamptzArray = this.colTimestamptzArray;
    final colInterval = this.colInterval;
    final colIntervalArray = this.colIntervalArray;
    return {
      if (id != null) 'id': id.toString(),
      if (colDate != null) 'col_date': colDate.toIso8601String(),
      if (colDateArray != null)
        'col_date_array':
            colDateArray.toString().replaceAll("[", "{").replaceAll("]", "}"),
      if (colTime != null)
        'col_time': DateFormat('HH:mm:ss.SSS').format(colTime),
      if (colTimeArray != null)
        'col_time_array':
            colTimeArray.toString().replaceAll("[", "{").replaceAll("]", "}"),
      if (colTimetz != null)
        'col_timetz': DateFormat('HH:mm:ss zzzz').format(colTimetz),
      if (colTimetzArray != null)
        'col_timetz_array':
            colTimetzArray.toString().replaceAll("[", "{").replaceAll("]", "}"),
      if (colTimestamp != null) 'col_timestamp': colTimestamp.toIso8601String(),
      if (colTimestampArray != null)
        'col_timestamp_array': colTimestampArray
            .toString()
            .replaceAll("[", "{")
            .replaceAll("]", "}"),
      if (colTimestamptz != null)
        'col_timestamptz': colTimestamptz.toUtc().toString(),
      if (colTimestamptzArray != null)
        'col_timestamptz_array': colTimestamptzArray
            .toString()
            .replaceAll("[", "{")
            .replaceAll("]", "}"),
      if (colInterval != null) 'col_interval': colInterval.toString(),
      if (colIntervalArray != null)
        'col_interval_array': colIntervalArray
            .toString()
            .replaceAll("[", "{")
            .replaceAll("]", "}")
    };
  }

and im guessing to test this i should just.. fetch table. -> table.toJson -> output -> table.fromJson(output)

mmvergara commented 2 months ago

changes applied in cli 1.5.7, please update the cli and let me know you're stil having problems.

Achraf-cyber commented 2 months ago

It works perfectly fine but for the enum I think the package could just col.name instead of `col.toString().split('.").last. For example


// Promotion doesn't work well with public fields due to the possibility of the field being modified elsewhere.
    final id = this.id;
    final colMood = this.colMood;
    return {
      if (id != null) 'id': id.toString(),
      if (colMood != null) 'col_mood': colMood.name
    };
  }```
mmvergara commented 2 months ago

We'll leave it like that for now, i just started to see that we can combine update,insert,toJson into one function.

Achraf-cyber commented 2 months ago

Ok. Thanks

Achraf-cyber commented 2 months ago

There still a little error when it comes to bool type. The toJson method convert them toString. But the fromJson expects boolean to be Strings and not bool. This was fine for the update and insert method because supabase will interpret the bool itself. But for the toJson method, it causes another error

mmvergara commented 2 months ago

There still a little error when it comes to bool type. The toJson method convert them toString. But the fromJson expects boolean to be Strings and not bool. This was fine for the update and insert method because supabase will interpret the bool itself. But for the toJson method, it causes another error

@Achraf-cyber should be fix in the new update 1.5.8. along with the new structure https://github.com/mmvergara/supadart/pull/69

mmvergara commented 2 months ago

Im gonna close this issue, will open if there are more problems

Achraf-cyber commented 2 months ago

There is another problem with arrays. The problem is that the toJson method encods List as String. But the fromJson method expects List to be List without any transformation. I looked at supabase dart api documentation and the supabase dart api also expects List to stay as it is without any encoding. So I think it will be nicer to keep List as it is when converting to json.

mmvergara commented 2 months ago

if without encoding i don't understand why this doesn't work

 // List of DateTimes with assumed timezones
  List<DateTime> insertTimestamptzes = [
    DateTime(2021, 10, 10, 10, 10, 10).toUtc(),
    DateTime.now(),
  ];

  // Tests for Timestamptz array
  test('Testing Timestamptz Array Create', () async {
    await cleanup(supabase, supabase.datetime_types);
    var createResult =
        await createTimestamptzArray(supabase, insertTimestamptzes);
    expect(createResult, null);
  });
test/supadart_test.dart: Testing Timestamptz Array Create [E]                                                                                             
  Expected: <null>
    Actual: JsonUnsupportedObjectError:<Converting object to an encodable object failed: Instance of 'DateTime'>

  package:matcher                                      expect
  test/datatypes/datetime/timestamptz_array.dart 26:5  performTimestamptzArrayTest.<fn>

you get errrors like this when you don't convert them to string when inserting as an array.

Im trying to find a better solution

Achraf-cyber commented 2 months ago

Now I understand the problem. It is like every object in the array needs to be encoded itself.

Le mar. 3 sept. 2024, 02:29, mmvergara @.***> a écrit :

if without encoding i don't understand why this doesn't work

// List of DateTimes with assumed timezones List insertTimestamptzes = [ DateTime(2021, 10, 10, 10, 10, 10).toUtc(), DateTime.now(), ];

// Tests for Timestamptz array test('Testing Timestamptz Array Create', () async { await cleanup(supabase, supabase.datetime_types); var createResult = await createTimestamptzArray(supabase, insertTimestamptzes); expect(createResult, null); });

test/supadart_test.dart: Testing Timestamptz Array Create [E] Expected: Actual: JsonUnsupportedObjectError:<Converting object to an encodable object failed: Instance of 'DateTime'>

package:matcher expect test/datatypes/datetime/timestamptz_array.dart 26:5 performTimestamptzArrayTest.

you get errrors like this when you don't convert them to string when inserting as an array.

Im trying to find a better solution

— Reply to this email directly, view it on GitHub https://github.com/mmvergara/supadart/issues/66#issuecomment-2325444990, or unsubscribe https://github.com/notifications/unsubscribe-auth/AUFBBDQXIP56MXCIHVVFTBDZUUGH7AVCNFSM6AAAAABNIARK7GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRVGQ2DIOJZGA . You are receiving this because you were mentioned.Message ID: @.***>

mmvergara commented 2 months ago

yeah. im kinda lost at this point on what is the right approach.

we can make a toJson that is compatible with fromJson. we can make a toJson that is compatible with supabase client operations.

mmvergara commented 2 months ago

@Achraf-cyber cli bumped to 1.5.9 toJson and fromJson are now compatible roundtrip, nearly blew my brains out after 3 days.

I made some test for EVERY supadart supported types, they look like this

  test(
      "Testing UUID serialization roundtrip maintains data integrity and object equivalence",
      () async {
    var readResult = await readUUID(supabase);
    expect(readResult, isNotNull);
    expect(readResult!.isNotEmpty, true);

    var originalObject = readResult[0];

    // Test toJson() followed by fromJson()
    var toJson = originalObject.toJson();
    var fromJson = StringTypes.fromJson(toJson);
    expect(fromJson.colUuid, originalObject.colUuid);

    // Test full roundtrip and object equivalence
    var roundTripJson = fromJson.toJson();
    expect(roundTripJson, originalObject.toJson());
  });

please let me know what you think about it or if there some issues. thanks again

Achraf-cyber commented 2 months ago

Congraculations. I think it works perfectly now.