realm-dotnet icon indicating copy to clipboard operation
realm-dotnet copied to clipboard

Raise PropertyChanged event immediately when in a transaction, and re-raise if transaction is rolled back

Open nielsenko opened this issue 7 years ago • 6 comments

Feature request - I almost think it is a bug :-)

Today (at least with Realm 3.0.0) PropertyChanged are not raised until the transaction is committed on the thread owning the transaction. In this all realms are treated equal including the realm that has the transaction.

For short lived transaction you will hardly notice, but for transaction that lives longer, fx. to support complex undo scenarios, this is a problem, since the UI will not automatically update while the transaction is ongoing :-/

I would suggest that while in a transaction, the PropertyChanged is raised immediately on the owning thread and re-raised, if the transaction is rolled back. All other threads should raise event when the transaction is committed.

I think this would be a better fit with the expectations of .NET programmers, and would make it trivial to use Realm to implement complex undo functionality, by taking advantage of long running transactions, combined with realms deterministic and automatic conflict resolution, that will ensure that conflicts are always resolved - which is where Realm really shines compared to the competitors.

I have have included a test-case to illustrate the "problem":

`

public class Dork : RealmObject
{
	public int State { get; set; }
	public int ShadowState { get; set; }

	protected override void OnPropertyChanged(string propertyName)
	{
		switch (propertyName)
		{
			case nameof(State):
				if (!IsManaged || Realm.IsInTransaction)
					ShadowState = State;
				else
					Realm.Write(() => ShadowState = State);
				break;
		}
	}
}

public class RealmTests
{
	[Test]
	public void PropertyChangedRaisedEvenIfNotManagedYetTest()
	{
		AsyncContext.Run(async () =>
		{
			var config = new RealmConfiguration("test.db");
			Realm.DeleteRealm(config);
			using (var realm = Realm.GetInstance(config))
			{
				var d = new Dork
				{
					State = 1
				};

				Assert.That(1, Is.EqualTo(d.State));
				Assert.That(d.State, Is.EqualTo(d.ShadowState));

				d.State = 2;
				Assert.That(2, Is.EqualTo(d.State));
				Assert.That(d.State, Is.EqualTo(d.ShadowState));

				realm.Write(() =>
				{
					realm.Add(d);
					d.State = 3;
				});
				Assert.That(3, Is.EqualTo(d.State));
				Assert.That(d.State, Is.Not.EqualTo(d.ShadowState)); // Sadly so :-/

				d.PropertyChanged += (s, e) => { }; // just here to show interest

				using (var transaction = realm.BeginWrite())
				{
					d.State = 4;
					await Task.Yield();
					Assert.That(4, Is.EqualTo(d.State));
					Assert.That(d.State, Is.Not.EqualTo(d.ShadowState)); // Sadly so :-/
					transaction.Commit();
				}
				await Task.Yield(); // otherwise OnPropertyChanged won't be raised! :-/
				Assert.That(4, Is.EqualTo(d.State));
				Assert.That(d.State, Is.EqualTo(d.ShadowState)); // now it works

				realm.Write(() => d.ShadowState = 0); // reset shadow state!
				using (var transaction = realm.BeginWrite())
				{
					d.State = 5;
					transaction.Rollback();
				}
				await Task.Yield();
				Assert.That(4, Is.EqualTo(d.State));
				Assert.That(0, Is.EqualTo(d.ShadowState)); // Sadly so :-/
			}
		});
	}
}

The obvious next question becomes, what about RealmResult and RealmList? Well, I expect that doing this for RealmResult would be near impossible, but that it could be done for RealmList at least to the extend that a simple Refresh event is raised on roll-back.

nielsenko avatar Apr 26 '18 06:04 nielsenko

An easy, if not complete solution, would be to simply introduce a new OnPropertyChanging(string propertyName) virtual method, that is called from RealmObject.SetX methods, just after setting the property, fx:

        [SuppressMessage("StyleCop.CSharp.DocumentationRules", "SA1600:ElementsMustBeDocumented")]
        protected void SetStringValue(string propertyName, string value)
        {
            Debug.Assert(IsManaged, "Object is not managed, but managed access was attempted");

            _objectHandle.SetString(_metadata.PropertyIndices[propertyName], value);
            OnPropertyChanging(propertyName)
        }

This would give the user a place to hook-in and do their own handling

nielsenko avatar Nov 27 '18 12:11 nielsenko

That's a bit tricky though because it will work only for a very limited set of cases and could introduce subtly unexpected behavior. E.g. if you want to manually raise PropertyChanged in the OnPropertyChanging override, you could get into a situation like:

var foo1 = realm.Find<Foo>(1);
var foo2 = realm.Find<Foo>(1);

// foo1 and foo2 are different instances pointing to the same object

foo1.PropertyChanged += (s, e) =>
{
    // Update UI
};

realm.Write(() =>
{
    foo2.Bar = 123;
    // foo1 will not raise property changed because its OnPropertyChanging will not be invoked
    
    // Do other stuff
});

This may not be a problem for individual objects because you can save a reference as a class level variable and pass that to your UI, but it will be quite annoying for collections where every time you access an element you get a new instance.

So while it will be fairly easy to add what you're suggesting, I'm not 100% certain it will address your use case nicely.

nirinchev avatar Nov 27 '18 12:11 nirinchev

I agree. Another problem is rollback. How to catch that? I still find it useful though. Fx. if I want to update property A whenever property B is changed, etc. And if I'm careful, I can orchestrate UI updates during transactions by calling RaisePropertyChanged in OnPropertyChanging.

nielsenko avatar Nov 27 '18 13:11 nielsenko

I would call it a bug that UI updates is bound to transactions. In some scenarios (like undo) it forces me not to bind to RealmObject properties and move these to the view model - or simply rebuild the entire ItemSource.

timahrentlov avatar Dec 19 '18 11:12 timahrentlov

Perhaps you could make a OnPropertyChanging virtual method (and a PropertyChanging event) that is called within the transaction?

This would not break existing semantics, and if I (or @timahrentlov) would like to raise an actual PropertyChanged event within the transaction, then we have the handle to do so by calling the protected RealmObject.RaisePropertyChanged.

Obviously it would also be nice to get a callback on any changed object, if the transaction is rolled back, but I believe the first part should be fairly easy to add.

nielsenko avatar Nov 13 '19 10:11 nielsenko

I was looking for an analogue to an event found in MSAcess called BeforeUpdate(Cancel As Integer) which gives you that cancel token as well as access to an .OldValue property. I believe these features were added to address the issue discussed in this thread.

In C#, the event would come with an EventArgs object that has the cancel token and the old value property. This would give you a chance to hook into changes before they are committed to the DB and allow you to cancel/undo and even do something with the new or old values as needed.

A9G-Data-Droid avatar Mar 30 '23 23:03 A9G-Data-Droid