Break on minified app

[p1gp1g]

Break on minified app

[Unpublished]

Shrinking apps can definitely get troublesome, let me expand further:

Could you please elaborate a bit more what a 3rd party app needs to do? As I understand you, we should

* Merge your branch into the official SSO lib (would you mind opening a PR for that?)

Yes by declaring them as consumer rules R8/ProGuard automatically picks them up.

Ideally there would be separate modules for sso-rxjava2/3 like in retrofit. Regardless of minification, apps end up with deps they don't use as they probably won't use rxjava version 2 and 3 at same time. Just merging my branch would feel a bit dirty, though it would be better than now. Splitting into modules can always happen afterwards.

* The 3rd party apps should still need to add the flag `-dontobfuscate`? Where exactly?

This goes into the proguard-rules.pro of a 3rd party app like news-android for example.
Enabling obfuscation always breaks SSO in my experiments and isn't needed in OSS in my view.

* Maybe we could add a section in the `README.md` that explains this and your instructions for `android.r8.fullMode` before merging your branch?

Sure, I can add a R8/ProGuard section with some references to the README.

[stefan-niedermann]

I assume because the classes have to have the exact same name on both sides (sso / 3rd party app and Nextcloud).

We had similar issues with the Notes app once and stopped our minifying experiments because of it, but i assume if you define exception rules for the sso package, it should work. Feedback welcome 😉

[p1gp1g]

At least I'm not the only one who have seen this issue 😄

I will probably experiment a bit around it soon.

Nevertheless that information should probably be somewhere in the readme ?

[stefan-niedermann]

Actually, it makes sense in an architectural way - the method and class names are part of an API (between the apps) - minifying one end changes the method and class names to random stuff - a breaking change.

However, I absolutely agree with you, and I propose to add the documentation as soon as

I will probably experiment a bit around it soon.

we know some rules which work 😉 if you share the result with us.

[p1gp1g]

Using the following works

postprocessing {
    removeUnusedCode false
    removeUnusedResources true
    obfuscate false
    optimizeCode true
}

Instead of

minifyEnabled true
shrinkResources true

Note that it might be possible to set removeUnusedCode to true depending on the application.

In that case, using @SerializedName() for all parameters of the models (and the nested classes) - for instance here - will make that library work with obfuscated applications (== minifyEnabled true).

[stefan-niedermann]

Awesome, thanks for the update! I'll test the proposed configuration.

[p1gp1g]

You will test on Notes ? Let me know if removeUnusedCode to true works :)

[dasbiswajit]

Guy's just to let you know Bookmarks also faild with the similer error.

[Unpublished]

This branch https://github.com/Unpublished/Android-SingleSignOn/tree/proguardRules contains proguard consumer rules.

The provided rules work quite good together with -dontobfuscate. If you additionally use android.r8.fullMode=true you need to keep your retrofit and gson related classes, as following for UP-NextPush:

# retrofit
-keep class org.unifiedpush.distributor.nextpush.api.ApiResponse
# gson
-keep class org.unifiedpush.distributor.nextpush.api.SSEResponse { <fields>; }

[stefan-niedermann]

@Unpublished I don't have much experience with shrinking and / or obfuscating apps (due to issues with Nextcloud SSO I just disabled it and did not think about it again).

Could you please elaborate a bit more what a 3rd party app needs to do? As I understand you, we should

• Merge your branch into the official SSO lib (would you mind opening a PR for that?)
• The 3rd party apps should still need to add the flag -dontobfuscate? Where exactly?
• Maybe we could add a section in the README.md that explains this and your instructions for android.r8.fullMode before merging your branch?

[stefan-niedermann]

@p1gp1g would you mind testing the changes from #545 and do a review? 🙂 🚀

[stefan-niedermann]

I think this issue can be closed (see #545).

Feel free to reopen.