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

Add support for transient fields #4436

Merged
merged 7 commits into from
Apr 7, 2017
Merged

Add support for transient fields #4436

merged 7 commits into from
Apr 7, 2017

Conversation

cmelchior
Copy link
Contributor

While looking into how to best support databinding, I ran across #4279 , which was easily fixable.
It doesn't make it possible to extend BaseObservable yet, but it is a step in the right direction.

Copy link
Contributor

@bmeike bmeike left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me


if (variableElement.getAnnotation(Index.class) != null) {
if (!categorizeIndexField(element, variableElement)) { return false; }
if (field.getAnnotation(Index.class) != null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would this be clearer if it were an &&, as the previous test is an ||

Copy link
Contributor

@zaki50 zaki50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should skip to transform the accesses to transient field.

Something like:

diff --git a/realm-transformer/src/main/groovy/io/realm/transformer/BytecodeModifier.groovy b/realm-transformer/src/main/groovy/io/realm/transformer/BytecodeModifier.groovy
index d64572ea3..e369ceddc 100644
--- a/realm-transformer/src/main/groovy/io/realm/transformer/BytecodeModifier.groovy
+++ b/realm-transformer/src/main/groovy/io/realm/transformer/BytecodeModifier.groovy
@@ -30,6 +30,10 @@ class BytecodeModifier {
 
     private static final Logger logger = LoggerFactory.getLogger('realm-logger')
 
+    static boolean isModelField(CtField field) {
+        !field.hasAnnotation(Ignore.class) && !Modifier.isTransient(field.getModifiers()) && !Modifier.isStatic(field.getModifiers())
+    }
+
     /**
      * Adds Realm specific accessors to a model class.
      * All the declared fields will be associated with a getter and a setter.
@@ -40,7 +44,7 @@ class BytecodeModifier {
         logger.debug "  Realm: Adding accessors to ${clazz.simpleName}"
         def methods = clazz.getDeclaredMethods()*.name
         clazz.declaredFields.each { CtField field ->
-            if (!Modifier.isStatic(field.getModifiers()) && !field.hasAnnotation(Ignore.class)) {
+            if (isModelField(field)) {
                 if (!methods.contains("realmGet\$${field.name}".toString())) {
                     clazz.addMethod(CtNewMethod.getter("realmGet\$${field.name}", field))
                 }
diff --git a/realm-transformer/src/main/groovy/io/realm/transformer/RealmTransformer.groovy b/realm-transformer/src/main/groovy/io/realm/transformer/RealmTransformer.groovy
index d7ca24a76..9a9ee08ac 100644
--- a/realm-transformer/src/main/groovy/io/realm/transformer/RealmTransformer.groovy
+++ b/realm-transformer/src/main/groovy/io/realm/transformer/RealmTransformer.groovy
@@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableSet
 import com.google.common.collect.Sets
 import com.google.common.io.Files
 import groovy.io.FileType
-import io.realm.annotations.Ignore
 import io.realm.annotations.RealmClass
 import javassist.ClassPool
 import javassist.CtClass
@@ -30,7 +29,6 @@ import org.gradle.api.Project
 import org.slf4j.Logger
 import org.slf4j.LoggerFactory
 
-import java.lang.reflect.Modifier
 import java.util.jar.JarFile
 import java.util.regex.Pattern
 
@@ -121,7 +119,7 @@ class RealmTransformer extends Transform {
         def allManagedFields = []
         allModelClasses.each {
             allManagedFields.addAll(it.declaredFields.findAll {
-                !it.hasAnnotation(Ignore.class) && !Modifier.isStatic(it.getModifiers())
+                BytecodeModifier.isModelField(it)
             })
         }
         logger.debug "Managed Fields: ${allManagedFields*.name}"

@cmelchior
Copy link
Contributor Author

Good catch @zaki50. I'll adjust that.

@cmelchior
Copy link
Contributor Author

Updated @zaki50

Copy link
Contributor

@zaki50 zaki50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
But still needs another 👍 since this PR contains my code 😃

Copy link
Contributor

@beeender beeender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RealmObject.java javadoc needs to be fixed:

The only restriction a RealmObject has is that fields are not allowed to be final, transient' or volatile.

Other than this, 👍

Copy link
Contributor

@beeender beeender left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@cmelchior cmelchior merged commit 66ccab2 into master Apr 7, 2017
@cmelchior cmelchior deleted the cm/transient-fields branch April 7, 2017 09:13
@cmelchior cmelchior removed the S:Review label Apr 7, 2017
@cmelchior cmelchior mentioned this pull request May 9, 2017
3 tasks
@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

Successfully merging this pull request may close these issues.

4 participants