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

RDART-941: Add support for collections in RealmValue #1469

Merged
merged 18 commits into from
Feb 1, 2024

Conversation

nirinchev
Copy link
Member

@nirinchev nirinchev commented Jan 10, 2024

Fixes #1505
Fixes #1504

TODO:

  • Decide on an implementation of results/list.indexOf/contains
  • Decide on equality implementation for collections inside RealmValue
  • Restore tests with indexed RealmValue field
  • Consider adding a RealmValueType enum to RealmValue
  • Consider moving RealmList/Map to common. Right now .asList/.asMap returns the dart list/map, but that doesn't have the extra information RealmList/RealmMap has. Implemented as extensions inside the realm package.
  • Tests for equality
  • Tests for results/list.indexOf/contains
  • Tests for queries on mixed fields
  • Tests for Set<RealmValue> behavior
  • Enable notifications tests when Missing list notifications for inner collection changes realm-core#7270 is addressed
  • Enable @type = $0 query tests when Allow query substitution for the @type argument realm-core#7289 is addressed.

@nirinchev nirinchev marked this pull request as ready for review January 23, 2024 22:43
Copy link
Contributor

@nielsenko nielsenko left a comment

Choose a reason for hiding this comment

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

Did core fix the issues on their side yet? If so, lets merge.

/// Extensions on RealmValue providing convenience conversion operators
extension RealmValueConvenience on RealmValue {
/// Casts [value] to a List<RealmValue>. It will throw an exception if [value] is not a list.
RealmList<RealmValue> asList() {
Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed using a getter instead as it is slightly shorter, but it actually goes against https://dart.dev/effective-dart/design#prefer-naming-a-method-as___-if-it-returns-a-different-representation-backed-by-the-original-object, so 👍

@nirinchev
Copy link
Member Author

There's a series of issues I opened for the query engine - not sure if those are legitimate bugs or things are working as designed, so I'll wait to see what @jedelbo comes back with.

@cla-bot cla-bot bot added the cla: yes label Jan 25, 2024
@nirinchev nirinchev merged commit 55cb591 into ni/core-vnext Feb 1, 2024
25 checks passed
@nirinchev nirinchev deleted the ni/collections-in-mixed-2 branch February 1, 2024 13:59
nirinchev added a commit that referenced this pull request Mar 6, 2024
* Update to Core next-major

* Update old-format file, update changelog

* Add support for collections in RealmValue (#1469)

* Initial stab at collections in mixed

* regenerate ffi bindings

* Add more tests

* Add notification tests

* Return the types

* Address some PR feedback

* Add RealmValueType

* Fix some tests

* Add query tests

* Add tests for indexOf/contains; add tests for sets

* Create a symlink for test data inside tests

* Add a few more tests

* Correct Core issue link

* enable notifications tests

* Enable some tests

* Re-generate ffi

* Fix test

* Added a changelog entry

* Update Core and reenable some tests

* Update core submodule

* Move cmake presets to root of repo

* Move cmake presets back in realm_dart

* Update Core

* Fix paths

* Regenerate ffi bindings

* Remove deprecated members (#1526)

* Update changelog

* Update paths in prepare-release.yml

* [Release 2.0.0-alpha.1] (#1527)

* [Release 2.0.0-alpha.1]

* fix paths

* Read package version from the correct file

* More release workflow fixes

* ..

* Remove some symlink removal

* tar the correct packages

---------

Co-authored-by: nirinchev <[email protected]>
Co-authored-by: Nikola Irinchev <[email protected]>

* Fixes to the release process

* [Release 2.0.0-alpha.2] (#1528)

* [Release 2.0.0-alpha.2]

* Remove revived symlink

---------

Co-authored-by: nirinchev <[email protected]>
Co-authored-by: Nikola Irinchev <[email protected]>

* Add support for dynamic.getSet/getMap (#1533)

* Add support for dynamic.getSet/getMap

* Fix test

* More test fixes

* Apply suggestions from code review

Co-authored-by: Kasper Overgård Nielsen <[email protected]>

* Regenerate models

* Update changelog

---------

Co-authored-by: Kasper Overgård Nielsen <[email protected]>

---------

Co-authored-by: Realm CI <[email protected]>
Co-authored-by: nirinchev <[email protected]>
Co-authored-by: Kasper Overgård Nielsen <[email protected]>
@nirinchev nirinchev changed the title Add support for collections in RealmValue RDART-941: Add support for collections in RealmValue Mar 14, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 13, 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.

2 participants