@Retained @Arg

[Codewaves]

@Retained @Arg is not working as expected. As I understand, argument must be restored into variable only when savedInstanceState is null. Otherwise it will always overwrite retained value. Retained value must have higher priority over argument, except first time activity/fragment is created.

P.S. Thanks for great library.

[tom91136]

Acknowledged, working on a fix. If I remember correctly, this should be as trivial as moving a bock of code.

[tom91136]

You can try the snapshot with:

repositories {
    //...
    maven { url "https://jitpack.io" }
}
dependencies {
    compile 'com.github.tom91136.akatsuki:akatsuki-api:4c7eeb459a'
    apt 'com.github.tom91136.akatsuki:akatsuki-compiler:4c7eeb459a'
}

The first sync will take about 5 minutes because it is built lazily

[iainconnor]

@tom91136 Doesn't appear to be working. I believe you should be calling Activity.getIntent().removeExtra() as opposed to Activity.getIntent().getExtras().removeExtra(), as the latter creates a new copy of the extras each time.

However, I still don't think that's going to fix things completely. There's no sort of containsKey check around the assignments, so removing the argument from the intent will just result in it overwriting the value with whatever the default is... 0 in the case of .getInt().

[iainconnor]

Would it be more reliable to just swap the order things are restored in? https://github.com/tom91136/Akatsuki/pull/17

[tom91136]

Good catch. I'm currently working a full rewrite of the library, so I'll make note of this and remember to have this bug fixed. You can probably see that code base is a complete mess right now, we got 3 abstractions for the Field kind and they're used all over the place. The tests aren't that well written either, this bug was supposed to caught by at least a few of the integration tests but it slipped through.

As for the best way to solve this: Yes, I agree with you. Reversing the order will probably work fine(restoration will happen twice in this case, not sure if this is a good thing). One thing that I was trying before I started the rewrite was to simply clear the entire argument bundle by calling Bundle.clear() and then check for Bundle.size() == 0 later. But then the user might be passing their own stuff through that bundle too so we can't just clear it.

I'm now stuck with two options:

1. containsKey check before restoring
2. Add a boolean value in the bundle ("consumed" perhaps?)so that we know the bundle is consumed and not to read from it

Oh, and for the pull request: I would keep the statement: {{bundle}}.remove({{keyName}}. If we don't, Android will try to restore the argument bundle on it's own as described here

Both saves Akatsuki from restoring twice, but I'm not sure which approach is better. 1 seems logical but 2 looked faster.

What do you think?

[iainconnor]

Thanks for the reply. Very exciting to hear about the rewrite. I think this library has huge promise in terms of what it is attempting to solve and the performance-first methodology it's taking to solve it... but I'll admit that getting into the code and trying to understand thing didn't go so well 😀.

I see what you mean by my oversight in that pull request. Something about adding a tombstone value I think feels wrong or lazy to me as well. One other option would be that after you apply the arguments, while you're restoring the bundle, instead of leaving the 2nd parameter of, for example Bundle.getInt("myInt") as omitted, you set it to the current value `Bundle.getInt("myInt", whatever.myInt). Obviously you'd need to do some null checking for the primitives, but it would allow you to get away with not having to reinspect the bundle over and over.

[tom91136]

Right now I'm doing some evaluation on how to handle "state" bundles and "arg" bundles. I figured that putting a Bundle in a bundle with a special key seems to be a good idea. (Bundle has a method called putBundle that supports putting another bundle; I've checked the source and it's not the same as putAll). This seems to address most of the issue we have right now with @Arg and @Retained stepping on each other.

With this approach, we're limiting Akatsuki to work with it's own Bundle. This is good because we are far less likely to break other libraries by living in out own Bundle "namespace" (reduced key collisions).

It goes like this(restore):

1. Akatsuki.restore(...) called
2. Checks whether an entry for arguments exist
3. If arg entry exists, restore them and clear the entry
4. If arg entry does not exist, restore all retained fields

Any thoughts or suggestions?

[iainconnor]

How would @arg entries get in? Would they still be (whether they're individual or part of a bundle) part of the intent? The only thing I'm still slightly concerned with is that I get the feeling getIntent() is expected to be immutable... or, at very least, that the APIs for interacting with an intent seem inconsistent on whether they truly alter the incoming intent or simply a copy of it.

[tom91136]

That was what I was hoping, I imagine there will be security concerns if it was mutable. But, check the documentation for [setIntent()],(http://developer.android.com/reference/android/app/Activity.html#setIntent(android.content.Intent)), it says it's copied(maybe, the docs are inconsistent from place to place).

I did some digging and from what I see, it seems that Intent contents(backed by a Bundle BTW), is not immutable. getIntent() returns a package private field called mIntent which is assigned from a method called attach(). attach() is called from ActivityThread from a class called ActivityClientRecord, there are lots of assignments but no signs of copying. I believe bundle values are only copied when it needs to get across process boundaries, and even if that's the case, I sometimes still see objects with the same reference after being passed from another process or restored from bundle.

Personally, I think the whole state persistence mechanism in Android is extremely poorly documented and awkward to use. The lifecycle of these states(the intents, or arguments for fragment) are never explicitly specified. It's also unclear what happens and what to do if your Bundle explodes. Because of all these, it's very hard to write my library as I'm trying to adhere to some imaginary rules created based on my experience.

But after all, the goal for my library is to fix this situation, so yeah, at least future Android developers does't have to worry about this.

I could be blind and did't read the docs carefully enough to understand what's happening. In that case, please point me in the right direction.

[iainconnor]

No, I think you're 100% right about everything there (including how messy the implementation is :angry: ), my only thought was to consider whether you're planning on abusing an "undocumented" feature that could easily change in the underlying source code. Having re-read the documents... I don't think you are and I personally haven't seen many (or any that come to mind) examples of Android projects suffering regressions due to "fixes" in the underlying AOSP.

If I'm to think optimistically, I think your plan sounds completely fair and is the most performant option.

If I'm to think pessimistically, I think that we should flip the flow to be;

1. Akatsuki.restore(...) called
2. Checks whether an entry for @Retained bundle (in saved instance state) exists and process it, restoring all nodes
3. If arg entry exists, restore it ONLY if it doesn't exist in @Retained bundle

[iainconnor]

Just spotted https://github.com/workarounds/bundler which seems to have a very similar scope. Their "solution" to this problem is what we were discussing earlier... letting the restoration happen twice, which, yes, is wasteful, but ensures the behaviour you'd expect.

[tom91136]

I would say the scope is identical :). Looks like a good library, but unit tests are missing. I hope the author finds time to add some unit tests as the scope is relatively complex.

Since I'm currently doing computer science in university, I like challenges; I'm more towards creating a reusable framework for annotation processing and code generation. (Justified after reviewing the current codebase).

Now, I did do some research on annotation processing before I started this project. It seems that support is very limited. We don't have a proper testing framework for all the javax API's. Mocking them is not fun. Code generation is not any better either, JavaPoet is our current best option. There's nothing wrong with JavaPoet, it's very nice to use and well documented. The problem I have is that it is very difficult to piece together what the generated source will look like just by looking at our model.

To solve to two problems above, I wrote two other libraries:

1. Code generation will now be something like this sample, It's using an expereimental library I'm working on right now. The idea is that the source model should look extremely similar to the actual output, reducing mental load when designing the model.
2. Full unit test will be possible as I added a layer of abstraction that will allow testing during runtime with reflection instead of using javax's API. This is something I can extract into a library.

I hope this gives you an idea on what I'm working on right now and where I'm headed (and maybe justify the long wait since 0.2.0). You can probably see that I'm enjoying the development process more than actually having the end product ready in a timely fashion.

Sorry for this long blab, I guess I just need to write this down somewhere. As for the real question about restoration: I'll do some testing on whether the penalty of restorating twice is actually great enough to switch to my nested bundle implementation. I'll let you know when the rewrite is done.

In the meantime, suggestions are welcomed. (especially for the code generation idea mentioned above in point 1)