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

Allow nullable types in collections #708

Closed
wants to merge 21 commits into from

Conversation

nielsenko
Copy link
Contributor

@nielsenko nielsenko commented Jul 2, 2022

Ultimately this is about supporting nullable primitive elements on realm lists, ie.

@RealmModel()
class _Player {
  @PrimaryKey()
  late String name;
  _Game? game;
  final scoresByRound = <int?>[]; // null means player didn't finish
}

@RealmModel()
class _Game {
  final winnerByRound = <_Player>[]; // I intended to support <_Player?> here as well, but alas core doesn't support it
  int get rounds => winnerByRound.length;
}

In the process I refactored how the generator lays out the schema, so it is now all done compile time with const constructable objects, so the above will generate Player:

class Player extends _Player with RealmEntityMixin, RealmObjectMixin {
  Player(
    String name, {
    Game? game,
    Iterable<int?> scoresByRound = const [],
  }) {
    _nameProperty.setValue(this, name);
    _gameProperty.setValue(this, game);
    _scoresByRoundProperty.setValue(this, RealmList<int?>(scoresByRound));
  }

  Player._();

  static const _nameProperty = ValueProperty<String>(
    'name',
    RealmPropertyType.string,
    primaryKey: true,
  );
  @override
  String get name => _nameProperty.getValue(this);
  @override
  set name(String value) => throw RealmUnsupportedSetError();

  static const _gameProperty = ObjectProperty<Game>('game', 'Game');
  @override
  Game? get game => _gameProperty.getValue(this);
  @override
  set game(covariant Game? value) => _gameProperty.setValue(this, value);

  static const _scoresByRoundProperty =
      ListProperty<int?>('scoresByRound', RealmPropertyType.int);
  @override
  RealmList<int?> get scoresByRound => _scoresByRoundProperty.getValue(this);
  @override
  set scoresByRound(covariant RealmList<int?> value) =>
      throw RealmUnsupportedSetError();

  @override
  Stream<RealmObjectChanges<Player>> get changes =>
      RealmObjectMixin.getChanges(this);

  static const schema = SchemaObject<Player>(
    Player._,
    'Player',
    {
      'name': _nameProperty,
      'game': _gameProperty,
      'scoresByRound': _scoresByRoundProperty,
    },
    _nameProperty,
  );
  @override
  SchemaObject get instanceSchema => schema;
}

Notice how schema is const constructed, and the strongly typed property types.

Dynamic realms, use _DynamicSchemaProperty and benefits by allowing me to drop getList<T> for just get<List<T>>, ie. not special property access needed for lists.

Resolves #163

@cla-bot cla-bot bot added the cla: yes label Jul 2, 2022
@nielsenko nielsenko self-assigned this Jul 2, 2022
@nielsenko nielsenko force-pushed the kn/nullable-in-collections branch 9 times, most recently from d0d28e3 to 913c3ae Compare July 6, 2022 10:02
@nielsenko nielsenko force-pushed the kn/nullable-in-collections branch 3 times, most recently from ba2de2f to 2b55f66 Compare August 12, 2022 17:24
@nielsenko nielsenko force-pushed the kn/nullable-in-collections branch 2 times, most recently from 84e90e7 to de556d3 Compare August 18, 2022 20:13
@nielsenko nielsenko force-pushed the kn/nullable-in-collections branch 2 times, most recently from ec9a8a9 to 68c1632 Compare August 25, 2022 06:57
@coveralls
Copy link

coveralls commented Aug 25, 2022

Pull Request Test Coverage Report for Build 3037342011

  • 236 of 264 (89.39%) changed or added relevant lines in 16 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+5.3%) to 92.523%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/src/type_utils.dart 4 5 80.0%
generator/lib/src/dart_type_ex.dart 3 5 60.0%
lib/src/utils.dart 2 4 50.0%
generator/lib/src/realm_object_generator.dart 8 11 72.73%
lib/src/realm_object.dart 67 73 91.78%
lib/src/realm_property.dart 28 42 66.67%
Files with Coverage Reduction New Missed Lines %
lib/src/realm_object.dart 2 86.67%
Totals Coverage Status
Change from base Build 3036688559: 5.3%
Covered Lines: 396
Relevant Lines: 428

💛 - Coveralls

@nielsenko nielsenko requested review from nirinchev and blagoev and removed request for nirinchev August 26, 2022 13:13
@nielsenko nielsenko marked this pull request as ready for review August 26, 2022 13:21
Copy link
Contributor

@blagoev blagoev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we are doing too much in this PR. We definitely need to have a discussion about the changes around the generator. I think we are putting too much responsibility into the generator which was designed to be lightweight and something we plan to replace in the future.

@nielsenko
Copy link
Contributor Author

I think we are doing too much in this PR. We definitely need to have a discussion about the changes around the generator. I think we are putting too much responsibility into the generator which was designed to be lightweight and something we plan to replace in the future.

There is no more logic in the generator than before, it is just as fast as it has always been. It just lays out the generated code in a smarter way. It allows the runtime to skip work, because more is known compile time. I don't see how this in anyway will make it more difficult to transition to static meta-programming once it lands.

@nielsenko
Copy link
Contributor Author

This is superseded by #902 and #903 which splits the feature implementation from the refactoring

@nielsenko nielsenko closed this Sep 14, 2022
@Shreedhar73
Copy link

Shreedhar73 commented Nov 9, 2023

@nielsenko Hello.
Is it so, Nullable types in Collections are only supported for local database ?

When i try to have a field of Type List<String?> it is throwing sync error.
Invalid schema change (UPLOAD): sync does not support collections of nullable primitives unless using the mixed type .

Is it the expected behavior or I am missing something here?

@nirinchev
Copy link
Member

That is correct. The server team does plan to eventually add support but it hasn't been a highly requested feature and thus we don't have any timeframe for when we'll get to it. If it's important for your app, definitely reach out to your AE to clarify your requirements.

@Shreedhar73
Copy link

@nirinchev Thanks for Clarification.

@nirinchev nirinchev deleted the kn/nullable-in-collections branch November 10, 2023 13:19
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support nullable types in collections
6 participants