android10 / frodo

Android Library for Logging RxJava Observables and Subscribers.
1.48k stars 101 forks source link

@RxLogObservable not working on interface method #1

Closed realdadfish closed 8 years ago

realdadfish commented 8 years ago

I'm using Retrofit2 and let it return instances of Observable<Response<Payload>>. If I annotate such a method with @RxLogObservable and kick off a request, I cannot see anything in Logcat.

Now, I'm not exactly sure if I'm doing everything right, since the interface in question resides in a module and yes, I have set android { defaultPublishConfig "debug" } set globally for all my Android modules.

Is logging supposed to work on interface methods?

realdadfish commented 8 years ago

Ah, ignore me, we flavors in our library and need to work around that differently... sorry for the noise.

realdadfish commented 8 years ago

Got it working - short explanation here: We have a multi-flavor library and android { publishNonDefault true } enabled already on that, so defaultPublishConfig "debug" is not needed.

Now the issue was that when we referenced this library in our app module, we previously used a fixed release configuration, which will obviously not work. The solution is to do

yourFlavorCompile project(path: ':yourlibrary', configuration: isProduction ? 'yourOtherFlavorRelease' : 'yourOtherFlavorDebug')

and have isProduction a project property that you give gradle on command line with ./gradlew -PisProduction=true to switch between the configurations.

realdadfish commented 8 years ago

Hah, now I guess it is actually an issue - annotating proxy interfaces is really not working, all other observables are. This workaround should however do the trick:

@RxLogObservable
private Observable<Response<Payload>> createObservable() {
    return myApi.createProxiedRequest();
}
android10 commented 8 years ago

@tommyd3mdi thanks for the feedback. I will look into this.

damianflannery commented 8 years ago

I have the same issue (although I'm using Retrofit 1.9). Wrapping the call in a method as described above works. See: https://www.dropbox.com/s/nhic4ejo3pftje7/Screenshot%202015-11-05%2016.10.45.png?dl=0

Although I really don't want to do that for all 30+ Retrofit api calls. It would be great if I could just directly annotate those interface method signatures :)

android10 commented 8 years ago

@damianflannery @tommyd3mdi thanks for the feedback. Really I'm thinking whether or not makes sense to be able to annotate interfaces, since the common use case would be to just print implementations. I have to say I do not use retrofit so I might be missing something here.

damianflannery commented 8 years ago

I thought everybody used Retrofit!

It supports RxJava directly and you use an interface to define your http calls (with various annotations). Here is a snapshot of my Retrofit interface (which currently has 3062 http calls defined) and you'll see why it would be so convenient if Frodo had this feature: https://www.dropbox.com/s/cshn6t1ewh4tcea/Screenshot%202015-11-06%2009.26.11.png?dl=0

android10 commented 8 years ago

@damianflannery it depends on the use case. For me it does too many things so I usually create an API wrapper around OkHttp. We do the same at SoundCloud.

damianflannery commented 8 years ago

@android10 OK, it's definitely a 'nice to have' since you can manually wrap the interface. It might be worth adding something to the README in case anybody else follows the same steps and wonders why there is no output from Frodo.

Maybe you could consider as a future enhancement if enough people request it. Thanks for sharing this library, I think it's going to be really useful :)

realdadfish commented 8 years ago

Since I have no idea of the technical aspects here, what is the underlying issue that proxy interfaces cannot be processed?

android10 commented 8 years ago

@damianflannery I will create an issue with this as a future enhancement. @tommyd3mdi AspectJ gives me a JoinPoint and from there I detect the return type and inject code to log observables. By returning an interface I guess I should search for all implementations in the code and weave logging code in each one of them. I have to investigate a bit more here. Anyway, as I mentioned, I will create an issue and label it as possible enhancement.

Here is the point when I'm detecting whether is an observable: https://github.com/android10/frodo/blob/master/frodo-runtime%2Fsrc%2Fmain%2Fjava%2Fcom%2Ffernandocejas%2Ffrodo%2Faspect%2FLogObservable.java

android10 commented 8 years ago

Guys, closing this one. I created a new issue to work on this. https://github.com/android10/frodo/issues/8

Thanks for the feedback.