realm-dart icon indicating copy to clipboard operation
realm-dart copied to clipboard

Expose dynamic Realm API

Open nirinchev opened this issue 2 years ago • 6 comments

This exposes a set of string-based access API for the Realm. The current PR touches only the Realm entrypoint and a follow-up PR will be done to improve the experience when working with objects. The API is as follows

// Lookup all objects of type Car
final allCars = realm.dynamic.all('Car');

// The collection can be further filtered using the normal API
final carsFromJapan = allCars.query('manufacturer.domicile == "Japan"');

// Lookup an object by PK
final honda = realm.dynamic.find('Car', 'Honda');

Note: One can use the RealmObject.get<...>(...) API to then read properties on the object, but those will change in the future, so not showing them in the example.

This is a part of https://github.com/realm/realm-dart/issues/70.

nirinchev avatar Apr 25 '22 20:04 nirinchev

Pull Request Test Coverage Report for Build 2225904910

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.677%

Totals Coverage Status
Change from base Build 2211365211: 0.0%
Covered Lines: 400
Relevant Lines: 427

💛 - Coveralls

coveralls avatar Apr 25 '22 20:04 coveralls

There are three reasons why I didn't go for dynamic.

  1. We've been burned by dynamics in the .NET SDK. I know that dart doesn't yet have anything Unity-like in its ecosystem, but I would imagine MS didn't envision Unity either when they implemented dynamic in C#.
  2. The string-based API is slightly more discoverable as you get "intellisense" for it. So you have at least some pointers on what to do.
  3. It is more flexible. You can for example read the property information from the schema and then access properties by their name - e.g.
    for (final prop in properties) {
        if (prop.type == RealmPropertyType.int) {
            final intValue = obj.dynamic.get<int>(prop.name);
            // Do something with intValue
        }
    }
    
    You can't achieve something like that (or at least I can't see how) with the dynamic type in dart. It's not often, but we have had a handful of requests for that in the .NET SDK and I think it's quite powerful.

nirinchev avatar Apr 29 '22 09:04 nirinchev

I believe RealmObject.get has always been exposed - that's what is used in the generated classes. We use it in tests, but I haven't changed the visibility of it (at least not intentionally).

nirinchev avatar May 02 '22 11:05 nirinchev

I have been constantly thinking and revisiting this PR for the last weeks and I think we are doing a step in the wrong direction here. I have taken a look at how Realm Swift does this and am I really leaning towards a similar solution for Realm Flutter. I think Realm .NET made a mistake by enabling a generic dynamic API available at all times, which is only required while doing migrations. Instead I believe we should be providing this API only in the migration callback as Realm Swift does. We have this generic dynamic support in Dart which allows us to provide two dynamic objects, old and new version, in the migration callback from which we can allow even nicer than Swift API, cause dynamic objects in Dart allow access to any property even non existing ones. Let's discuss later when we regroup.

blagoev avatar Jul 24 '22 13:07 blagoev

String-based property setting/getting seems to be available via subscripts, like foo["bar"] = 123. As far as I can tell this functionality is always available, not just in migrations.

It also has dynamicObjects functionality which is roughly equivalent to what I'm proposing with this PR. It even goes further allowing creating of dynamic objects, which is not something covered by my work, but can be done as a follow up.

Virtually all of our SDKs offer dynamic and static API (except JS which only offers dynamic) and while I agree that the dynamic API are niche and mostly useful for migrations, I do not agree that exposing them is a mistake. It makes the SDK more powerful and gives developers flexibility.

nirinchev avatar Aug 01 '22 12:08 nirinchev

We have discussed internally and it seems we need a dynamic API in the Dart SDK to support certain use cases like app users building the Realm schema at runtime etc.

My comments about the _getValues function are still valid concerns. I suggest we refactor this code not to use such late function optimizations. This function is becoming an important internal SDK function while at the same time providing weird and unreadable signature. I guess I could help refactoring that code.

blagoev avatar Aug 15 '22 07:08 blagoev

I have inlined _getValues - it's a fair amount of code duplication, but I don't suppose that code will change that often, so it's probably fine. @blagoev please take another look and let me know if something is unclear. Happy to also do a live review session if that'll help get this PR merged faster.

nirinchev avatar Aug 15 '22 13:08 nirinchev

@blagoev refactored the while loops to use recursion. While I don't see much of a difference, it's not a hill I'm willing to die on, so can you take another look and see if there's anything left or we can merge.

nirinchev avatar Aug 18 '22 21:08 nirinchev

@blagoev refactored the while loops to use recursion. While I don't see much of a difference, it's not a hill I'm willing to die on, so can you take another look and see if there's anything left or we can merge.

It is not perfect in both ways. But it is accectable.

desistefanova avatar Aug 18 '22 23:08 desistefanova