realm / realm-java

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

Support Kotlin delegated properties for Realm objects #4185

Open marchy opened 7 years ago

marchy commented 7 years ago

Realm objects do not support Kotlin delegated properties, making it impossible to add custom setters (ie: observable) to Realm properties.

This seems to be due to the limitation that the annotation processor tries to prevent final fields from being mapped to Realm fields. Instead it should be made smart enough to recognize if the value is a delegated property and allow the delegate to assign the value on its own. The delegated object will take care of storing the value internally and doing the right thing.

Note the related issue https://github.com/realm/realm-java/issues/2921 which makes it unable to assign the "@Ignore" annotation to a delegated property either (big limitation) which means you cannot use delegated properties for even NON-Realm properties. This related issue should be a P1 since it effectively makes Realm incompatible with Kotlin - as is likely less effort than adding proper support for delegated properties.

cmelchior commented 7 years ago

Hi @marchy

I was looking a bit at this. A few observations:

First:

@Ignore open var foo: String by Delegate()

Is not prevented by Realm. Android Studio tells me "[WRONG_ANNOTATION_TARGET] This annotation is not applicable to Member property with Delegate". I don't fully understand yet why since our annotation has the ElementType.FIELD target specifier. Possible some limitation in Kotlin?

Second: Given this sample class:

open class Dog : RealmObject() {
    open var del: String by Delegate()
}

The resulting java code is

public class Dog extends RealmObject {
   @NotNull
   private final Delegate del$delegate = new Delegate();
   // $FF: synthetic field
   private static final KProperty[] $$delegatedProperties = new KProperty[]{(KProperty)Reflection.mutableProperty1(new MutablePropertyReference1Impl(Reflection.getOrCreateKotlinClass(Dog.class), "del", "getDel()Ljava/lang/String;"))};

   @NotNull
   public String getDel() {
      return this.del$delegate.getValue(this, $$delegatedProperties[0]);
   }

   public void setDel(@NotNull String <set-?>) {
      Intrinsics.checkParameterIsNotNull(<set-?>, "<set-?>");
      this.del$delegate.setValue(this, $$delegatedProperties[0], <set-?>);
   }
}

To me it looks like the easiest way to support delegates would just be to treat any field with the Delegate class as automatically having the @Ignore annotation? Since the delegate has setter/getter methods, it will be responsible for getting/storing the data and if it does this by using another field in the Realm Model class that should work just fine.

That would also conveniently get us around the first problem. Would this solve your problem?

cmelchior commented 7 years ago

Digging a little further. Detecting when something is a delegate is not straight forward it seems: https://kotlinlang.org/api/latest/jvm/stdlib/kotlin.properties/-read-write-property/

It looks like we cannot just check if a class implements the ReadWriteProperty since that is optional. That leaves us with a few ways of solving it:

1) Just detect the naming pattern "$delegate" . This means we depend on an implementation detail though that might change without notice.

2) Detect if the Delegate class implements ReadWriteProperty. People are not required to implement ReadWriteProperty though.

3) We need to check for the presence of two methods in the class: public String getValue(@NotNull Dog thisRef, @NotNull KProperty property) and public void setValue(@NotNull Dog thisRef, @NotNull KProperty property, @NotNull String value)

Neither of the 3 is 100% foolproof although I would guess the chance of the last one actually matching a wrong class as being virtually 0.

cmelchior commented 7 years ago

Some empirical testing shows that it isn't possible to annotate a delegated field, period. I tried allowing every ElementType option for the annotation and it still wasn't possible to add it. This means that this problem needs to be solved by one or some of the above options.

kneth commented 7 years ago

@marchy Have you had a chance to try any of the options?

marchy commented 7 years ago

@kneth I don't think this is on us to try the options. These options were referring to the Realm team implementing the annotations in different ways, right?

cmelchior commented 7 years ago

Yes, the above is something that we must implement

davidgarciaanton commented 7 years ago

Hi,

I was thinking that something like annotating a whole type to be ignored by realm could work.

Maybe I'm wrong and you've considered that too, but adding a new annotation like for instance @IgnoreType targeting just Types would render any property of that type to be @Ignored.

May that work? How does it sound to you @cmelchior ?

davidgarciaanton commented 7 years ago

Hi Again

Actually I think that it currently is supported annotating a property as follows

@delegate:Ignore val whatever by Delegate()