Add support for suppressing specific notifications from writes #4253

tgoyne · 2016-10-24T22:00:11Z

Obj-c/Swift side of realm/realm-object-store#200. This is mostly just tests and doc updates since everything interesting is done in the object store.

Closes #3665.

jpsim · 2016-10-25T16:19:02Z

RealmSwift/Tests/RealmCollectionTypeTests.swift

+        // notification is called (the first would error on .update)
+        theExpectation = expectation(description: "")
+        let realm = realmWithTestPath()
+        realm.beginWrite();


this and the 3 other beginWrite()s have trailing semicolons

jpsim · 2016-11-15T21:54:49Z

Are any of the failing CI jobs relevant?

tgoyne · 2016-11-15T22:25:55Z

The interprocess test failure might be spurious or might be related to these changes.

jpsim · 2016-11-15T22:27:29Z

CHANGELOG.md

@@ -61,7 +61,8 @@ GM seed.

 ### Enhancements

-* None.
+* Add the ability to skip calling specific notification blocks when committing


Wrong changelog section

tgoyne · 2016-11-17T22:07:40Z

The remaining CI failures here are unrelated (SwiftLint errors are for files not modified in this PR, one job had a test failure that has rarely occurred in the past without these changes, and other two were just the simulator failing to boot).

jpsim · 2016-11-17T22:33:12Z

SwiftLint errors are for files not modified in this PR

There are no SwiftLint errors according to Jenkins: https://ci.realm.io/job/objc_pr/4338/configuration=Release,swift_version=3.0.1,target=swiftlint/console

jpsim · 2016-11-17T22:33:49Z

CHANGELOG.md

@@ -75,6 +76,13 @@ GM seed.
  of process termination.
 * Fix a crash that could occur when working with a `RLMLinkingObject` property
  of an unmanaged object.
+* Deliver collection notifications when beginning a write transaction which


These are still in the wrong changelog section though.

tgoyne · 2016-11-17T22:34:03Z

The Travis Swiftlint check failed. I assume it's running a different version of swiftlint.

jpsim · 2016-11-17T22:35:51Z

The Travis Swiftlint check failed. I assume it's running a different version of swiftlint.

I see! No, it's actually running with Swift 2.2 whereas Jenkins just runs SwiftLint on 3.0.1.

jpsim · 2016-11-17T22:49:10Z

Realm/RLMRealm.h

- already in a write transaction will throw an exception. Calls to
- `beginWriteTransaction` from `RLMRealm` instances in other threads will block
- until the current write transaction completes.
+ Only one write transaction can be open at a time for each Realm file. Write


Drop the "file": "for each Realm.". This will make the doc more generalized because we don't really expose that in-memory or synced Realms are backed by files.

jpsim · 2016-11-17T23:13:03Z

Realm/RLMRealm.h

@@ -202,6 +224,40 @@ typedef void (^RLMNotificationBlock)(RLMNotification notification, RLMRealm *rea
 - (BOOL)commitWriteTransaction:(NSError **)error;

 /**
+ Commits all write operations in the current write transaction, without
+ notifying specific callbacks of the changes.


"callbacks" -> "notification blocks"

jpsim · 2016-11-17T23:15:06Z

Realm/RLMRealm.h


- Notifications are sent when a write transaction is committed whether or not
- automatic refreshing is enabled.
+ Write transaction will still always advance a Realm to the latest version and


"Write transaction" -> "Write transactions"

jpsim · 2016-11-17T23:19:04Z

Realm/RLMRealm.mm

@@ -70,6 +70,13 @@ - (void)stop {
    _block = nil;
 }

+- (void)suppressNextNotification {
+    auto block = _block;


A brief comment or different variable name might help explain what this is doing.

It wasn't immediately clear to me that this approach to suppressing notifications completely bypasses the Object Store's mechanism for suppressing notifications.

jpsim · 2016-11-17T23:22:08Z

Realm/Tests/NotificationTests.m

+    [realm deleteAllObjects];
+    [realm commitWriteTransactionWithoutNotifying:@[token] error:nil];
+
+    // local realm notifications are called synchronously so no need to wait for anything


[token stop] to prevent the deallocation warning from being logged?

jpsim · 2016-11-17T23:22:29Z

Realm/Tests/NotificationTests.m

+    RLMRealm *realm = [RLMRealm defaultRealm];
+    [realm beginWriteTransaction];
+    XCTAssertThrows([realm commitWriteTransactionWithoutNotifying:@[token] error:nil]);
+    [realm cancelWriteTransaction];


[token stop] to prevent the deallocation warning from being logged?

jpsim · 2016-11-17T23:22:35Z

Realm/Tests/NotificationTests.m

+    [realm beginWriteTransaction];
+    XCTAssertThrows([otherRealm commitWriteTransactionWithoutNotifying:@[_token] error:nil]);
+    [realm cancelWriteTransaction];
+    [otherRealm cancelWriteTransaction];


[token stop] to prevent the deallocation warning from being logged?

jpsim · 2016-11-17T23:24:42Z

RealmSwift/Realm.swift

-     Write transactions cannot be nested, and trying to execute a write transaction on a Realm which is already
-     participating in a write transaction will throw an error. Calls to `write` from `Realm` instances in other threads
-     will block until the current write transaction completes.
+     Only one write transaction can be open at a time for each Realm file. Write


"for each Realm."

Just plain "for each Realm" is misleading, as to me it sounds like it's referring to Realm instances.

yeah, ok. Hard to come up with phrasing that is generalized, accurate and not misleading. Fine to keep as-is.

Collection notifications aren't sent recursively (because it can't be done in any consistent and useful way), so do the same for Realm notifications for consistency.

…syncTests testErrorHandling]

jpsim

🎉 🎉 🎉

chrisamanse · 2016-11-28T18:38:57Z

Is there a way to use closures for this? i.e.:

try realm.write(withoutNotifying: tokens) {
  store.accounts.move(from: source, to: destination)
}

Right now, I'm using:

realm.beginWrite()

store.accounts.move(from: source, to: destination)

try realm.commitWrite(withoutNotifying: tokens)

jpsim · 2016-11-28T19:49:08Z

No, there's no closure version of this API yet. We've considered that exact API that you've proposed in your comment, and might end up adding that in the future.

tgoyne added the S:Review label Oct 24, 2016

tgoyne self-assigned this Oct 24, 2016

jpsim suggested changes Oct 25, 2016

View reviewed changes

jpsim mentioned this pull request Nov 9, 2016

NotificationBlock was not called when 2 NotificationBlocks were added to 2 RLMResult #4315

Closed

tgoyne force-pushed the tg/changeset-skip branch 3 times, most recently from bfccb33 to b32121c Compare November 15, 2016 21:02

jpsim reviewed Nov 15, 2016

View reviewed changes

jpsim mentioned this pull request Nov 15, 2016

Integrate Fine-Grained Notifications in iOS app realm/realm-tasks#352

Merged

tgoyne force-pushed the tg/changeset-skip branch 5 times, most recently from 90e87fb to 45433f7 Compare November 16, 2016 23:27

jpsim reviewed Nov 17, 2016

View reviewed changes

tgoyne force-pushed the tg/changeset-skip branch 2 times, most recently from d874972 to 33d19aa Compare November 17, 2016 23:03

jpsim suggested changes Nov 17, 2016

View reviewed changes

tgoyne added 5 commits November 18, 2016 10:53

Update to latest ObjectStore

a0addfe

Add support for suppressing specific notifications from writes

0939970

Don't send Realm notifications recursively

6abd638

Collection notifications aren't sent recursively (because it can't be done in any consistent and useful way), so do the same for Realm notifications for consistency.

Remove some unused methods

f8b5834

Increase the timeout for the interprocess tests

9cf90c1

tgoyne added 3 commits November 18, 2016 10:54

Reduce how much stuff is done with the lowered open file limit in -[A…

626e2c8

…syncTests testErrorHandling]

Explicitly stop notifications to silence warnings in tests

83e0719

Improve the docs a bit

a6f36c9

tgoyne force-pushed the tg/changeset-skip branch from 668b417 to a6f36c9 Compare November 18, 2016 18:55

jpsim approved these changes Nov 18, 2016

View reviewed changes

tgoyne merged commit e9d2764 into master Nov 18, 2016

tgoyne deleted the tg/changeset-skip branch November 18, 2016 21:48

tgoyne removed the S:Review label Nov 18, 2016

This was referenced Nov 18, 2016

Change notifications sometimes miss insertions #4186

Closed

"UITableView: The number of rows contained in an existing section..." #4244

Closed

jpsim mentioned this pull request Dec 21, 2016

Replace HandoverPackage collection with ThreadSafeReference realm/realm-object-store#157

Merged

stel mentioned this pull request Jan 16, 2017

Notification block does not work always with Realm Object Server #4538

Closed

liamnichols mentioned this pull request Sep 18, 2019

Add support for suppressing notifications using closure-based write/transaction methods #6252

Merged

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

Add support for suppressing specific notifications from writes #4253

Add support for suppressing specific notifications from writes #4253

tgoyne commented Oct 24, 2016

jpsim Oct 25, 2016

jpsim commented Nov 15, 2016

tgoyne commented Nov 15, 2016

jpsim Nov 15, 2016

jpsim Nov 17, 2016

tgoyne commented Nov 17, 2016

jpsim commented Nov 17, 2016

jpsim Nov 17, 2016

tgoyne commented Nov 17, 2016

jpsim commented Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

jpsim Nov 17, 2016

tgoyne Nov 17, 2016

jpsim Nov 17, 2016

jpsim left a comment

chrisamanse commented Nov 28, 2016

jpsim commented Nov 28, 2016

Add support for suppressing specific notifications from writes #4253

Add support for suppressing specific notifications from writes #4253

Conversation

tgoyne commented Oct 24, 2016

Choose a reason for hiding this comment

jpsim commented Nov 15, 2016

tgoyne commented Nov 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tgoyne commented Nov 17, 2016

jpsim commented Nov 17, 2016

Choose a reason for hiding this comment

tgoyne commented Nov 17, 2016

jpsim commented Nov 17, 2016

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

Choose a reason for hiding this comment

jpsim left a comment

Choose a reason for hiding this comment

chrisamanse commented Nov 28, 2016

jpsim commented Nov 28, 2016