Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Single Object Notifications #4101

Closed
1 of 6 tasks
cmelchior opened this issue Jan 24, 2017 · 6 comments
Closed
1 of 6 tasks

Single Object Notifications #4101

cmelchior opened this issue Jan 24, 2017 · 6 comments
Assignees
Milestone

Comments

@cmelchior
Copy link
Contributor

cmelchior commented Jan 24, 2017

Motivation
realm/realm-object-store#324 added support for single object notifications to the Object Store

realm/realm-swift#4537 is adding the public API's on the Cocoa side.

We should add similar capabilities to Android.

Use cases

  1. Object change listeners will now only be called if the object actually changed. It should reduce the number of unnecessary updates which is happening right now due to the change notification being table based.

  2. It should make it possible to register listeners for a single field value, e.g. if the UI should only be refreshed if a single field changed.

  3. It should be possible to register a listener on an object and be give the old and new values, e.g. useful if you only need to refresh the UI if a String changed in a particular way.

  4. It should now be possible to be notified when an object is deleted. See findFirstAsync is not emitting object when queried object is deleted from Realm #3138. Realm classes only implementing "RealmModel" might be tricky to support.

  5. We should strive for easy integration with Databinding https://developer.android.com/topic/libraries/data-binding/index.html. How this should be done is still undecided.

This issue consists of the following tasks:

  • Design the public API for this.
    • It is not possible to register listeners on linked objects (verify this).
    • Changes to linked objects and lists cannot be reported except "they changed" (verify this).
    • Figure out how to best parse the map of new and old values for primitive values.
    • Should ideally support Databinding out of the box or at least make integration very easy.
    • Should work for both classes that extends RealmObject and implements RealmModel.
  • It is unclear at this point if we depend on Integrate OS Results & Async query #3834 being merged first. This needs to be clarified
  • Implement API based on the Object Store implementation
  • Javadoc
  • Unit tests
  • Website documentation
@beeender
Copy link
Contributor

beeender commented Jan 24, 2017

This has been almost implemented with #3834 but gets reverted to make PR smaller. When start this, check 99fd787 fist. It is the commit removed the RowNotifier.

@zaki50
Copy link
Contributor

zaki50 commented Jan 25, 2017

Does this API provide an ability to get notification when a linked object is changed?

@cmelchior
Copy link
Contributor Author

I don't believe so. If I remember correctly that is the default behaviour on .NET and the rationale being that if you care about changes to a linked object you should register the listener there instead.

That argument makes sense, although I can see supporting it would probably lead to a more natural syntax in code:

// Version A: Listeners on each object
Person p = getPerson();
p.addFieldChangeListener("name", new RealmFieldChangeListener() {
  public void onChange(Person p, ChangeSet change) {
    updateUI(p);
  }
});
p.getDog().addFieldChangeListener("name", new RealmFieldChangeListener() {
  public void onChange(Dog d, ChangeSet change) {
    Person p = d.getOwner(); // Either backlink or direct link required
    updateUI(p);
  }
}

// Version B: Linked fields supported (we would essentially implement the logic above behind the scene
Person p = getPerson();
RealmFieldChangeListener listener = new RealmChangeListener() {
  public onChange(Person p, ChangeSet change) {
    updateUI(p);
  }
};

// B1
p.addFieldChangeListener("name", listener);
p.addFieldChangeListener("dog.name", listener);

// B2
p.addFieldChangeListener(listener, "name", "dog.name");

// B3
p.addFieldChangeListener(new String[] { "name", "dog.name" }, listener);

Navigating back to parents automatically would require backlink support though. We already have that in Core, so theoretically we could probably do it without exposing it in the public API first.

In any case, I think supporting linked fields should probably be left for a 2nd iteration of this.

@zaki50
Copy link
Contributor

zaki50 commented Jan 27, 2017

@cmelchior In version A, users need to keep the reference to Dog instance as well to prevent the object will be GCed, right?
If so, I prefer Version B.

But yes, this should be supported in another issue.

@beeender
Copy link
Contributor

also it should be triggered when the object gets deleted. See #3138

@beeender beeender mentioned this issue Mar 9, 2017
24 tasks
@beeender beeender self-assigned this Mar 15, 2017
beeender added a commit that referenced this issue Mar 16, 2017
- Add RowObject for wrapping Object Store's Object class.
- Enabled detailed object notification.
- Add RealmObjectChangeListener, ObjectChangeSet.
- Fix false positive change notifications for RealmObject. Now the
  notification will only be sent when the object actually changes.

See #4101
@beeender
Copy link
Contributor

beeender commented Mar 16, 2017

beeender added a commit that referenced this issue Mar 30, 2017
See #4101

- Add ObjectChangeSet & RealmObjectChangeListener.
- Add OsObject to wrap ObjectStore's Object for notifications.
- No more false positive notifications for RealmObject.
- Use ObserverPairList in ProxyState instead of normal list to solve the
  potential listener removal problems which is handled well by the
  ObserverPairList.
- Fix tests.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants