Skip to content
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

Use @SerializableField properties in Query.parseRow #98

Open
BenVercammen opened this issue Mar 21, 2023 · 2 comments
Open

Use @SerializableField properties in Query.parseRow #98

BenVercammen opened this issue Mar 21, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@BenVercammen
Copy link
Contributor

Consider the following classes:

/// Enumeration class requiring custom (de)serialization
/// We want it to be persisted as 'R', 'G', 'B' instead of 0, 1, 2
enum Color {
  red('R'),
  green('G'),
  blue('B');
  final String code;
  const Color(this.code);
}

/// The class containing the custom (de)serializable enum
@orm
@serializable
abstract class _HasCar extends Model {
  // ...

  /// Our enum field with custom (de)serializers
  @SerializableField(
    serializesTo: String,
    serializer: #colorToCode,
    deserializer: #codeToColor,
  )
  Color? color;
}

/// The deserializer method
Color? codeToColor(String? code) => code == null
    ? null
    : Color.values.firstWhere((color) => color.code == code);

/// The serializer method
String? colorToCode(Color? color) => color?.code;

Currently, the generated HasCarQuery class will have a parseRow method that looks something like this:

  Optional<HasCar> parseRow(List row) {
    ...
    var model = HasCar(
      ...
      color: fields.contains('color')
          ? row[3] == null
              ? null
              : Color.values[(row[3] as int)]  // <- results in type cast exception
          : null,
      ...
    );
    return Optional.of(model);
  }

The generated HasCarSerializer will (de)serialize using the defined colorToCode(model.color) and codeToColor(map['color']) methods. This results in the value being persisted in the databse as a String, but since the enum handler just assumes that the indeces are being persisted, it will try to read that value as an int, resulting in a type cast exception.

Therefore, I'd like to propose that the OrmGenerator will also take the following @SerializableField properties into account:

  • serializer
  • deserializer
  • serializesTo

Any thoughts on this? I'll submit a PR as soon as possible...

@dukefirehawk
Copy link
Collaborator

I can see use cases for custom serialization. Good feature to have.

BenVercammen added a commit to BenVercammen/angel that referenced this issue Mar 22, 2023
BenVercammen added a commit to BenVercammen/angel that referenced this issue Mar 22, 2023
@BenVercammen
Copy link
Contributor Author

A couple of points to note:

  • I used conventional commit messages, which works well in combination with melos, though I've noticed that previous commits don't follow those conventions, so you may want to reword the commit messages for consistency
  • I've added a "workspace level" pubspec.yaml file, as this is required when using melos:3.0 (don't think melos:2.x cares...)
  • I've (temporarily) added some dependency_overrides to get the tests working on my local machine
  • I've added a stand-alone unit test (enum_test.dart) as I wanted to validate my issue. Not sure if that's the best place to do so though, so you may want to look into that one as well...

@dukefirehawk dukefirehawk added the bug Something isn't working label Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants