Map: Remove Keys When Object Values Become nil

[bdkjones]

Context

Consider this code:

final class Foo: Object
{
    @Persisted var kids: Map<String, Bar?>
}

final class Bar: Object
{
    ...
}

// Somewhere, with a `Bar` object that is part of a Realm and referenced in a `Foo.kids` map:
try someRealm.write {
    someRealm.delete(someBar)
}

Problem:

The docs for Map do not explain what happens when the value is an Object subclass and that object is deleted from the Realm. Does:

A) The Map remove both the Key and Value from the Map, as if the deleted Object were part of a List?
or
B) Does the Key persist in the Map and simply have a nil value now?

The actual behavior is inconsistent and surprising:

1. If you directly set a value to nil (myMap["someKey"] = nil), then both the key and value are removed.

2. But if an object indirectly becomes nil (you delete it from a Realm), then the key remains and the value is nil.

Behavior Breaks During Sync:

A map with ["someKey": nil] does not exist on MongoDB documents. If the object becomes nil, Atlas Device Sync completely eliminates both the key and value from the document property that represents the map. (Or, at least, it's not shown in the "Browse Collections" UI on MongoDB's website; the map property is empty.) If you delete your local Realm file and sync from the cloud, you get back an empty Map instead of the ["someKey": nil] Map that you had before. This is technically data-loss.

Solution

Map should adopt the same rule as all other Realm Collections: if an Object becomes nil, it is nuked from the collection completely. For Map, that means destroying the value and the associated key.

No other Realm collection swaps in a nil placeholder for a newly-deleted Object. Neither should Map. There is no difference in return value between using a key with a nil placeholder assigned and using a key that does not exist in the Map: they both return nil. The ONLY impact would be if you're enumerating key/value pairs and expect a given key to exist. But enumeration isn't what Map is meant for; List is the choice for that. Map is specifically for accessing a particular value very quickly by key.

Adopting this change also restores Map to a consistently-syncable state. No data-loss can occur because Map cannot get into a state that is unrepresentable on Atlas Device Sync.

Secondly, the docs for Map should be updated to explicitly describe this behavior so that people know what will happen when Objects are deleted.

Alternatives

Change Atlas Device Sync so that it can represent a ["someKey": nil] Map?

How important is this improvement for you?

Would be a major improvement

Feature would mainly be used with

Atlas Device Sync and Local Databases

[tgoyne]

Like object links and unlike other collections the map entry is set to null if the target object is deleted, which indeed is why the value is required to be optional.

[bdkjones]

@tgoyne Thanks! That would be a very good clarification to make in the Map docs because there is a reasonable presumption that the collection might behave the way other Realm collections behave: automatically culling themselves when member objects are deleted.

[bdkjones]

@tgoyne When using Atlas Device Sync, if I simply delete the object that is the value (instead of first removing it from the Map), on MongoDB's website the Map (the document property that represents it) is completely empty. The object is gone, but so is the Key to which it was assigned. Is this simply because MongoDB documents do not have a nil placeholder?

If I were to delete the local Realm file and re-sync from the server, would those keys with nil values be destroyed? If the decision to keep the keys was intentional and is important to some scenarios, wouldn't this situation (re-syncing from Atlas) produce data loss? I wouldn't get back my [someKey: nil] Map; I'd just get back an empty Map.

It seems like Map ought to have adopted the same rule as the other collections: if an object is deleted from the Realm, it vanishes from collections. In Map’s case, the remnant of the object (the key) remains and appears to put the Map in an invalid state for syncing. It seems a better option is to nuke both key and value when the object is deleted.

Right now, the Swift SDK's treatment of Map doesn't appear to line up with what Atlas supports?

[bdkjones]

@tgoyne On the MongoDB Forums, Jay informed me of yet more incongruent behavior here:

1. If I explicitly set a Map value to nil, both the value and its key are removed from the Map.

2. If, however, the value indirectly becomes nil (I delete the object from the Realm), the key remains but the Map is now in a state that cannot be properly represented on Atlas Device Sync for the reasons above.

This behavior seems like it should be unified: anytime a value in a Map goes nil, the value and its key are removed. Simple, clean, and the same as other Ream collections, which do not add null placeholders to slots previously occupied by now-deleted Objects.

(I assumed object values in a Map had to be Optionals because Realm dynamically pulls them on access and that process might fail. I did not assume they were Optionals because adding nil values was an intended ability--and, in fact, it is not because directly setting a map value to nil nukes the value and key!)

[bdkjones]

Above, cueNotes is a Map. You can see that my local copy of it in Swift has 3 keys with null placeholder values that are not present on MongoDB's backend. To get into this state, I simply deleted the Object values that were assigned to those keys without first removing them from the Map. cueNotes will persist exactly as-is across re-launches of my application as long as I don't delete the local Realm file.

Next, I delete my local Realm file and re-download from Atlas Device Sync. Now, the same cueNotes Map in my Realm is empty and I have lost the keys that had null values:

So, if retaining the keys after Objects are deleted was an intentional design decision, it currently leads to an unsync-able state and data-loss.

[nirinchev]

The reason why maps behave differently from other collections is because we felt deleting the keys would be too destructive. There's reasonable presumption that keys carry information that may be important to people's applications and it would be semantically incorrect to delete X, which triggers a deletion of the somewhat unrelated data Y.

We don't intend to change the behavior for the local collections, but the behavior you're describing with sync doesn't appear to be correct and we'll look into it.

[bdkjones]

@nirinchev if the keys are important, why are they deleted if I explicitly set a Map value to nil? (myMap["someKey"] = nil)

It's just very inconsistent behavior and it's not documented at all. You end up with a bunch of junk keys and null values because you assume the collection is self-tidying, like all other Realm collections.

It's like Map "leaks" Keys. And if this behavior IS intentional, ha, then leave Atlas Device Sync alone--it acts as a way to clean up the leaked Keys if you forget to remove them (which you will, because, again, ZERO documentation about this behavior).

[nirinchev]

Keys should not be deleted when you set the value to nil. We're investigating the behavior of maps with the Swift SDK and will report back when we understand the root cause of the issues you're seeing. In the meantime, the intended behavior is:

1. You can store nil values in maps, unlike other collections.
2. Removing an object sets the value to nil, but does not remove the key.
3. With Realm Sync, the field value in the object is set to nil, but the field itself is not removed.
4. Removing a value from the map does not delete the object itself (this is the same as with other collections, but just wanted to be explicit about it).

While I understand the sentiment that keys are being leaked from the perspective of a user who doesn't care about the keys, we're being conservative here and catering to the possibility of keys being important and not wanting to risk loss of data. Especially in the context of MongoDB, where maps are modeled as objects, it's reasonable to not want to remove fields from an object because something in an unrelated collection got deleted.

Again, we understand that there's some unexpected behavior right now and we'll fix it, but we don't intend to change the behavior from the rules described above. We'll make sure to update the documentation to reflect this and also suggest strategies for key cleanup in the case a developer wants to do that.

[bdkjones]

@nirinchev All of that seems reasonable. It resolves all the inconsistencies, at least.

When you update the docs, I would stress that Map isn't really the correct collection to use if you're going to enumerate. (That's really the only case where keeping keys with nil values matters. In accessing the Map by key, there is no difference between asking for a key that does not exist and a key associated with a nil value.)

I'm not sure how Map is built, but enumerating Key-Value collections is usually less performant than enumerating an Array.

But, yes, the Map docs need a lot more detail.

[bdkjones]

@nirinchev Thinking more on this, I really recommend that you guys reconsider at least one part. Users are going to do this:

someMap["someKey"] = nil

In Swift dictionaries, that removes BOTH key and value, as explicitly stated here: https://developer.apple.com/documentation/swift/dictionary#2846229

(This is also how Realm Maps currently behave. It is not possible to assign a nil value; the only way to get one in the Map is to assign an Object and then delete the Object from the Realm.)

But if Realm adopts your proposed behavior, users now have to remember that Realm doesn't follow idiomatic Swift and, instead, they need to use this magic incantation to avoid leaking Keys:

someMap.removeObject(forKey: someKey)

The better approach is to behave as native Swift collections behave: setting the value of a key to nil removes BOTH the value and the key. There's no good reason for Realm to buck the language convention here—80% of users will end up leaking keys because they're used to standard Swift syntax for removing objects from collections. For Maps that see frequent use, this could be a real invisible performance/memory pitfall.

[nirinchev]

Hm, I was not aware of the Swift behavior (I'm primarily working on the C# SDK). This is going to be a bit tough to consolidate across the different languages we support - i.e. a developer who has a Swift and a Kotlin app would expect to see the same behavior across both. I'll bring it up for discussion within the team and we'll see if we can find some common ground.

[tkaye407]

Hey @bdkjones. Thanks for filing this issue. I am an engineer on the Device Sync Server team. I cannot speak to the more recent issues you bring up, but I wanted to follow up on the issue of having a dictionary of links and the behavior when an object is removed. I was able to reproduce the issue you described (removing the keys), and I agree that it is incorrect behavior for this operation.

As a light explanation, we generally try to treat setting optional fields to null as removing them from the MongoDB document in order to conserve space on the users' cluster and more generally bridge the gap between Realm and MongoDB data modeling. You are right to point out that in the case of dictionaries, this is the wrong thing to do since the key carries value. I have filed a ticket for this and will try to get it into the next release in about 2 weeks.

[tgoyne]

We match what Swift does for dictionaries: foo["bar"] = nil removes the key from the dictionary, while foo["bar"] = .none sets the key "bar" to the value nil. The is unrelated to the behavior of deleting the object which a key links to, which normal dictionaries don't have any equivalent of.

[bdkjones]

@tgoyne yep! The status quo for someMap["someKey"] = nil is correct. There was some talk of changing that above, but the .none case (documentation anyone?) solves the issue of how you set a nil value without deleting an Object.

@tkaye407 thanks! I don't suppose you guys know who to contact about that atrocious backend website UI that cuts off "cueNotes" on a 32" display, eh? THAT part of Mongo could really use help.

[Jaycyn]

In summary and a question:

foo["bar"] = nil completely removes the key:value pair from the map

foo["bar"] = .none sets the value to nil, leaving the key in place

and then a question

foo["bar"] = someRealmObject

and then someRealmObject is deleted from realm

Is the proposed behavior, to leave the key in place with a value of nil, like assigning it .none, or does it remove the entry entirely?

[bdkjones]

@Jaycyn the proposed behavior is to leave the key in place and the value becomes nil.

I personally think that's a bad call because it makes the collection leaky--no other Realm collection requires manual cleanup. It's also ham fisted because you can have nil values in a map, but you can't set nil directly; you have to know .none exists and that's all completely undocumented. If the collection supports nil, then you ought to be able to set nil directly. But that would conflict with idiomatic Swift, which removes key and value if you set a value to nil.

Having the keys disappear when an Object value is deleted would neatly clean up this entire mess, but that's not the route the team has chosen.

Map will need much better documentation. Right now, it's a foot-gun.

[bdkjones]

@Jaycyn In my opinion, the default behavior should be that when an Object value is deleted from the Realm, both Key and Value are removed from a Map.

If it is important that a Key persist, it should be on the developer to go back and explicitly set someMap["someKey"] = .none. That allows Map to support nil objects while still adopting sane default behavior.

90% of the time, if the Object is nil, the Key in the Map no longer matters. But Realm is set up to inconvenience the 90% to accommodate the 10% doing weird stuff with Map. I think the default behavior should be inverted so that Map isn't leaky by default. But it sounds like that ship has sailed and Realm is committed to this non-standard behavior for Map.

[bdkjones]

Just to make sure this horse is really dead: another irritating side-effect of Map's insistence on keeping keys is this:

let values: Results<SomeObject?> = someMap.where({ $0.someCondition == true })

The Results is full of optionals. Adds extra overhead to unwrap those and then you have to handle the case where a value in the collection suddenly becomes nil, which is a headache if you're using this Results to power a UI because its count isn't really the number of rows you want in your tableView, etc. But if you compactMap() the thing you lose the live collection.

I think the best design decision would have been:

1. Map does not allow nil values.

2. For the edge cases where a developer needs Map keys to persist, instead of deleting the Object assigned to that key, he should instead mark the Object as "null-ified" with a custom property on the subclass such as isValid: Bool. (Ideally it'd be nice to just set the value to -1 or "", but Realm can't do mixed-type collections and doesn't support polymorphism.)

This would have ticked all the boxes: No ugly optional overhead, perfect conformance with Swift collection idioms, no weird edge-cases to handle where values might suddenly go nil, still allows the flexibility to keep Keys if those Keys are meaningful for your use case, etc.

Map as implemented is just a poor design. Unfortunately, I think it would take a major version bump and deprecation process to change it now.

[nirinchev]

Hey @bdkjones coming back to this (sorry for the delay!) - are you sure someMap.where({ $0.someCondition == true }) contains null values? I have a few tests, which all show that we correctly filter out null values from the query, which means that you can plug it into a tableView and have the correct number of rows reported by count. The unwrapping part is annoying - I agree - but in terms of correctness, as long as you put some filter on the map, you'd get the expected behavior of only non-null objects being returned.

Again, not a swift developer, so it's possible the Swift behavior differs, but I'd be surprised if it does, considering all of that is handled by the query engine of the core database.

Regarding the map design - I appreciate the feedback. When we designed it, we erred on the side of being safe and consistent across languages and MongoDB (e.g. if you delete a document in MongoDB that was referenced in an embedded object somewhere else, we don't update the embedded object). We'll review the design considerations and see whether a different default behavior would make sense. That being said, we're quite conservative with making breaking changes, so I don't expect it to be a rushed decision.

[bdkjones]

@nirinchev I think this one has been handled with a fix on Atlas. The determination was that map is allowed to contain nil values (unlike all other Realm collections) because the key might have importance to someone, somewhere.

I think that's a poor design decision that leads to cruft and unanticipated nil values. But if you documented the behavior correctly, I suppose developers can work around it.

Filtering the map will, of course, remove all values that don't match the filter. But that isn't the point. The underlying map on my model object still has keys that are paired with nil values. The filter is transitory, in other words.

I think this behavior is a violation of Realm's "live collection" philosophy, because deleting the object doesn't actually clean up maps where that object was used. They're left in an inconsistent state and YOU have to manually remember to go remove the associated keys. It's cumbersome, messy, prone to database corruption and unanticipated behavior. That's ESPECIALLY true because the docs for map explain absolutely NOTHING about this deletion behavior that leaves keys in place.

The ship has sailed, I guess. Having discovered this undocumented bloat in map, I can at least workaround the issue manually.

[nirinchev]

Right - the Atlas fix was a legitimate bug and it was addressed. I was mostly replying to your last comment and the design feedback. I agree that it's not ideal for the Swift use case and it's possibly not ideal for other client-side applications and I have some ideas I'll run by the team to see if they're too crazy - e.g. we could offer the option to declare a Map<String, Model> where the model is not optional and have it behave like lists/pure Swift maps. This could either be a new data structure or provide compatibility with existing maps by internally filtering out pairs with a null value (this is kind of what lists do already anyway). This is super untested and may turn out to either be very difficult to implement or run into weird corner cases we have no answers for, but doesn't hurt to brainstorm a little.

Again, can't promise any quick resolutions, just wanted to let you know we haven't forgotten about this ticket. For now, I'm happy that this is not a hard blocker for you and other swift developers, even if it is annoying to use.

[bdkjones]

I’m not too worried. The argument that the keys should remain because they have important meaning is flawed. Consider a `List`: it may be semantically important that an object is at index 3 in that collection. But when the object is deleted, we don’t fill index 3 with `nil` to retain that semantic information. We nuke the object and slide whatever was at index 4 into 3, etc. Likewise, when a value is deleted in a `map`, the key should be removed so the map is automatically cleaned up, just as all other live collections. If it is important to know that a given key exists, but has a nil value, then `map` isn’t the right choice. Create an object with an optional property. Model it explicitly.

[bdkjones]

It’s really pretty simple: all collections in Realm are self-cleaning EXCEPT `map`. Map will accrue cruft as objects are deleted. It requires very special handling. That fact is not documented anywhere and it should be because the behavior is 100% inconsistent with all other Realm collection types.

[bdkjones]

Map’s behavior is ALSO inconsistent with how Swift dictionaries behave. If you nuke a value in a Swift dictionary, the key gets removed as well. Everything about this design runs contrary to what developers are trained to expect and there is zero warning about it in the docs. That should be addressed. Nice big red warning box informing developers that Map requires careful, manual pruning.

[nirinchev]

I've filed realm/realm-core#7568 as that'd be a prerequisite for this to be addressed. We'll track the Core ticket and when/if that gets handled, we'll expose things in other SDKs, including Swift.