Realm-java: Old/New values pairs for Single Object Notifications

[beeender]

This is for following enhancement of #4101 which are not implemented in #4331

Cocoa API will notify user when the object changes with old/new value pair:

@interface RLMPropertyChange : NSObject

/**
 The name of the property which changed.
 */
@property (nonatomic, readonly, strong) NSString *name;

/**
 The value of the property before the change occurred. This will always be `nil`
 if the change happened on the same thread as the notification and for `RLMArray`
 properties.

 For object properties this will give the object which was previously linked to,
 but that object will have its new values and not the values it had before the
 changes. This means that `previousValue` may be a deleted object, and you will
 need to check `invalidated` before accessing any of its properties.
 */
@property (nonatomic, readonly, strong, nullable) id previousValue;

/**
 The value of the property after the change occurred. This will always be `nil`
 for `RLMArray` properties.
 */
@property (nonatomic, readonly, strong, nullable) id value;
@end

By extending current java interface ObjectChangeSet, realm-java could have something similar.

The proposed API 1:

public interface ObjectChangeset {
  boolean isDeleted();
  String[] getChangedField();
  isFieldChanged(String fieldName);

  public static class FieldChange<T> {
    T oldValue;
    T newValue;
  }

  // This will always return null for RealmList field.
  <E> FieldChange<E> getChange(E type, String fieldName);
}

The proposed API 2:

public interface ObjectChangeset {
  boolean isDeleted();
  String[] getChangedField();
  isFieldChanged(String fieldName);

  Long getOldLongValue(String fieldName);
  Long getNewLongValue(String fieldName);
  Date getOldDateValue(String fieldName);
  Date getNewDateValue(String fieldName);
  // ... Other getters
}

Potential problems & limitations:

• [ ] Storing old values requires memory and potentially both memory & cpu consuming operation since there might be big strings needed to be converted from StringData to Java String. We may want to give user an option to disable the old/new values in the notification. Something like RealmObject.addChangeListener(RealmObjectChangeListener listener, boolean needValues).
• [ ] Same like cocoa, the value of the property before the change occurred. This will always be nil
  if the change happened on the same thread as the notification.
• [ ] All primitive types will be returned as boxed type. This is because of we have to use generic for FieldChange<T>.
• [ ] short/byte/int/long values will always be reported as FieldChange<Long> since they are all stored as long underneath. From this perspective, the API 2 might be safer for users.

[cmelchior]

1) Allowing opt-in of the new/old values sounds like a good idea. Having a boolean parameter is probably fine, although we should probably provide a default value for it:

obj.addChangeListener(new RealmObjectChangeListener() { ... }) ; // Default to sending the new/old?
obj.addChangeListener(new RealmObjectChangeListener() { ... }, false) ; // Disable new/old calculation.

Trying to do something like obj.addNoCompareChangeListener(new RealmObjectChangeListener() { ... } looks very cumbersome at least.

2) Having the diff being null for local commits still makes no sense to me. It means you need to coordinate between the change listener and you commit instead of being able to keep them separately. It sounds more like a limitation of the current implementation to me, but I'm fine with shipping with the same limitation as Cocao and then we can re-evaluate.

3) If people know they want a Long, they should be able to do getFieldChange(Short.class, "field")?

Also we should IMO ship isFieldChanged(String fieldName); as part of #4331, it doesn't have to be part of this.

[beeender]

If people know they want a Long, they should be able to do getFieldChange(Short.class, "field")?
Yes, we can do that. The only problem is we won't be able to do type checks:

public class extends RealmObject {
    public int age;
}

// We can convert the long to short here, but we are not able to warn user that the original field is an int
getFieldChange(Short.class, "age");

[zaki50]

I think obj.addChangeListener(new RealmObjectChangeListener() { ... }) ; should be identical to obj.addChangeListener(new RealmObjectChangeListener() { ... }, false) ;. Opt-in for old/new value is better. When user call ObjectChangeset.getChange(...) without enabling old/new value feature, we can throw IllegalStateException since it's a program error (and user is able to know the feature is opt-in).

[cmelchior]

After initial investigation by @beeender, it was determined that this feature would need to undergo major rewrite after moving the our schema to the Object Store, so we are moving this out of scope for 3.2 and waiting for https://github.com/realm/realm-java/pull/3800 to be complete instead

[pankajsoni19]

Original query 5451

Any update on this? This could really help in alot of places to us.
Else will have to resort to keeping object copy and using diff utils in onchange. This mutes the point of the change listener

[pankajsoni19]

Will this be worked on?

[cmelchior]

At some point, yes, but I cannot give you any timeline right now.