realm / realm-java

Realm is a mobile database: a replacement for SQLite & ORMs
http://realm.io
Apache License 2.0
11.44k stars 1.75k forks source link

Finegrained notifications #989

Closed Trikke closed 7 years ago

Trikke commented 9 years ago

Hi Realm devs,

we now have the RealmChangeListener to listen for changes if some background thread updates the Realm instance, which is great. But i'd like to put a feature request forward for more fine grained control when a listener can be called. It would be nice if it were possible to add a RealmChangeListener that only fires if a certain class of RealmObject is updated.

The issue i have now is that i have multiple fragments for multiple classes of RealmObject (eg. RealmUser, RealmMessage, RealmBeer) open at once, then a background services updates one of the RealmUser instances. Now all the RealmChangeListener are fired, and all fragments are updated individually. While ideally only the one that deals with RealmUser should be updating.

cmelchior commented 9 years ago

Hi @Trikke Yes. more finegrained notifications are also very high on our priority list, and we already have someone working on it. Unfortunately it require some changes to the core database, but you will hopefully see some progress on this soon.

feugatos commented 9 years ago

Hi @cmelchior

Will this enhancement also include some sort of support for adding RealmChangeListeners to RealmResults<> as well and get notifications when a RealmObject referenced by the RealmResults<> gets updated?

This would be a very useful feature and would surely improve the performance of RealmBaseAdapter. For instance the number of draw calls/redraws to a bound ListView would be vastly reduced.

Thanks in advance, Dimitris.

PS. Is there a roadmap or expected release date for this feature?

rynti commented 9 years ago

+1

treylon commented 9 years ago

+1

bdbergeron commented 9 years ago

I'm also anxiously awaiting this feature. In my opinion, this is definitely one of the features that needs to be implemented before a 1.0 release!

yolapop commented 9 years ago

I'm also excited with this feature :+1:

Btw @feugatos, can you explain what you mean by this?

This would be a very useful feature and would surely improve the performance of RealmBaseAdapter. For instance the number of draw calls/redraws to a bound ListView would be vastly reduced.

beeender commented 9 years ago

Comments from #1378 by mshusek.

When we use RecyclerView or we want reload view under certain conditions default onChange is not sufficient. Now, in RecyclerView we have to reload visible rows so it is not a optimal solution. Moreover, we can not use animation when item is delete or insert to our model.

I think better sollution will be implementing something like that:

 ContentObserver.ModelChangeListener() 
{
    @Override
    public void onModelChanged() { 
    } 
    @Override
    public void onModelUpdated(Object obj) {
    }
    @Override
    public void onModelDeleted(Object obj) {
    }
    @Override
    public void onModelInserted(Object obj) {
    }
};
gpulido commented 9 years ago

Another +1 for have this feature implemented. Alongside with a RealmAdapter for recyclerviews those will be an amazing plus to an already great library!

cmelchior commented 9 years ago

Work-in-progress API proposal:

Design criteria:

Open questions

//
// Realm API
//

RealmChangeListener listener = new RealmChangeListener() {
  @Override
  public void onChange() {
    // change detected
  }
}

// Realms
realm.addChangeListener(listener);

// Query results
RealmResult<Foo> results = realm.allObjects(Foo.class);
results.addChangeListener(listener);

// Lists
RealmList<Bar> list = realm.where(Foo.class).findFirst().getBarList();
list.addChangeListener(listener);

// Objects
// Note: This will only work if we keep a RealmObject base class which is debatele. 
Foo obj = realm.where(Foo.class).findFirst(); 
obj.addChangeListener(listener);

// Alternative object listener
realm.addChangeListener(obj, listener);

// KVO support, i.e. observing on individual properties
String keyPath = "fieldName"; // Name of field
keyPath = "bar.childField"; // Name of child field
keyPath = "bar[0]"; // What about RealmList entries?

// Option 1
obj.addChangeListener(keyPath, new RealmPropertyChangeListener() {
  @Override
  public void onChange(Object oldValue, Object newValue) {
    // Types is an issue.
  }
});

// Option 2
obj.addChangeListener(keyPath, new RealmChangeListener() {
  @Override
  public void onChange() {
    // Has to ask for the value. No access to the previous value.
  }
});

//
// RecyclerView.Adapter
//
// Enabling animations in the RecyclerViewAdapter also means we need to support information
// about how things has changed in RealmResults/RealmList

notifyItemChanged(int position)
notifyItemChanged(int position, Object payload)
notifyItemInserted(int position)
notifyItemMoved(int fromPosition, int toPosition)
notifyItemRangeChanged(int positionStart, int itemCount)
notifyItemRangeChanged(int positionStart, int itemCount, Object payload)
notifyItemRangeInserted(int positionStart, int itemCount)
notifyItemRangeRemoved(int positionStart, int itemCount)
notifyItemRemoved(int position)
beeender commented 9 years ago

@cmelchior Any use case for "Should closing a Realm trigger a onChange?"

cmelchior commented 9 years ago

Yes, like removing references to objects that would otherwise crash or to reset UI state.

yaming116 commented 8 years ago

@cmelchior Whether you can provide database changes, tell me which table changed。

RealmChangeListener listener = new RealmChangeListener() {
  @Override
  public void onChange(Class changeClass) {
    // change detected
  }
}
waltarmour commented 8 years ago

Yes, I would definitely like to see a way to catch changes to specific classes of objects. However, I'm not sure about the exact signature from @yaming116 . What happens when a single transaction changes multiple classes? Would we get several simultaneous onChange() calls? Maybe, if this approach was used, it could be a Class[] or something to allow for a single invocation.

Of course, the desired behaviour could be synthesized using the query results change listener but that seems a bit hacky at first glance. Not sure in the long run.

dadino commented 8 years ago

@waltarmour @cmelchior it could be something like this.

RealmChangeListener listener = new RealmChangeListener(Class[] classes) {
  @Override
  public void onChange() {
      // change detected
  }
}

Or something much more precise, using PrimaryKey (but it may be too much):

class ChangedClass{
    Class class;
    ChangedEntity[] entities;
}

class ChangedEntity{
    Object primaryKeys;
    int change; //created, updated, deleted
}

RealmChangeListener listener = new RealmChangeListener(Class[] classes) {
  @Override
  public void onChange(ChangedClass[] changed) {
      // change detected
  }
}

With this one the RecyclerView animations should be easy. I don't know how it could work without a PrimaryKey.

Also, don't use RealmList/RealmResult/RealmObject .addChangeListener(). Use realm.addChangeListener(RealmList/RealmResult/RealmObject, listener) instead and get the needed classes for the listener from the first parameter.

eneim commented 8 years ago

Thanks @cmelchior for taking me here. In my opinion, using Generic Type for Change Listener would be more sense:

public abstract class RealmChangeListener<T> {

    /**
     *
     * @param objects the changed objects (added objects as well as updated objects)
     */
    public void onChange(T... objects);
}
carotorrehdz commented 8 years ago

+1

andresgarza commented 8 years ago

:+1:

joseffilzmaier commented 8 years ago

:+1:

andresgarza commented 8 years ago

:+1:

beeender commented 8 years ago

Move comments from #2560

1 Make a way to add WeakRef of change listeners to RealmResults, and generalize it as a public API. User might need it as well for their own public Api. 2 Subscribe to RealmResults instead of Realm to avoid false positive notifications in the RealmBaseAdapter 3 Check if we can do the same to RealmList.

SandNerd commented 8 years ago

I saw RealmPropertyChangeListener proposed above but I'm not sure if you have plans to implement it. We are attempting to use state machines within our realm models and so something like that can be really helpful, especially if that is cheaper for Realm core.

cmelchior commented 8 years ago

We have not settled on an API yet, but some sort of single field listener will most likely come, as that is similar to KVO on iOS

cmelchior commented 8 years ago

For future reference: http://www.xmailserver.org/diff2.pdf https://developer.android.com/reference/android/support/v7/util/SortedList.html

marchy commented 8 years ago

Any ETA on when this is going to achieve feature parity with iOS?

This is a pretty big deal and has big impacts architecturally on how consuming apps should be implemented. Would be great to know if it's expected soon (next month-or-two), later in the year or unknown/later. Cheers!

cmelchior commented 8 years ago

Fine-grained notifications are a P1 for us, which means it is something we are working actively on. However right now it require #1445 being done first since that contains all the underlying logic. A best guess right now is that it is 2-3 months away. But that is a very rough estimate, so most likely it will take a bit longer. Definitely this year though.

marchy commented 8 years ago

@cmelchior thanks a lot for the rough ballpark and dependency highlighting Christopher. That helps us make the necessary decisions architecture-wise. Good luck to the team with it!

usernotnull commented 8 years ago

is the RealmObject.addChangeListener() working as intended or is that what this post is about?

I'm facing an issue when I add a change listener to a class implementing RealmModel. If I restore a backup, the change listener is not being called ( both if that object was changed with the backup or remained the same). If I add the change listener to the Realm instance as a whole, I don't face this issue but i figured to implement a custom .equals() to check if that object has changed before updating my fragments. That's the long way around until I figure out why isn't the changeListener being called when bound to an object.

cmelchior commented 8 years ago

RealmObject.addChangeListener is working as intended, although a bit corse-grained right now. Note that change listeners are only triggered as long as the object is not GC'ed so e.g. the following pattern is invalid:

public void test() {
  Person person = realm.where(Person.class).findFirst();
  person.addChangeListener(listener); // person might be GC'ed
}

// Instead do

Person person; // Class variable
public void test() {
  person = realm.where(Person.class).findFirst();
  person.addChangeListener(listener); // person is no longer GC'ed as long as the class is alive.
}

If that doesn't help, please create another issue for it.

royale1223 commented 8 years ago

Any ETA on this?

Zhuinden commented 7 years ago

... Huh? Won't this be a breaking change? Surprised it's not a 2.0 milestone.

loudenvier commented 7 years ago

WOW! 2.0.0 is now released and still no fine grained notifications? I've stopped using realm notifications altogether in my own code and created a RealmNotificationHub which I call for every and each changed object. It is a general purpose notification hub in which listeners can subscribe to specific classes and even specific instances. It requires a primary key, which, IMHO, every single object should have. The fact is: it shouldn't be my concern to create all of this. When Realm promises reactive programming it should have fine grained notifications from the start. I don't even need field-level change notifications (but I can see it would be very useful to some people), but not being able to check which actual object changed or, even worse, if you add a change listener to a single object even if another object is changed, the listener will be called and you won't be able to be sure it was a change on the object the listener was added to, or in another object of the same class.... unless you keep state for every single object you are listening too... it was way easier to engineer the notification hub instead of using realm notifications... I was struggling against the framework.

I believe the situation will be a LOT worse for users of the Realm Mobile Platform... If they use Recycler Views they will find themselves calling adapter.notifyDatasetChange instead of notifyItemChange whenever a single String field is changed remotely....

Zhuinden commented 7 years ago

@loudenvier does the realm notification hub rely on synchronous transactions on the UI thread?

loudenvier commented 7 years ago

@Zhuinden the listeners get the notification on the UI thread, but the dispatching mechanism uses a background thread. It is very stupid and dumb, since it does not automate many things (which it could if it was worth). Since you are responsible to "notify" the hub that an object was changed, and it does not work "automatically" it was only a matter of creating a background task and a synchronized queue with the "notified" changes and then notify each listener on the UI thread, consuming this queue. That's why I don't know why Realm didn't provide a simple "per object" notification mechanism. It would be a piece of cake! They certainly know which objects are being changed when commit is called. Granted it would be cool to have the previous and new values of fields, but that's something for a Unit of Work pattern, which I find overkill, as it would have to keep track of previous field values, and in a memory restricted environment as is found in many mobile devices it could easily create out of memory situations. I just don't wank to have a notification attached to an object being called by any modification occurring on other objects other than that specific one.

Zhuinden commented 7 years ago

It's a shame the Sync came in sooner, because now Realm-Android is behind Realm-Xamarin in fine-grained notifications 😮

biajoeknee commented 7 years ago

@cmelchior I don't think RealmChangeListener.onChange() is being called when I set an Integer field in the RealmObject to null by doing <RealmObject subclass>.setField(null). Is that expected behavior?

Zhuinden commented 7 years ago

If the table is modified, then the change listener should be called after commit at some point (when the Realm handles the notification)

biajoeknee commented 7 years ago

@Zhuinden I also execute an async transaction right before the async transaction that sets the field null -- i cannot combine the async transactions into one. Is it possible that the second notification is somehow eclipsed by the first notification, causing onChange() to not get called for the second change?

biajoeknee commented 7 years ago

@Zhuinden I don't think that the second notification is somehow eclipsed because I have the same pattern elsewhere in my code -- executing two async transactions right after one another, -- but in this case the second transaction doesn't set a field null, it calls RealmObject.deleteFromRealm() on a RealmObject field, and RealmChangeListener.onChange() is called twice; once for each transaction. I think this is somehow related to setting a field null, maybe also having to do with the fact that the field is also of Integer type.

Teja-Konjeti commented 7 years ago

@cmelchior Will something similar to Realm Xamarin Fine Notifications be implemented soon? Any ETA? That feature on Xamarin Platform is exactly the one needed. Right now updating the Recycler View and writing my own code for finding which row got updated is getting very cumbersome for me which is the reason I am looking to migrate to Realm. Do provide me with an ETA. Thanks! :)

Edit: I just checked and all the other Realm platforms have the fine grained notifications except for Java. Hope it will be implemented soon.

Zhuinden commented 7 years ago

@Sai-Teja Collection Notifications depend on https://github.com/realm/realm-java/pull/3834 which were started a few days ago, so it's bound to happen

Teja-Konjeti commented 7 years ago

@cmelchior I mean this request is one of the most important reasons people use orms. It has been put up for one and a half year now. This is there in all the other platforms of Realm and it is the basic feature which should have been there before the release of v1.0 and now this library is in 2.2 I would prefer to know the ETA of this feature as this is why I want to use orm other than SQLite other wise I might use some other ORM. So, do comment if you can specify an ETA. Nd @Zhuinden The third comment in #3834 is saying something else anyways if you can do give an ETA! :)

Zhuinden commented 7 years ago

@Sai-Teja that comment is responding to me, that "no, that is a different problem". Although I still think that might fix those issues in one go :tongue:

The object store integration is a complex thing. It will be done eventually.

I am not actually working on it so I do not know the exact ETA.

cmelchior commented 7 years ago

@Sai-Teja To my knowledge, no ORM provide fine-grained notifications to the level described in this issue. "Table"-based notifications (some object of type A changed) are supported by e.g SQLBrite (and possibly others), but Realm also has support for that.

This issue is for tracking changes to collections (added/removed/moved) + changes to single objects (fields changed). If you are looking for support for that today, DiffUtil (https://developer.android.com/reference/android/support/v7/util/DiffUtil.html) can be used for the collection changes no matter what DB solution you use.

Regarding ETA. All I can say is that we are working actively on this, but we don't have a timeline we can share yet.

Teja-Konjeti commented 7 years ago

@cmelchior I just wanted to correct "no ORM provide fine-grained notifications to the level described in this issue" DBFlow has finegrained notifications so I'm going for it now! I'll watch this thread for updates then! I'm not against realm It's just this is the main feature I'm looking for! :)

evgeny-sureev commented 7 years ago

@cmelchior Realm Swift provides such fine-grained notifications on collections and on single objects via KVO: https://realm.io/docs/swift/latest/#notifications

Zhuinden commented 7 years ago

@evgeny-sureev Realm Java depends on https://github.com/realm/realm-java/pull/3834 to add fine-grained notifications

eygraber commented 7 years ago

Has an API been decided for this yet? Will it look something like ListUpdateCallback`?

cmelchior commented 7 years ago

@eygraber There is a proposal in #3977 . Right now it is closer to the API from RecyclerView, but it doesn't seem much different from what you linked either. We are still looking for feedback, so if you have a use case not covered in #3977 we would love to hear about it.

beeender commented 7 years ago

For anyone who may want to try this feature before we release it: The PR #4191 to enable this feature has been merged, the snapshot will be available in a few hours which will include this feature as well. For how to use the fine grained notification with recycler view, see sample code here https://github.com/realm/realm-android-adapters/pull/83/files#diff-5db60168121b71f755fdff3c780e4c3dR56 .

The listeners on RealmList will be available in the next release as well. See https://github.com/realm/realm-java/pull/4216 .

The next release won't have fine grained notification for the object level. That has been tracked by another issue #4101 .

Feel free to open a new issue if you meet problems when trying it with snapshot.

Thanks for all your patience.