dart-backend / angel

A polished, production-ready backend framework in Dart for the VM, AOT, and Flutter.
https://github.com/dukefirehawk/angel
BSD 3-Clause "New" or "Revised" License
171 stars 22 forks source link

orm generator not supporting non-nullable types? #68

Closed 3rpse-ai closed 1 year ago

3rpse-ai commented 2 years ago

Hi - I tried to make use of your orm package and after investigating a bit I have to say I really like the syntax!

However I face an issue (yet I am not sure if it is a bug or I am missing some info).

I followed your car example but the generated code does not seem to be handling the non-nullable types.

The problem is that the parseRow method in the CarQuery class, as well as the fromMap method in the CarSerializer class throw errors, due to trying to assign potential null values.

So my question being: do I miss anything? Should I just make everything nullable?

Besides that, for your info 2 more remarks regarding the package:

  1. If I do not use the @serializable annotation (hence the respective packages) the code generation does not work in this example. (Which is kinda understandable as it is generating the referenced class). Is there a way to use the package without serializable?
  2. I did not find any explanation that I need to add the optional package for the generated code to work. Might be nice to just include it in the orm package?

My car file:

import 'package:angel3_orm/angel3_orm.dart';
import 'package:angel3_serialize/angel3_serialize.dart';
import 'package:angel3_migration/angel3_migration.dart';
import 'package:optional/optional.dart';

part 'car.g.dart';

@serializable
@orm
abstract class _Car extends Model{
  String get make;

  String get description;

  bool get familiyFriendly;

  DateTime get recalledAt;
}

The generated file

// GENERATED CODE - DO NOT MODIFY BY HAND

part of 'car.dart';

// **************************************************************************
// MigrationGenerator
// **************************************************************************

class CarMigration extends Migration {
  @override
  void up(Schema schema) {
    schema.create('cars', (table) {
      table.serial('id').primaryKey();
      table.timeStamp('created_at');
      table.timeStamp('updated_at');
      table.varChar('make', length: 255);
      table.varChar('description', length: 255);
      table.boolean('familiy_friendly');
      table.timeStamp('recalled_at');
    });
  }

  @override
  void down(Schema schema) {
    schema.drop('cars');
  }
}

// **************************************************************************
// OrmGenerator
// **************************************************************************

class CarQuery extends Query<Car, CarQueryWhere> {
  CarQuery({Query? parent, Set<String>? trampoline}) : super(parent: parent) {
    trampoline ??= <String>{};
    trampoline.add(tableName);
    _where = CarQueryWhere(this);
  }

  @override
  final CarQueryValues values = CarQueryValues();

  List<String> _selectedFields = [];

  CarQueryWhere? _where;

  @override
  Map<String, String> get casts {
    return {};
  }

  @override
  String get tableName {
    return 'cars';
  }

  @override
  List<String> get fields {
    const _fields = [
      'id',
      'created_at',
      'updated_at',
      'make',
      'description',
      'familiy_friendly',
      'recalled_at'
    ];
    return _selectedFields.isEmpty
        ? _fields
        : _fields.where((field) => _selectedFields.contains(field)).toList();
  }

  CarQuery select(List<String> selectedFields) {
    _selectedFields = selectedFields;
    return this;
  }

  @override
  CarQueryWhere? get where {
    return _where;
  }

  @override
  CarQueryWhere newWhereClause() {
    return CarQueryWhere(this);
  }

  Optional<Car> parseRow(List row) {
    if (row.every((x) => x == null)) {
      return Optional.empty();
    }
    var model = Car(
        id: fields.contains('id') ? row[0].toString() : null,
        createdAt: fields.contains('created_at') ? mapToDateTime(row[1]) : null,
        updatedAt: fields.contains('updated_at') ? mapToDateTime(row[2]) : null,
        make: fields.contains('make') ? (row[3] as String) : null,
        description: fields.contains('description') ? (row[4] as String) : null,
        familiyFriendly:
            fields.contains('familiy_friendly') ? mapToBool(row[5]) : null,
        recalledAt:
            fields.contains('recalled_at') ? mapToDateTime(row[6]) : null);
    return Optional.of(model);
  }

  @override
  Optional<Car> deserialize(List row) {
    return parseRow(row);
  }
}

class CarQueryWhere extends QueryWhere {
  CarQueryWhere(CarQuery query)
      : id = NumericSqlExpressionBuilder<int>(query, 'id'),
        createdAt = DateTimeSqlExpressionBuilder(query, 'created_at'),
        updatedAt = DateTimeSqlExpressionBuilder(query, 'updated_at'),
        make = StringSqlExpressionBuilder(query, 'make'),
        description = StringSqlExpressionBuilder(query, 'description'),
        familiyFriendly =
            BooleanSqlExpressionBuilder(query, 'familiy_friendly'),
        recalledAt = DateTimeSqlExpressionBuilder(query, 'recalled_at');

  final NumericSqlExpressionBuilder<int> id;

  final DateTimeSqlExpressionBuilder createdAt;

  final DateTimeSqlExpressionBuilder updatedAt;

  final StringSqlExpressionBuilder make;

  final StringSqlExpressionBuilder description;

  final BooleanSqlExpressionBuilder familiyFriendly;

  final DateTimeSqlExpressionBuilder recalledAt;

  @override
  List<SqlExpressionBuilder> get expressionBuilders {
    return [
      id,
      createdAt,
      updatedAt,
      make,
      description,
      familiyFriendly,
      recalledAt
    ];
  }
}

class CarQueryValues extends MapQueryValues {
  @override
  Map<String, String> get casts {
    return {};
  }

  String? get id {
    return (values['id'] as String?);
  }

  set id(String? value) => values['id'] = value;
  DateTime? get createdAt {
    return (values['created_at'] as DateTime?);
  }

  set createdAt(DateTime? value) => values['created_at'] = value;
  DateTime? get updatedAt {
    return (values['updated_at'] as DateTime?);
  }

  set updatedAt(DateTime? value) => values['updated_at'] = value;
  String get make {
    return (values['make'] as String);
  }

  set make(String value) => values['make'] = value;
  String get description {
    return (values['description'] as String);
  }

  set description(String value) => values['description'] = value;
  bool get familiyFriendly {
    return (values['familiy_friendly'] as bool);
  }

  set familiyFriendly(bool value) => values['familiy_friendly'] = value;
  DateTime get recalledAt {
    return (values['recalled_at'] as DateTime);
  }

  set recalledAt(DateTime value) => values['recalled_at'] = value;
  void copyFrom(Car model) {
    createdAt = model.createdAt;
    updatedAt = model.updatedAt;
    make = model.make;
    description = model.description;
    familiyFriendly = model.familiyFriendly;
    recalledAt = model.recalledAt;
  }
}

// **************************************************************************
// JsonModelGenerator
// **************************************************************************

@generatedSerializable
class Car extends _Car {
  Car(
      {this.id,
      this.createdAt,
      this.updatedAt,
      required this.make,
      required this.description,
      required this.familiyFriendly,
      required this.recalledAt});

  /// A unique identifier corresponding to this item.
  @override
  String? id;

  /// The time at which this item was created.
  @override
  DateTime? createdAt;

  /// The last time at which this item was updated.
  @override
  DateTime? updatedAt;

  @override
  String make;

  @override
  String description;

  @override
  bool familiyFriendly;

  @override
  DateTime recalledAt;

  Car copyWith(
      {String? id,
      DateTime? createdAt,
      DateTime? updatedAt,
      String? make,
      String? description,
      bool? familiyFriendly,
      DateTime? recalledAt}) {
    return Car(
        id: id ?? this.id,
        createdAt: createdAt ?? this.createdAt,
        updatedAt: updatedAt ?? this.updatedAt,
        make: make ?? this.make,
        description: description ?? this.description,
        familiyFriendly: familiyFriendly ?? this.familiyFriendly,
        recalledAt: recalledAt ?? this.recalledAt);
  }

  @override
  bool operator ==(other) {
    return other is _Car &&
        other.id == id &&
        other.createdAt == createdAt &&
        other.updatedAt == updatedAt &&
        other.make == make &&
        other.description == description &&
        other.familiyFriendly == familiyFriendly &&
        other.recalledAt == recalledAt;
  }

  @override
  int get hashCode {
    return hashObjects([
      id,
      createdAt,
      updatedAt,
      make,
      description,
      familiyFriendly,
      recalledAt
    ]);
  }

  @override
  String toString() {
    return 'Car(id=$id, createdAt=$createdAt, updatedAt=$updatedAt, make=$make, description=$description, familiyFriendly=$familiyFriendly, recalledAt=$recalledAt)';
  }

  Map<String, dynamic> toJson() {
    return CarSerializer.toMap(this);
  }
}

// **************************************************************************
// SerializerGenerator
// **************************************************************************

const CarSerializer carSerializer = CarSerializer();

class CarEncoder extends Converter<Car, Map> {
  const CarEncoder();

  @override
  Map convert(Car model) => CarSerializer.toMap(model);
}

class CarDecoder extends Converter<Map, Car> {
  const CarDecoder();

  @override
  Car convert(Map map) => CarSerializer.fromMap(map);
}

class CarSerializer extends Codec<Car, Map> {
  const CarSerializer();

  @override
  CarEncoder get encoder => const CarEncoder();
  @override
  CarDecoder get decoder => const CarDecoder();
  static Car fromMap(Map map) {
    return Car(
        id: map['id'] as String?,
        createdAt: map['created_at'] != null
            ? (map['created_at'] is DateTime
                ? (map['created_at'] as DateTime)
                : DateTime.parse(map['created_at'].toString()))
            : null,
        updatedAt: map['updated_at'] != null
            ? (map['updated_at'] is DateTime
                ? (map['updated_at'] as DateTime)
                : DateTime.parse(map['updated_at'].toString()))
            : null,
        make: map['make'] as String,
        description: map['description'] as String,
        familiyFriendly: map['familiy_friendly'] as bool,
        recalledAt: map['recalled_at'] != null
            ? (map['recalled_at'] is DateTime
                ? (map['recalled_at'] as DateTime)
                : DateTime.parse(map['recalled_at'].toString()))
            : null);
  }

  static Map<String, dynamic> toMap(_Car? model) {
    if (model == null) {
      throw FormatException("Required field [model] cannot be null");
    }
    return {
      'id': model.id,
      'created_at': model.createdAt?.toIso8601String(),
      'updated_at': model.updatedAt?.toIso8601String(),
      'make': model.make,
      'description': model.description,
      'familiy_friendly': model.familiyFriendly,
      'recalled_at': model.recalledAt?.toIso8601String()
    };
  }
}

abstract class CarFields {
  static const List<String> allFields = <String>[
    id,
    createdAt,
    updatedAt,
    make,
    description,
    familiyFriendly,
    recalledAt
  ];

  static const String id = 'id';

  static const String createdAt = 'created_at';

  static const String updatedAt = 'updated_at';

  static const String make = 'make';

  static const String description = 'description';

  static const String familiyFriendly = 'familiy_friendly';

  static const String recalledAt = 'recalled_at';
}
dukefirehawk commented 2 years ago

Glad you like it.

The code should be able to support non-nullable types. I believe it could be a bug and will look into it. Anyway, for ORM nullable type generally works better at the moment.

  1. Currently @serializable is required. ORM generated code depends on it.

  2. Will add the Optional package. This was added as part of upgrading Angel3 ORM to support null safety. It was used primary to return either 0 or 1 record. Returning Optional object that wrap the result avoid the use of null to represent no record found.

3rpse-ai commented 2 years ago

Thanks for the quick reply! Just out of curiosity: What is the benefit of using Optional over just working with null in this context?

dukefirehawk commented 2 years ago

Cleaner code. It allows parseRow() to return non-nullable type instead nullable type.

atreeon commented 2 years ago

I'm just trying Angel & the ORM out. I ran the builder on the book model from the documentation here https://angel3-docs.dukefirehawk.com/guides/serialization

  BookType get type;

I too get an error (on the fromMap method)

        type: map['type'] is BookType
            ? (map['type'] as BookType) ?? null
            : (map['type'] is int
                ? BookType.values[map['type'] as int]
                : null));

the type property is not nullable but the generated code is treating it like it should be. It seems the bug is in the generator code somewhere.

dukefirehawk commented 2 years ago

These ORM null safety bugs are being addressed on bug-fix/orm-generator branch. 3 new models have been added to angel_orm_test package to cater to these use cases; asset.dart, boat.dart and bike.dart. DateTime type is also undergoing changes to support timezone properly. Support for precision and scale in numeric field is also being worked on.

dukefirehawk commented 2 years ago

For this use case, there are two ways of fixing it. First option is to return a default BookType instance and second option is to use bang operator '!' since map can potentially return null. My preference is 2nd option as throwing exception on null assignment to non-nullable field seems more logical. This is similar issue with non nullable temporal field assignment. However, for temporal field, epoch time, 1970-01-01 00:00:00, can be used whenever null is encountered.

dukefirehawk commented 1 year ago

Resolved in angel3_orm_generator 6.2.0