"How to use this library" steps in readme are not very clear

[z3ntu]

1. The numbering jumps directly from 4 to 4.1.1
2. The Typical use of API using Retrofit text should be reworded (maybe In typical applications, one would use the API like this, but with this library you have to use the NextcloudAPI or something)

[David-Development]

The numbering jumps directly from 4 to 4.1.1

There are two cases here:

1. Using Retrofit (4.1.1)
2. Without Retrofit (4.2)

I agree, it should be 4.1 and not 4.1.1 for the headline.

The Typical use of API using Retrofit text should be reworded (maybe In typical applications, one would use the API like this, but with this library you have to use the NextcloudAPI or something)

I'll changed that.. 👍

Thanks for the feedback! Let me know if you encounter any issues adding this library to your app! May I ask what kind of app you are working on?

[z3ntu]

Thanks!

Thanks for the feedback! Let me know if you encounter any issues adding this library to your app! May I ask what kind of app you are working on?

Another app for News, it started out for testing out the fancy new things in Android Jetpack and Kotlin, but I want to make it usable for at least my purposes :)

I also want to create my own (Nextcloud) Bookmarks app with support for the recently added folders because the existing apps aren't very good imo and don't support folders.

[z3ntu]

Also it should be documented that the getCurrentSingleSignOnAccount doesn't automatically have a value once the user has selected an account but that you have to set it yourself using setCurrentAccount

[z3ntu]

Oh yeah and the Gson situation is quite awkward. This GsonConfig.GetGson() from the readme is a method just available in your News app (maybe explain if GsonBuilder().create() is enough or if registering custom things is necessary)

[David-Development]

Another app for News, it started out for testing out the fancy new things in Android Jetpack and Kotlin, but I want to make it usable for at least my purposes :)

Oh awesome, sounds interesting! Will it be open-source as well?

I also want to create my own (Nextcloud) Bookmarks app with support for the recently added folders because the existing apps aren't very good imo and don't support folders.

That sounds like a great idea as well! I might as well use this one then once you release it! (if you plan on doing so).

Thank you for all the feedback! 👍 I made a couple of changes to the readme. The GsonBuilder().create() should be enough for most use-cases.

And good point about the getCurrentSingleSignOnAccount. I added the missing part in the docs.

Hopefully everything else is working as expected? Did you get the sso library working yet? Let me know if you encounter any issues!

[z3ntu]

Yes I'm planning to open source the app(s), once I have it working to my satisfaction :)

Did you get the sso library working yet?

Yes it's working fine.

Let me know if you encounter any issues!

This is probably because of the way I handle the nextcloudapi instance but i used the apiconnected listener to change the fragment to the view with the news list but when exiting the app (pressing the back button) and relaunching the app this apiconnected listener doesn't fire so my view never gets to what it should be.

Long story short: Is there a way to check if the API is currently connected?

[David-Development]

Yes I'm planning to open source the app(s), once I have it working to my satisfaction :)

Awesome, looking forward to seeing them! :)

This is probably because of the way I handle the NextcloudAPI instance but i used the apiconnected listener to change the fragment to the view with the news list but when exiting the app (pressing the back button) and relaunching the app this apiconnected listener doesn't fire so my view never gets to what it should be.

I guess this happens due to the fact that the service is still active/connected. Or did you close the connection in the onStop method?

I think it might not be a great idea to rely on the onConnected callback as it won't be called if the service is still connected. If you start your app and you want to reload some stuff from the server, just do it (onResume/onCreate/onStart).
I noticed that if you make a request right after creating a new instance of the API, you'll get some error messages such as: "Attempt to invoke interface method 'android.os.ParcelFileDescriptor com.nextcloud.android.sso.aidl.IInputStreamService.performNextcloudRequest(android.os.ParcelFileDescriptor)' on a null object reference". This happens due to the fact that the api won't be connected at that time. This is an issue in the single sign on library that I fixed. Just waiting for approval: #38. After the PR is merged, you don't need worry anymore about the api being ready or not as the requests will wait for the api to be ready.

I also updated the examples (take a look at the example in section 4.2 as well as some general information in section 4 (first couple of lines of section 4))

[desperateCoder]

Oh yeah and the Gson situation is quite awkward. This GsonConfig.GetGson() from the readme is a method just available in your News app (maybe explain if GsonBuilder().create() is enough or if registering custom things is necessary)

If you ask me, i'd prefer to have the option to get the JSON instead of the Gson-parsed object. If i could choose, i'd always rather work with Jackson then with Gson.

@David-Development Sorry for being so picky, hope you don't feel offended by my critics! You have done a great job on SSO, thank you for that!

[David-Development]

@desperateCoder No worries, all good! Glad to get some feedback!

I'd prefer to have the option to get the JSON instead of the Gson-parsed object.

You certainly can use jackson! You can take a look at the method, where gson is used to parse the response data.

Android-SingleSignOn/src/main/java/com/nextcloud/android/sso/api/NextcloudAPI.java

Line 187 in 3b66894

public <T> T performRequest(final @NonNull Type type, NextcloudRequest request) throws Exception {

You can easily call performNetworkRequest yourself and parse the response data using jackson.

Why do you prefer Jackson over Gson?
Gson seems to be faster (http://www.doublecloud.org/2015/03/gson-vs-jackson-which-to-use-for-json-in-java/)

[desperateCoder]

@David-Development well, glad we talked about this :D very good to know!
The only drawback: i have to handle the InputStream by myself, but that would be ok.

Why do you prefer Jackson over Gson?

I am the kind of guy, who likes code to be bullet proof. I had several fights against Gson (it won), but never had a single issue with Jackson. It did anything just like expected.
While thinking about daily business, the difference in performance may be several millis (or even nanos?) except you are parsing gigabytes of JSON...
I don't say Jackson is better than Gson, i just prefer it

[David-Development]

@desperateCoder Yes, you would have to handle the InputStream yourself.

I'm not sure about whether it's a good idea to include Jackson itself in the library as this would increase the number of dependencies (support both gson and jackson). I haven't used Jackson yet so I can't really compare the two. I always went with Gson as it was more common to use for retrofit.

I process many megabytes of data in my news application while synchronizing new feeds. Therefore the sync needs to be fast. On newer devices you won't notice the difference but on older devices it might be noticeable. However I agree that this is not a reason to prefer one over the other as developers can implement their own parser (using the InputStream directly) if our json parser is too slow for them.

[desperateCoder]

@David-Development I totally agree, don't add the dependency!
Handling the Stream isn't a problem at all, so this is totally fine, since i know now i can bypass Gson, i'm good. Thanks for that!
Being able to use Retrofit properly would be a way bigger improvement for SSO. ;)