RCOCOA-2271: Collections in Mixed #8546

dianaafanador3 · 2024-04-12T16:14:02Z

Added support for storing nested collections (List and Map not ManagedSet) in a AnyRealmValue.

class MixedObject: Object {
  @Persisted var anyValue: AnyRealmValue
}

// You can build a AnyRealmValue from a Swift's Dictionary.
let dictionary: Dictionary<String, AnyRealmValue> = ["key1": .string("hello"), "key2": .bool(false)]

// You can build a AnyRealmValue from a Swift's Map
// and nest a collection within another collection.
let list: Array<AnyRealmValue> = [.int(12), .double(14.17), AnyRealmValue.fromDictionary(dictionary)]

let realm = realmWithTestPath()
try realm.write {
  let obj = MixedObject()
  obj.anyValue = AnyRealmValue.fromArray(list)
  realm.add(o)
}

Added new operators to Swift's Query API for supporting querying nested collections.

realm.objects(MixedObject.self).where { $0.anyValue[0][0][1] == .double(76.54) }

The .all operator allows looking up in all keys or indexes.

realm.objects(MixedObject.self).where { $0.anyValue["key"].all == .bool(false) }

dianaafanador3 · 2024-04-22T11:17:44Z

This is waiting for this PR to be merged to core. Also, there is one failing test which is working locally, taking a look at it, but putting this on review on the meantime.

tgoyne

A first pass over things.

Realm/RLMDictionary.h

Realm/RLMAccessor.mm

Realm/RLMArray_Private.hpp

Realm/RLMConstants.h

Realm/RLMAccessor.mm

RealmSwift/AnyRealmValue.swift

RealmSwift/List.swift

tgoyne · 2024-04-23T15:56:20Z

RealmSwift/Map.swift

@@ -779,6 +779,14 @@ public final class Map<Key: _MapKey, Value: RealmCollectionValue>: RLMSwiftColle
    }
 }

+// MARK: - Hashable
+
+extension Map: Hashable {


Similarly, this is not at all a valid Hashable implementation.

Using ObjectIdentifier given that we already conform to Equatable

ObjectIdentifier is not a valid definition either. Two managed dictionary objects which correspond to the same field are equal to each other but have different ObjectIdentifiers. All of our accessor types are fundamentally incompatible with Hashable due to that their identity changes when you promote an unmanaged object.

@tgoyne we do need to conform to Hashable to be able to include Map or List as a an associated value in AnyRealmValue if we want to go that approach.
I can think of a few approaches for this:

Use ObjectIdentifer and sacrifice equality for managed and unmanaged object

Have associated values as Swift's native types Array/Dictionary, this will clearly will go bad with nested collections which will need to be instantiated completely to convert it (Discarded)

Store some reference to the collection as the associated value and then and have some static function that returns the collection (Not need to conform to hashable, but worse developer experience).

I'm open to discuss different solutions

We can override the hash function on AnyRealmValue and return a constant value for the Map and List cases.

RealmSwift/Tests/ObjectCreationTests.swift

Realm/RLMAccessor.mm

leemaguire · 2024-04-23T14:01:30Z

Realm/RLMAccessor.mm

@@ -890,15 +914,17 @@ void RLMSetSwiftPropertyAny(__unsafe_unretained RLMObjectBase *const obj, uint16
 }

 id RLMAccessorContext::box(realm::Mixed v) {
-    return RLMMixedToObjc(v, _realm, &_info);
+    auto property = (currentProperty) ? currentProperty : _info.propertyForTableColumn(_colKey);


why would currentProperty be nil here?

For any other mixed wrapped types besides list and dictionary the accessor is constructed without the current property because it is not needed to box the native value

Realm/RLMCollection.mm

Realm/RLMValue.h

Realm/Tests/KVOTests.mm

Realm/Tests/NotificationTests.m

Realm/Tests/ObjectTests.m

RealmSwift/AnyRealmValue.swift

elle-j

Nice to see the Swift implementation of this as well! 🙂

CHANGELOG.md

Package.swift

Realm/RLMUtil.mm

Realm/RLMValue.h

tgoyne · 2024-04-29T19:27:46Z

There are a bunch of new warnings here that need to be fixed.

Realm/ObjectServerTests/RealmServer.swift

Realm/RLMAccessor.mm

Realm/RLMManagedArray.mm

Realm/RLMQueryUtil.mm

RealmSwift/Object.swift

RealmSwift/AnyRealmValue.swift

Realm/RLMManagedArray.mm

Realm/RLMCollection.mm

Realm/RLMManagedDictionary.mm

Realm/RLMQueryUtil.mm

RealmSwift/Object.swift

RealmSwift/ObjectiveCSupport+AnyRealmValue.swift

tgoyne · 2024-05-08T19:21:14Z

Realm/RLMValue.mm

+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+- (RLMPropertyType)rlm_valueType {
+    REALM_UNREACHABLE();


This is not unreachable. rlm_valueType is part of the public API.

@tgoyne This is a very particular case because we are deprecating the declaration but also changing the type to be used. You are right because this is part of the public, the user can ask for rlm_valueType , but because we don't want to add any new types to property type we are in some sort of limbo here.
Solutions
We can return RLMPropertyTypeAny like we do for Null, and this is actually true because this indeed a colection of mixed?

Returning RLMPropertyTypeAny seems like the best option here.

REALM_UNREACHABLE() marks a place that should be logically impossible to hit and if we get there it always indicates a bug in our code.

RealmSwift/Impl/ObjcBridgeable.swift

tgoyne · 2024-05-08T19:26:03Z

RealmSwift/Query.swift

+    }
+    /// :nodoc:
+    public subscript(key: String) -> Query<AnyRealmValue> {
+        .init(appendKeyPath("['\(key)']", options: [.isPath]))


This needs to handle key containing quotes.

@tgoyne
Do you mean because of this ?

Note The expressions you write inside parentheses within an interpolated string can’t contain an unescaped backslash (\), a carriage return, or a line feed. However, they can contain other string literals.

https://docs.swift.org/swift-book/documentation/the-swift-programming-language/stringsandcharacters/

If you do $0.anyCol["'"] == 0 to query on the dictionary key "'" this will produce invalid syntax.

$0.anyCol["'"] is not allowed for AnyRealmValue subscripts or I'm missing something

I don't know what you mean. any["'"] == %@ (or any['\''] == %@) is a perfectly valid query, which this currently fails to generate a valid string for.

tgoyne

There are still several new warnings. I'm not sure why the implicit casts from id to RLMArray are producing warnings here but not on master, but the other warnings seem straightforward.

CHANGELOG.md

Realm/RLMQueryUtil.mm

tgoyne · 2024-05-22T00:33:38Z

RealmSwift/Query.swift

+    }
+    /// :nodoc:
+    public subscript(key: String) -> Query<AnyRealmValue> {
+        .init(appendKeyPath("['\(key)']", options: [.isPath]))


If you do $0.anyCol["'"] == 0 to query on the dictionary key "'" this will produce invalid syntax.

RealmSwift/Query.swift

RealmSwift/RealmCollection.swift

RealmSwift/ObjectiveCSupport+AnyRealmValue.swift

dianaafanador3 · 2024-05-22T23:44:30Z

@tgoyne solved PR comments and removed the warnings

Realm/RLMSwiftValueStorage.mm

tgoyne · 2024-05-23T18:12:41Z

Realm/RLMValue.mm

+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+- (RLMPropertyType)rlm_valueType {
+    REALM_UNREACHABLE();


Returning RLMPropertyTypeAny seems like the best option here.

REALM_UNREACHABLE() marks a place that should be logically impossible to hit and if we get there it always indicates a bug in our code.

Realm/Tests/NotificationTests.m

Realm/Tests/QueryTests.m

RealmSwift/AnyRealmValue.swift

RealmSwift/Object.swift

tgoyne · 2024-05-23T18:30:25Z

RealmSwift/Query.swift

+    }
+    /// :nodoc:
+    public subscript(key: String) -> Query<AnyRealmValue> {
+        .init(appendKeyPath("['\(key)']", options: [.isPath]))


I don't know what you mean. any["'"] == %@ (or any['\''] == %@) is a perfectly valid query, which this currently fails to generate a valid string for.

RealmSwift/RealmCollection.swift

RealmSwift/Tests/MixedCollectionTest.swift

cla-bot bot added the cla: yes label Apr 12, 2024

github-actions bot assigned dianaafanador3 Apr 12, 2024

dianaafanador3 closed this Apr 12, 2024

dianaafanador3 reopened this Apr 12, 2024

dianaafanador3 marked this pull request as draft April 12, 2024 16:16

dianaafanador3 force-pushed the dp/collections_in_mixed branch 8 times, most recently from b60f46f to 3386d7a Compare April 22, 2024 07:36

dianaafanador3 marked this pull request as ready for review April 22, 2024 11:15

dianaafanador3 requested review from tgoyne, nirinchev and leemaguire April 22, 2024 11:15

dianaafanador3 changed the title ~~Collections in Mixed~~ RCOCOA-2271: Collections in Mixed Apr 22, 2024

dianaafanador3 force-pushed the dp/collections_in_mixed branch 9 times, most recently from ed91d6b to 45a17bd Compare April 23, 2024 14:44

tgoyne reviewed Apr 23, 2024

View reviewed changes

leemaguire reviewed Apr 23, 2024

View reviewed changes

elle-j reviewed Apr 25, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Package.swift Outdated Show resolved Hide resolved

Realm/RLMUtil.mm Show resolved Hide resolved

tgoyne reviewed Apr 25, 2024

View reviewed changes

Realm/RLMValue.h Outdated Show resolved Hide resolved

dianaafanador3 requested review from elle-j and tgoyne April 26, 2024 12:26

tgoyne reviewed Apr 29, 2024

View reviewed changes

dianaafanador3 force-pushed the dp/collections_in_mixed branch 4 times, most recently from cede8e2 to 26d3366 Compare May 7, 2024 15:05

tgoyne reviewed May 8, 2024

View reviewed changes

dianaafanador3 force-pushed the dp/collections_in_mixed branch from f92fe16 to 760f3e5 Compare May 14, 2024 12:11

dianaafanador3 requested a review from tgoyne May 14, 2024 12:18

tgoyne mentioned this pull request May 21, 2024

Upgrade to core 14.7.0 #8597

Open

tgoyne reviewed May 22, 2024

View reviewed changes

dianaafanador3 added 7 commits May 22, 2024 15:40

Collections in mixed

15e3b94

Solved PR comments

94f881f

Solve PR comments

3141e75

Solve PR comments

effd060

Solve PR comments and use core version

1ad20a0

Fix CI issues and warnings

f8ee537

Solve PR comments

7d5023f

dianaafanador3 force-pushed the dp/collections_in_mixed branch 3 times, most recently from 3683365 to 98a39c3 Compare May 22, 2024 23:36

Solve PR comments

2a3861f

dianaafanador3 force-pushed the dp/collections_in_mixed branch from 98a39c3 to 2a3861f Compare May 22, 2024 23:43

dianaafanador3 requested a review from tgoyne May 22, 2024 23:43

tgoyne reviewed May 23, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RCOCOA-2271: Collections in Mixed #8546

RCOCOA-2271: Collections in Mixed #8546

dianaafanador3 commented Apr 12, 2024 •

edited

dianaafanador3 commented Apr 22, 2024

tgoyne left a comment

tgoyne Apr 23, 2024

dianaafanador3 Apr 24, 2024

tgoyne Apr 25, 2024

dianaafanador3 Apr 26, 2024

tgoyne Apr 26, 2024

leemaguire Apr 23, 2024

dianaafanador3 Apr 24, 2024

elle-j left a comment

tgoyne commented Apr 29, 2024

tgoyne May 8, 2024

dianaafanador3 May 13, 2024

tgoyne May 23, 2024

tgoyne May 8, 2024

dianaafanador3 May 14, 2024 •

edited

tgoyne May 22, 2024

dianaafanador3 May 22, 2024 •

edited

tgoyne May 23, 2024

tgoyne left a comment

tgoyne May 22, 2024

dianaafanador3 commented May 22, 2024

tgoyne May 23, 2024

tgoyne May 23, 2024

RCOCOA-2271: Collections in Mixed #8546

Are you sure you want to change the base?

RCOCOA-2271: Collections in Mixed #8546

Conversation

dianaafanador3 commented Apr 12, 2024 • edited

dianaafanador3 commented Apr 22, 2024

tgoyne 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

elle-j left a comment

Choose a reason for hiding this comment

tgoyne commented Apr 29, 2024

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

dianaafanador3 May 14, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dianaafanador3 May 22, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgoyne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dianaafanador3 commented May 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dianaafanador3 commented Apr 12, 2024 •

edited

dianaafanador3 May 14, 2024 •

edited

dianaafanador3 May 22, 2024 •

edited