Completely generate DataBinding models

[elihart]

I am almost done with #183 and it got me thinking that all somebody should really have to do is define a list of xml layouts that they want to use with databinding, and we could generate the whole EpoxyModel for them.

For example, with the upcoming support they would have to write a model like

@EpoxyModelClass(layout = R.layout_my_databinding_layout)
public abstract class MyModel extends DataBindingEpoxyModel {
   @EpoxyAttribute int variable1;
   @EpoxyAttribute boolean variable2;
    ... more attributes
}

but instead we could have them define layouts in an annotation like @EpoxyDataBindingLayouts({R.layout_my_databinding_layout1, R.layout_my_databinding_layout1})
and the annotation processor could inspect the R tree at process time to get the data variables and build the model completely. This is safer too because it guarantees that the variable names of the model stay in sync with the data names in the layout.

I don't think this is too hard - the R parsing would be the hardest part. It isn't super important because it isn't a lot of work to create a model like above, but it would be a nice boilerplate saver and I think it would be pretty cool.

[geralt-encore]

Sounds pretty cool! Any ideas about R parsing? Something similar to this part which was taken from Butterknife?
Just wanna to double-check. So your idea is to parse data variables from layout's XML -> generate handwritten-like model (like in example) -> feed it to annotation processor and generate final model?

[elihart]

@geralt-encore yeah, it would be cool! I just put up my WIP code for the databinding at #188 which this would build upon.

It seems like you understand it. Although I don't think we would actually generate the handwritten model, since then we would need to generate the _ model (the normal generated model) on top of that. That could work, but we would need to make sure that the annotation processor handles rounds correctly. Alternatively we could just only make the _ class to simplify. I don't see a need for the normal model. We would just need to add the fields to the generated model.

I'm thinking the generated model name would be based on the xml name. Camel cased, and possibly appended with something (Model? or at least _?)

And yes, I am thinking of parsing the R tree like butterknife does. I removed that R tree parsing in Epoxy since I figured out a simpler approach, but we could add it back in.

Basically for a layout like

<layout xmlns:android="http://schemas.android.com/apk/res/android">

    <data>
        <variable
            name="textRes"
            type="int" />

        <variable
            name="clickListener"
            type="android.view.View.OnClickListener" />
    </data>

    <Button
        android:id="@+id/button"
        android:layout_width="match_parent"
        android:layout_height="40dp"
        android:onClick="@{clickListener}"
        android:text="@{textRes}" />
</layout>

We should be able to identify the data block, and then take out each variable name and type.

As I type this out though, I realized the R class doesn't give you access to the xml layout. And I'm not sure we can look up an Xml file in an annotation processor, so that approach might be a dead end.

Alternatively we could use a gradle plugin to look at all xml files and find ones with layout tags and generate a model for each one. I think that is basically what the databinding framework does. I think this would work, and to prevent the plugin from duplicating all the code of the annotation processor it could make a model that looks like the handwritten model with the EpoxyAttribute annotations, and then the annotation processor could pick that up.

One other approach could be to have our annotation processor look for generated classes that extend ViewDataBinding. that way we can find what layouts had a data binding class generated for them, see what "setters" were created and get the variable name and type from the setter, and use that to generate the model. I'm not sure if that works though since the data binding classes may not be generated by the time our annotation processor runs, and I'm not sure of a good way to even look them up...

Haha, sorry, I just started thinking about this this morning, but I think it has promise. Happy to hear your ideas and it would be great if you can work on it. I should be able to merge my data binding PR tomorrow or the next day

[elihart]

I'm trying to learn exactly what databinding does under the hood. https://realm.io/news/data-binding-android-boyar-mount/ is a bit helful, but doesn't got into too much detail. Sounds like some sort of plugin at compile time.

If we go the gradle plugin route I think that could be a cool way to automatically make models for all databinding layouts, no extra work required.

If we did the annotation approach where you declare what layouts you want models made for I think we can take that layout name, figure out what the expected DataBinding name is, and then try to look that up. I'm still not sure if it will be generated and accessible at that time though.

[elihart]

@geralt-encore I did some testing, and I think it will be possible to do this with an annotation processor!

If we know the name of the binding class generated for the layout (by default it's based on the xml name) then we can look it up in the processor by name. For model_button.xml we would look up com.airbnb.epoxy.databinding.ModelButtonBinding. I was able to see full information about this class!

We can then look at enclosed elements to see the methods, and look at the setters to get the variable names and their types.

The only trick is the binding class is not available on the first round of processing so we just need to make sure the processor works on multiple rounds.

Right now I'm thinking we might do a package annotation with a list of layout resources as the params (I mentioned this above). The processor can look for that annotation, then use the layout resources to get the expected names of the binding classes, look those up, generate a model with normal epoxy annotations in the package that the package annotation was in, and then the normal epoxy processor should pick those up and generate the normal _ model.

It could be confusing to have two generated models though, so we could just go with one. but that would probably be more work to modify the large model processor to do that.

Any other ideas about how to declare the layouts besides a package annotation?

This seems like a decent approach, unless you liked the idea of a gradle plugin? That is a bit appealing to not have to declare anything at all, but it could also be a bit weird and be a lot more work.

[elihart]

@geralt-encore Were you interested in working on this? If not I'll start

[geralt-encore]

Sorry, that didn't answer you earlier. Yeah, it seems like a good approach and I am interested in working on this. Do we need to do anything to be sure that processor works on multiple rounds? As far as I remember it should be handled automatically, right?
Generating intermediate model looks like a good solution. I proposed it originally as well since it should be much easier to implement without heavy modification of the current processor.

[elihart]

@geralt-encore Great! I realized that the R class parsing in LayoutResourceProcessor will have to change as well to support the new annotation (@EpoxyDataBindingLayouts({R.layout_my_databinding_layout1, R.layout_my_databinding_layout1}). That is a bit complicated, so I can do that now and you could follow up with the code generation.

The rounds do happen automatically. However, we do something a little hacky in ControllerProcessor#resolveGeneratedModelNames - that method is passed the list of all the models generated in the previous step so it can figure out the class types of the generated models.

Eg, if the controller has automodels like

@AutoModel MyGeneratedModelClass_ model;

Then MyGeneratedModelClass_ won't exist when the controller processor runs. All we have is the class name MyGeneratedModelClass_, but not the package information. So by passing in the list of generated models we can find a matching fully qualified name.

Like I said, hacky :P if you can think of a better way to do it, be my guest! The problem is that the databinding classes are generated the first round, so we can't access them to look up the setters until the second round, so our databinding models aren't generated until the second round. But the controller processor will have already run, so if any of the databinding models were used in @AutoModel annotations then their package wouldn't have been resolved.

So, we would need some good way of delaying the auto model code generation until after the databinding models are generated. We should be able to run the controller processor on the annotations to collect the necessary data, and then just hold onto that and wait until a later round to actually write the java code

[elihart]

@geralt-encore I refactored the layout processing in #190, and stubbed out the remaining work in the DataBindingProcessor for you to continue with.

I haven't tried to fix the multiple round support, but hopefully your understand my notes about it. If not I can try to clarify further.

The intermediate models should be an easy solution (I hope), but it's also not my preferred solution since it creates two models, and users would only ever need the _ model. Having the intermediate model might be confusing, so it isn't my preferred approach. I'll leave it up to you thought to decide which way to do it.

I think it wouldn't be that hard to skip the intermediate model and generate the _ model directly. All we would need to do is add fields values to that model for each attribute.

Thanks for working on this! let me know if you'd like help.

[geralt-encore]

@elihart cool! I'll take a look as soon as I can.
I totally agree with you, don't think that it will be too hard to avoid generating intermediate models.

[elihart]

@geralt-encore Awesome! keep me posted on how it goes. I would also need to add fields to the generated class to support #171, so might as well do it now.

The GeneratedModelWriter class is getting pretty big and complicated, so we may want to decompose it a bit to support cases of this added functionality. Possibly we can change it to return the class builder so we can add things (like these fields), or we could add a callback to modify the builder before it writes to file. Let me know if you have ideas.

[elihart]

@geralt-encore Hey, how's it going with this?

[geralt-encore]

Didn't have that much time lately 😔
I've started implementing it with intermediate class but can't really show anything yet. I thought that it would be better to have this implementation first and then try to improve it since it requires quite some changes in current code.

[elihart]

No worries, I've been busy too :( The intermediate approach is fine, it's a good foundation we can release with to start and then build upon :)

Let me know if you want any help or code review!

[elihart]

@geralt-encore I've been working on generating models for litho components. In order to support that I've refactored how the model generation works to make it easy to create just a generated class without the user created model class as an intermediate step. This should make it easy to generate the databinding models. So, if you haven't worked much on it yet you might want to hold off until I finish this refactor.

[elihart]

Well, I made progress faster than I expected #202 :P

If you don't have time to work on this databinding generation I can start it. Do you have any initial code started?

[geralt-encore]

I haven't done much really so I can hold off until #202 will be merged. I should have enough time to deal with it now, so now worries :)
Without generating intermediate models it should be even easier since I only need to collect data basically since you already have done all ground work. I just need to check how you did it with Litho integration.

[elihart]

Great! 202 is merged, and I also just did #204 to lay some more foundation. The main thing that does it change the processors to work with multiple rounds. The way it is set up now I've stubbed out the calls in the correct places so that the databinding classes will exist (I think).

I also updated the code generation to get the layout resource off the generated databinding model. that could be cleaned up a lot, but it (should) work.

The main job of parsing the databinding class and building the model and attribute info is wide open :)

The code is a bit rough so feel free to clean it up as you go. Thanks!

[geralt-encore]

Here is my WIP implementation.
The problem is that even with the workaround for generating data binding related models during the last round I am still getting null while trying to access binding class. It might be that I am missing something so it would be nice if you could briefly review it.

[elihart]

Thanks for starting on this, I'll check out your work and see what is going on.

[elihart]

@geralt-encore I believe you need to enable databinding in the build.gradle file of the integration test module

[geralt-encore]

@elihart thanks for looking into it. It is already enabled.

[elihart]

ah right, hmmm. The code looks pretty great overall and nothing stands out as wrong. I'll pull the branch sometime today and poke around a bit.

[geralt-encore]

I just was thinking... Isn't data binding has nothing to do with annotation processing? Looks like it invoked by some Gradle task and probably it happens after/during the annotation processing task. So waiting for the last round doesn't actually change anything in that case. Or am I missing something?

[elihart]

From my understanding, databinding uses both a gradle plugin and annotation processing.

Just found the problem. The module name is wrong. the class is generated at com.airbnb.epoxy.databinding.ModelWithDataBindingBinding

I'm a bit confused about what is going on with the package. In the android manifest of the integration test module the package is defined as com.airbnb.epoxy. The databinding module manifest has a package of com.airbnb.epoxy.databinding.

The integration test module has an R file generated for both com.airbnb.epoxy.databinding and com.airbnb.epoxy, but the Binding class is only generated in com.airbnb.epoxy.databinding.

Since we use the R file in com.airbnb.epoxy (which should be the correct one), the annotation processor uses that as the package for the Binding class, which doesn't match.

I'll keep looking into this.

[geralt-encore]

I also was thinking about this but changing package didn't solve anything. You used the same package for accessing BR class and it did work out.

[elihart]

The BR class seems to be generated in both the packages (like R) :( weird stuff, I've got some meetings for a while but I'll look more later. let me know if you have ideas! I wonder if we rename the manifest package of the integration test module it might work better, since right now there multiple modules with that package name and it seems to cause issues sometimes

[geralt-encore]

Yeah, I am confused as well =(
I'll try to look into the package renaming once again - maybe I screwed up in a first place somehow.

[elihart]

oh... I completely missed this line in the data binding docs This class will be placed in a databinding package under the module package

https://developer.android.com/topic/libraries/data-binding/index.html#custom_binding_class_names

So that explains everything I think. Should be an easy fix to adjust for that.

[geralt-encore]

Looks like it crashes in a different place now. Can't figure out where exactly but it is still a progress anyway.

[geralt-encore]

@elihart thanks a lot for your help! Turned out we were right about package name but I missed it in a first place because I was getting another NPE.

[elihart]

ah good. Glad I could help! Let me know if you'd like help tracking now NPE's. I've been dealing with those lately, it's annoying that sometimes the stacktrace is not included.

[elihart]

#207