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

Switch to Retrofit + OkHttp + Gson #66

Open
artem-zinnatullin opened this issue Dec 23, 2014 · 8 comments
Open

Switch to Retrofit + OkHttp + Gson #66

artem-zinnatullin opened this issue Dec 23, 2014 · 8 comments

Comments

@artem-zinnatullin
Copy link
Owner

It's a part of Internal code architecture improvement (#53)

@ilya-murzinov
Copy link
Collaborator

I've tried Retrofit out, but immediately ran into problem - it turned out that there is no convenient way to add api_sig to request.
Or may be I overlooked something?
@artem-zinnatullin, don't you know if there is a nice simple solution for this?

@artem-zinnatullin
Copy link
Owner Author

@ilya-murzinov there is a pretty standard way to do it, when you create RestAdapter you can add request interceptor where you can add any parameters to the request before it'll be executed

like this:

RequestInterceptor requestInterceptor = new RequestInterceptor() {
    @Override
    public void intercept(RequestFacade request) {
        request.addHeader("User-Agent", "Retrofit-Sample-App");
    }
};

Or add manual parameter to each request in the interface, but I think first approach is cleaner

@ilya-murzinov
Copy link
Collaborator

@artem-zinnatullin, yes, I saw it. But we need to get all request parameters to generate api_sig and RequestFacade doesn't provide any capability for this.
Another way is to create our own Client, and override execute method to calculate and add api_sig before calling super.execute(). But then we need to parse url query string to get parameters. I don't think it's the right way.

@ilya-murzinov
Copy link
Collaborator

There is Spring's RestTemplate for Android, it provides much more advanced features for such use-cases, maybe we can use it instead?

@artem-zinnatullin
Copy link
Owner Author

@ilya-murzinov damn, you're right... hmm let me think about it, I'll check RestTemplate (but I don't love Spring: too big, a lot of magic, reflection for all the things and so on, I need to check sources of RestTemplate to make a decision).

Also, I'll try to discover solutions with Retrofit

But for practice, you can use RestTemplate, there is not so much API relative code, so we can easily change inner implementation later

Also, I'll be happy if you'll throw away OLDDEPRECATED (and unfortunately mine) library AsyncTaskExecutor.

AsyncTasks are bad bad bad, and also they are bad

@ilya-murzinov
Copy link
Collaborator

@artem-zinnatullin, I've used re-implemented Login fragment with RestTemplate and GreenRobot EventBus.
Please review this solution (see this commit) and tell me if it's ok to you.

@ilya-murzinov
Copy link
Collaborator

Also, I had to use XML responses from LastFM because JSON responses always have status 200 OK (WTF???). But it's not even a bit more complicated then JSON.

@artem-zinnatullin
Copy link
Owner Author

Great, I'm just arrived to Saint-Petersburg and had put your work to the
task list
29 дек. 2014 г. 15:11 пользователь "Ilya Murzinov" notifications@github.com
написал:

Also, I had to use XML responses from LastFM because JSON responses always
have status 200 OK (WTF???). But it's not even a bit more complicated then
JSON.


Reply to this email directly or view it on GitHub
#66 (comment)
.

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

No branches or pull requests

2 participants