Skip to content

Avoid modifying managed RealmList when user executes a query.#3812

Open
zaki50 wants to merge 4 commits intoreleasesfrom
my/issue3809
Open

Avoid modifying managed RealmList when user executes a query.#3812
zaki50 wants to merge 4 commits intoreleasesfrom
my/issue3809

Conversation

@zaki50
Copy link
Copy Markdown
Contributor

@zaki50 zaki50 commented Nov 17, 2016

WIP

In model's constructor, calling mutator methods of managed RealmList must be ignored if getAcceptDefaultValue$realm() is false or the name of the field is in the ignore list.

However, RealmList has many methods which is meant to modify itself and that is very hard to provide a natural non-op behavior. Of course it breaks expected List behavior, too.

For example:

private RealmList<Dog> dogs;

// default constructor of DogOwner model
public DogOwner() {
    dogs = new RealmList<Dog>();
    dogs.add(new Dog());

    Dog dog = dogs.get(0); // this line throws an exception or returns unexpected object when a code above is non-op.
}

For that reason, I'm leaning toward adding a method public static boolean RealmObject#acceptsDefaultValue(RealmModel) which returns false when the assigned values are ignored in the constructor.
And when that method returns false, all mutator methods in RealmList throws an exception (this is not an breaking chance since we are already throwing IllegalStateException now).

After introducing acceptsDefaultValue(RealmModel), users can skip any assignment of default values like:

private RealmList<Dog> dogs;

// default constructor of DogOwner model
public DogOwner() {
    if (!acceptsDefaultValue(this)) {
        return;
    }
    dogs = new RealmList<Dog>();
    dogs.add(new Dog());

    Dog dog = dogs.get(0);
}

As Christian commented on #3812 (comment), I think we can automatically add the check by our transformer in most cases. We need to check that adding the code to the following constructor is valid.

public class Foo extends RealmObject {
    final int BAR;
    public Foo() {
        BAR = 1;
    }

    public void printBar() {
        Log.d("baz", "BAR is " + BAR ); // This must print "BAR is 1"
    }
}

fixes #3809

@zaki50 zaki50 changed the title Avoid modifying managed RealmList when user executes s query. Avoid modifying managed RealmList when user executes a query. Nov 22, 2016
@beeender
Copy link
Copy Markdown
Contributor

beeender commented Nov 29, 2016

Remind me why do we need to transform all the getter/setters in the constructor? Can we just skip the transforming for the constructor (at least the default one)?

Ask user to have check of acceptsDefaultValue in the model constructor doesn't look good :(

@zaki50
Copy link
Copy Markdown
Contributor Author

zaki50 commented Nov 29, 2016

@beeender mainly for Realm.createObject(Class) to respect the default values.

@kneth
Copy link
Copy Markdown
Contributor

kneth commented Nov 29, 2016

You have provided a workaround to #3809, but I believe that Realm Java should be as easy to use as possible. From that point of view, I think this proposal makes sense.

@cmelchior
Copy link
Copy Markdown
Contributor

cmelchior commented Nov 29, 2016

I would really like to avoid anything like the above. It doesn't really solve the problem of being intuitive to use since you would need to read the documentation for both this and doing the work-around in #3809. If given a choice between the two I would probably prefer to just document the current bug and how to work around it instead of introducing new methods of questionable value?

However, could it be possible to inject that code using our transformer? That way people wouldn't need to do anything?

@zaki50
Copy link
Copy Markdown
Contributor Author

zaki50 commented Nov 29, 2016

@cmelchior That's a great idea.
I think it's possible.

@zaki50
Copy link
Copy Markdown
Contributor Author

zaki50 commented Feb 21, 2017

Adding if (!acceptsDefaultValue(this)) { return; } is possible but the following code does not work as expected.

public class Foo extends RealmObject {
    @Ignore
    final int BAR;
    public Foo() {
        BAR = 1;
    }

    public void printBar() {
        Log.d("baz", "BAR is " + BAR ); // This must print "BAR is 1"
    }
}

printBar() prints 0 instead of 1.

My current conclusion is that we should not inject if (!acceptsDefaultValue(this)) { return; } by our transformer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants