Results of primitives #893

nielsenko · 2022-09-09T14:37:34Z

Resolves #162

coveralls · 2022-09-11T15:24:34Z

Pull Request Test Coverage Report for Build 3322498460

33 of 37 (89.19%) changed or added relevant lines in 4 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.6%) to 88.884%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/src/list.dart	8	12	66.67%

Totals
Change from base Build 3322331710:	-0.6%
Covered Lines:	2199
Relevant Lines:	2474

💛 - Coveralls

blagoev

I have some suggestions

lib/src/list.dart

CHANGELOG.md

lib/src/list.dart

blagoev · 2022-10-24T20:20:30Z

lib/src/results.dart

-  RealmResults<T> query(String query, [List<Object> args = const []]) {
-    final handle = realmCore.queryResults(this, query, args);
-    return RealmResultsInternal.create<T>(handle, realm, _metadata);
+    final meta = _metadata;


Why not check if T is RealmObjectBase which is the real marker if this can create objects or it is a results of primitives.

We can check either one, but we should assert on the other - i.e. if we get a RealmObjectBase with no meta or meta with a non-RealmObjectBase generic, that would be a bug in the SDK.

I agree we can assert the metadata, but I think we should only check the type is RealmObjectBase since this is the real marker.

I have reworked this to check on type instead. Assert on _meta != null is implicit in:

final handle = realmCore.resultsGetObjectAt(this, index); final accessor = RealmCoreAccessor(metadata, realm.isInMigration);

I expect the first line to fail, and otherwise metadata => _metadata! will.

CHANGELOG.md

nirinchev · 2022-10-24T12:32:20Z

lib/src/native/realm_core.dart

+  Object? resultsGetElementAt(RealmResults results, int index) {
+    return using((Arena arena) {
+      final realm_value = arena<realm_value_t>();
+      _realmLib.invokeGetBool(() => _realmLib.realm_results_get(results.handle._pointer, index, realm_value));
+      return realm_value.toDartValue(results.realm);
+    });
+  }
+
+  RealmObjectHandle resultsGetObjectAt(RealmResults results, int index) {


Why are those different?

one is returning a RealmObject while the other is returning a realm_value

I can see that. Since realm_value can hold objects, why not use the same method for both?

You still need to convert the handle (toDartValue don't/can't), and the later is less efficient.

lib/src/results.dart

lib/src/list.dart

Moves results of objects specific functionality to extension method to avoid misuse.

lib/src/list.dart

CHANGELOG.md

desistefanova · 2022-10-26T10:37:06Z

CHANGELOG.md

@@ -4,6 +4,8 @@

 ### Enhancements
 * Added `MutableSubscriptionSet.removeByType` for removing subscriptions by their realm object type. (Issue [#317](https://github.com/realm/realm-dart/issues/317))
+* Support results of primitives, ie. `RealmResult<int>`. (Issue [#162](https://github.com/realm/realm-dart/issues/162))
+* Support notifications on all managed realm lists, including list of primitives, ie. `RealmList<int>.changes` is supported. ([#893](https://github.com/realm/realm-dart/pull/893))


Suggested change

* Support notifications on all managed realm lists, including list of primitives, ie. `RealmList<int>.changes` is supported. ([#893](https://github.com/realm/realm-dart/pull/893))

* Support notifications on all managed `RealmList`s, including list of primitive types, for example: `RealmList<int>.changes` is supported. ([#893](https://github.com/realm/realm-dart/pull/893))

lib/src/results.dart

cla-bot bot added the cla: yes label Sep 9, 2022

nielsenko self-assigned this Sep 11, 2022

nielsenko force-pushed the kn/results-of-primitives branch 2 times, most recently from 2aaea08 to d5c2af3 Compare September 13, 2022 09:59

nielsenko changed the base branch from master to kn/indexes September 13, 2022 12:20

nielsenko force-pushed the kn/indexes branch from 02cedea to 376729d Compare September 14, 2022 11:19

nielsenko force-pushed the kn/results-of-primitives branch from d5c2af3 to e2e7315 Compare September 14, 2022 11:21

nielsenko force-pushed the kn/indexes branch 2 times, most recently from 18a009d to e72bd4f Compare September 14, 2022 17:17

nielsenko force-pushed the kn/results-of-primitives branch 3 times, most recently from f1fe4c0 to c093026 Compare September 14, 2022 19:38

nielsenko linked an issue Sep 15, 2022 that may be closed by this pull request

Support results of primitives #162

Closed

nielsenko force-pushed the kn/results-of-primitives branch from c093026 to 8034b07 Compare September 20, 2022 13:15

nielsenko force-pushed the kn/indexes branch from ea2e7cc to 5dc7caa Compare September 22, 2022 10:49

nielsenko force-pushed the kn/results-of-primitives branch 2 times, most recently from 523cd5d to 4ea3988 Compare September 23, 2022 11:42

nielsenko force-pushed the kn/indexes branch from 5dc7caa to 5ab0a40 Compare September 23, 2022 11:42

nielsenko force-pushed the kn/results-of-primitives branch from 4ea3988 to 03eb130 Compare September 23, 2022 20:41

nielsenko force-pushed the kn/indexes branch 2 times, most recently from c7125a4 to 81beb39 Compare September 26, 2022 09:17

nielsenko force-pushed the kn/results-of-primitives branch from 03eb130 to 81e0810 Compare September 26, 2022 09:18

nielsenko force-pushed the kn/indexes branch from 81beb39 to d6334af Compare September 26, 2022 09:39

nielsenko force-pushed the kn/results-of-primitives branch from 81e0810 to 4531a9b Compare September 26, 2022 09:39

nielsenko force-pushed the kn/indexes branch from d6334af to afd9a7a Compare October 3, 2022 08:27

nielsenko force-pushed the kn/results-of-primitives branch from 4531a9b to 5ea1310 Compare October 3, 2022 08:27

nielsenko force-pushed the kn/indexes branch from afd9a7a to 52ad2a9 Compare October 3, 2022 14:47

nielsenko force-pushed the kn/results-of-primitives branch from 5ea1310 to 70b6cdb Compare October 3, 2022 14:47

nielsenko force-pushed the kn/indexes branch from 52ad2a9 to fad9a36 Compare October 4, 2022 09:49

nielsenko requested a review from nirinchev October 24, 2022 11:43

blagoev suggested changes Oct 24, 2022

View reviewed changes

nirinchev requested changes Oct 24, 2022

View reviewed changes

nielsenko force-pushed the kn/results-of-primitives branch 6 times, most recently from 3f1449e to bb15f26 Compare October 25, 2022 14:01

nielsenko requested review from nirinchev and blagoev October 25, 2022 15:07

nielsenko added 8 commits October 25, 2022 18:05

Allow results of primitives.

c465b72

Moves results of objects specific functionality to extension method to avoid misuse.

Update CHANGELOG

6cfd2e3

Address PR feedback

9759852

Support notifications on List of primitives

340ac3a

Support notifications on results of primitives

c494af5

Update CHANGELOG

b1c20a2

Test List.asResults().query

39b6532

Test on type instead of _metadata

f28f1b2

nielsenko force-pushed the kn/results-of-primitives branch from bb15f26 to f28f1b2 Compare October 25, 2022 16:06

desistefanova reviewed Oct 25, 2022

View reviewed changes

lib/src/list.dart Show resolved Hide resolved

nielsenko requested a review from desistefanova October 26, 2022 06:04

desistefanova reviewed Oct 26, 2022

View reviewed changes

CHANGELOG.md Show resolved Hide resolved

desistefanova reviewed Oct 26, 2022

View reviewed changes

desistefanova approved these changes Oct 26, 2022

View reviewed changes

nirinchev approved these changes Oct 26, 2022

View reviewed changes

blagoev approved these changes Oct 26, 2022

View reviewed changes

nielsenko merged commit eba2e31 into master Oct 26, 2022

nielsenko deleted the kn/results-of-primitives branch October 26, 2022 15:45

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Results of primitives #893

Results of primitives #893

nielsenko commented Sep 9, 2022 •

edited

Loading

coveralls commented Sep 11, 2022 •

edited

Loading

blagoev left a comment

blagoev Oct 24, 2022

nirinchev Oct 24, 2022

blagoev Oct 25, 2022

nielsenko Oct 25, 2022

nirinchev Oct 24, 2022

blagoev Oct 25, 2022

nielsenko Oct 25, 2022

nirinchev Oct 25, 2022

nielsenko Oct 25, 2022 •

edited

Loading

desistefanova Oct 26, 2022

	* Support notifications on all managed realm lists, including list of primitives, ie. `RealmList<int>.changes` is supported. ([#893](https://github.com/realm/realm-dart/pull/893))
	* Support notifications on all managed `RealmList`s, including list of primitive types, for example: `RealmList<int>.changes` is supported. ([#893](https://github.com/realm/realm-dart/pull/893))

Results of primitives #893

Results of primitives #893

Conversation

nielsenko commented Sep 9, 2022 • edited Loading

coveralls commented Sep 11, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3322498460

💛 - Coveralls

blagoev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nielsenko Oct 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nielsenko commented Sep 9, 2022 •

edited

Loading

coveralls commented Sep 11, 2022 •

edited

Loading

nielsenko Oct 25, 2022 •

edited

Loading