realm / realm-dart

Realm is a mobile database: a replacement for SQLite & ORMs.
Apache License 2.0
755 stars 85 forks source link

Enum support #681

Open n-doton opened 2 years ago

n-doton commented 2 years ago

Is enum support planned? As far as I can tell, its currently not a supported type. https://pub.dev/documentation/realm_common/latest/realm_common/RealmPropertyType.html

Thanks in advance!

lukaspili commented 2 years ago

See #579. Enhanced enum should make this trivial to support (from/to a string value internally)

desistefanova commented 2 years ago

Right, enums are not currently supported, but the value is always stored as int (or whatever is the type of the enum value) in the realm. But, yes for sure, we can consider supporting enums at least in Dart RealmModel and RealmModel generator.

dotjon0 commented 2 years ago

We use enums throughout our Flutter applications as it really helps to deliver fault tolerant applications and also provides a great developer readability experience. As MongoDB Atlas supports enums out of the box, it also makes a tone of sense to support enums in Flutter/Dart also. Will look forward to updates on this.

dotjon0 commented 2 years ago

@desistefanova some further thoughts if we may;

In MongoDB it's common for enum values to be in Snake Case (All Caps) i.e. USER_IS_ADMIN. In Flutter enums are in camelCase i.e. userIsAdmin. Would the Realm team consider putting in 'automatic case conversion' for enum 'values' (i.e. that are passed in via Realm Sync)? Perhaps with a similar approach to Google's json_serializable package via Build Configurations https://pub.dev/packages/json_serializable#build-configuration - note we have a related issue (auto case conversion for Flutter model/class properties) feature request at https://github.com/realm/realm-dart/issues/703 (CC @nielsenko ). If something like this is not included with Flutter Realm enum support we can not see how enums could work, unless the enums are stored in camelCase in MongoDB Atlas...

Many thanks again!

hlazarpesic commented 1 year ago

Any update here? Can we use enums in RealmModels?

dotjon0 commented 1 year ago

Just checking in to see if enums are planned?

hlazarpesic commented 1 year ago

@desistefanova ☝️🙂

desistefanova commented 1 year ago

Supporting enums is in our list, but I can only say that it is not planned to be released soon. There are many others tasks before it.

hlazarpesic commented 1 year ago

Thanks for answer @desistefanova

dotjon0 commented 10 months ago

Any updates for when this may land?

nirinchev commented 10 months ago

No - this requires core support which is tracked in this issue: https://github.com/realm/realm-core/issues/6159

dotjon0 commented 4 months ago

@nirinchev and @nielsenko thanks for private properties, this has made Enum support a bit better. So the getter side is sorted, what would be the best approach be for the setter side please?

@RealmModel()
class _Car {

  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;

}

enum CustomType { enabled, disabled }
nielsenko commented 4 months ago

@dotjon0 The above looks reasonable, but you will have to pass a String to the ctor.

import 'package:realm/realm.dart';

part 'main.realm.dart';

@RealmModel()
class _Car {
  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;
}

enum CustomType { enabled, disabled }

final realm = Realm(Configuration.local([Car.schema]));

void main() {
  realm.write(() {
    // best bet right now ..
    realm.add(Car(CustomType.enabled.name));
    // or
    realm.add(Car('')..type = CustomType.enabled);
  });
}

But I realize there is bug when combining private fields, with either optional values, or named arguments.
The following fx will generate invalid Dart code:

@RealmModel()
class _Car {
  @MapTo("type")
  late int _type = 0; // <-- will surface a bug :-/
  CustomType get type => CustomType.values[_type];
  set type(final CustomType value) => _type = value.index;
}
dotjon0 commented 4 months ago

Thanks @nielsenko and good spot re #1663. Could we not do the below with the 'named route'? If not, what is blocking this?

import 'package:realm/realm.dart';

part 'main.realm.dart';

// Use Realm Model Named
const GeneratorConfig _config = GeneratorConfig(ctorStyle: CtorStyle.allNamed);
const RealmModel realmModelNamed = RealmModel.using(
    baseType: ObjectType.realmObject,
    generatorConfig: _config,
);

@RealmModel()
class _Car {
  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;
}

enum CustomType { enabled, disabled }

final realm = Realm(Configuration.local([Car.schema]));

void main() {
  realm.write(() {
    realm.add(Car(
        type: CustomType.enabled,
    ));
  });
}
nielsenko commented 4 months ago

Problem is:

void main() {
  realm.write(() {
    realm.add(Car(
        type: CustomType.enabled, // <-- this is not a String, you need to add .name to the end
    ));
  });
}

Also, you will hit another variant of the bug fixed by #1664, since the argument is named (not because of the default value, but because of the explicit request for named arguments).

dotjon0 commented 4 months ago

Problem is:

void main() {
  realm.write(() {
    realm.add(Car(
        type: CustomType.enabled, // <-- this is not a String, you need to add .name to the end
    ));
  });
}

Also, you will hit another variant of the bug fixed by #1664, since the argument is named (not because of the default value, but because of the explicit request for named arguments).

ok. So cant we override the setter, ie so the setter converts the enum into a String before hitting the Realm db?

set type(final CustomType value) => _type = value.name;
nielsenko commented 4 months ago

@dotjon0 The realm generator doesn't know the relation between the field _type and the property getter/setter pair type. While there may exist a convention for how such a relation-ship should look, there is no guarantee they are actually related at all.

What the generator does is just realizing that type is not a field, so it disregards looking further at it, but that _type is and (since ^2.1.0) allows that private field to be stored.

Hence you need to use the type of _type (String) in the ctor call and cannot use the enum type of type (CustomType). Sorry for the confusion the overloaded use of the word type may cause, but it wasn't I who named these 😉.

You can improve the DX a bit, by using default values once we release #1663. Then you will be able to write:

@RealmModel()
class _Car {
  @MapTo("type")
  late String _type = CustomType.enabled.name; // or whatever default you prefer
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;
}
// ...
void main() {
  realm.write(() {
    realm.add(Car()); // car with type field set to default value, ie. enabled
    realm.add(Car()..type = CustomType.enabled); // explicitly set type
  });
}

or

@RealmModel()
class _Car {
  @MapTo("type")
  late int _type = 0;
  CustomType get type => CustomType.values[_type];
  set type(final CustomType value) => _type = value.index;
}

The first is safer, as it is a lot easier to inadvertently change the index value of an enum, than its name.

dotjon0 commented 4 months ago

Appreciate the detailed answer @nielsenko.

So is there a way for the below to be overcome?

"The realm generator doesn't know the relation between the field _type and the property getter/setter pair type."

nielsenko commented 4 months ago

@dotjon0 Not currently. We need some way of registering conversion functions so that you can use the custom type at the field level, and have the generator write the code that invokes the conversions.

dotjon0 commented 4 months ago

@dotjon0 Not currently. We need some way of registering conversion functions so that you can use the custom type at the field level, and have the generator write the code that invokes the conversions.

Unless others think otherwise:

Having this will mean flutter 'enums' are fully supported by Realm Flutter SDK (unless people think otherwise) - as this gives the ability to use an enum to get/set RealmModel properties. Having the actual enum value stored as a String is perfect, as this is how MongoDB Atlas works (enums' values are stored as strings) - so it will support both Device Sync and Local Only Realm consumers. Believe there is no need for anything in Realm Core for this, so its purely a Realm Flutter SDK feature request with no dependencies. This is perhaps how it will look (as copied from above):

import 'package:realm/realm.dart';

part 'main.realm.dart';

// Use Realm Model Named
const GeneratorConfig _config = GeneratorConfig(ctorStyle: CtorStyle.allNamed);
const RealmModel realmModelNamed = RealmModel.using(
    baseType: ObjectType.realmObject,
    generatorConfig: _config,
);

@RealmModel()
class _Car {
  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;
}

enum CustomType { enabled, disabled }

final realm = Realm(Configuration.local([Car.schema]));

void main() {
  realm.write(() {
    realm.add(Car(
        type: CustomType.enabled,
    ));
  });
}

Of course a two liner for the @RealmModel would be even more desirable, although the above solves it with not many lines:

// The approach above
@RealmModel()
class _Car {
  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name;
}

// Better approach
@RealmModel()
class _Car {
   @RealmEnum()
   late CustomType type;
}
nirinchev commented 4 months ago

The problem with an SDK-only approach is that it breaks some fundamental assumptions that Realm is built on - like a property getter will never throw. Since Realm and Atlas don't enforce the valid enum values, there's nothing preventing someone from adding an Atlas document with type: "crash-me", which will then result in exceptions being thrown when the property is read on the client. This is why we're overly cautious about adding what appears to be first-party support for enums - for someone not familiar with the relationship between Realm Core, Device Sync, and Atlas, it may appear like they can safely use the feature, when in reality, someone can break all their users by inserting an invalid enum value.

While we understand it's some extra work and somewhat annoying to have to manually do stuff like that, we have historically taken a conservative approach when designing our public API to have a good mix of usability and safety. While I don't expect us to add built-in support for enums before we're ready to commit to it across the entire stack, we'll be looking to improve the developer experience of customizing the models for dart - e.g. by allowing developers to ignore fields from the generated constructors or by allowing them to supply their own constructors. We don't have a timeframe for when such improvements could land, but we appreciate the feedback and take it into consideration during our planning meetings.

dotjon0 commented 4 months ago

Thanks for this and for more context.

MongoDB Atlas has supported enforcing enum values for years via Validation Schema - see the MongoDB Atlas Validation Schema example below where 'type' is an enum.

So right now if we use Realm Flutter SDK we run risk of invalid 'enum' values being passed to MongoDB Atlas database documents' properties (which use 'enums') via MongoDB Device Sync - and then no doubt MongoDB Atlas will reject the DB document create/update as the value passed to MongoDB Atlas failed the model validation schema. The proposed approach via the Realm Flutter SDK above resolves this entirely.

Given MongoDB Atlas enforces/supports enums, does it not make sense to support enums via setters/getters in the Realm Flutter SDK...

// MongoDB Atlas Validation Schema Example with enums
exports.validationSchema = {
  title: "Cars",
  bsonType: "object",
  required: [ "_id", "type" ],
  additionalProperties: false,
  properties: {
    _id: {
      title: "ID",
      bsonType: "objectId" 
    },
    type: {
        title: "Type",
        description: "The type of the Car. Either fast or slow.",
        bsonType: "string",
        enum: [ "fast", "slow" ]
    }
  }
}

So a half way house approach may be allowing Realm Flutter consumers to use custom (a) getters (as you already do) and (b) setters (not supported, needed), as then the responsibility of the getter/setter is with the consumer (and at the same time keeps fundamental assumptions that Realm is built on at play).

import 'package:realm/realm.dart';

part 'main.realm.dart';

// Use Realm Model Named
const GeneratorConfig _config = GeneratorConfig(ctorStyle: CtorStyle.allNamed);
const RealmModel realmModelNamed = RealmModel.using(
    baseType: ObjectType.realmObject,
    generatorConfig: _config,
);

@RealmModel()
class _Car {
  @MapTo("type")
  late String _type;
  CustomType get type => CustomType.values.byName(_type);
  set type(final CustomType value) => _type = value.name; // this is what we need, the missing piece
}

enum CustomType { enabled, disabled }

final realm = Realm(Configuration.local([Car.schema]));

void main() {
  realm.write(() {
    realm.add(Car(
        type: CustomType.enabled,
    ));
  });
}
nirinchev commented 4 months ago

MongoDB Atlas does support enum values, but that is not well supported across the other products involved here and unfortunately, we can't make the assumption that users have defined a json schema on Atlas.

Regarding setters - they do work, the problem is that right now the generated constructor doesn't use the custom property, but rather the private one, which has the incorrect type (i.e. string rather than CustomType). This could be alleviated by what I mentioned above - either allowing you to opt out of certain fields in the generated ctor and opt-in to others or by allowing you to specify a custom constructor:

@RealmModel()
class _Car {
  @MapTo("type")
  @ConstructorOmit()
  late String _type;

  CustomType get type => CustomType.values.byName(_type);

  @ConstructorInclude()
  set type (final CustomType value) => _type = value.name;
}

// this generates
class Car extends _Car {
  Car({ CustomType type }) {
    this.type = type;
  }
}

Or alternatively, allow you somehow to specify the ctor (don't have a proposal for a good syntax here). Again, I'm mostly spitballing here and may be overlooking complexities, which will be addressed when we actually get to working on that, but right now we have higher priorities that we need to tackle.

dotjon0 commented 4 months ago

In the context of the your example in the last message:

The requirement is that for this _Car example model:

Then when the Flutter app interacts with the _Car.type, the:

Is this what you are proposing via @ConstructorOmit() and @ConstructorInclude()? If yes, this is exactly what we are after and this will unlock enums in Flutter Realm.

This also means that it does not matter if the user has defined a json schema on Atlas or not - as we are just storing a String.

nirinchev commented 4 months ago

That is correct - those are my (very preliminary) thoughts on how we could approach this. There's certainly a ton of minor details I'm overlooking and we'll probably want to figure out nicer names than those, but I don't see why it shouldn't work. Though, in the interest of transparency, I'd like to reiterate that we have several higher priority/higher impact tasks we need to tackle before we can look into generator customizations (notably, several web-related tickets, one of which I believe is impacting you as well).

dotjon0 commented 4 months ago

Amazing, sounds perfect. Yes the web support (different issue) is top priority for us. Will look forward to hearing on enums at some point. Thanks again @nirinchev, really appreciated.