-
Notifications
You must be signed in to change notification settings - Fork 50
feat(android): Upgrade native SDK to 3.9.0 and adapt to breaking changes #671
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| dependencies { | ||
| classpath "com.android.tools.build:gradle:8.4.0" | ||
| classpath "com.android.tools.build:gradle:8.3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is being downgraded ?
| sourceCompatibility JavaVersion.VERSION_1_8 | ||
| targetCompatibility JavaVersion.VERSION_1_8 | ||
| sourceCompatibility JavaVersion.VERSION_11 | ||
| targetCompatibility JavaVersion.VERSION_11 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets updated the java version to 17 .
|
|
||
| kotlinOptions { | ||
| jvmTarget = '1.8' | ||
| jvmTarget = '11' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets update to 17
pmathew92
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@utkrishtsahu I would suggest you also create Migration guide for v2 along with this PR
| minSdkVersion 24 | ||
| testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" | ||
| manifestPlaceholders = [auth0Domain: "test-domain", auth0Scheme: "test"] | ||
| manifestPlaceholders = [auth0Domain: "dev-z0xy0f8x5xj51m2q.us.auth0.com", auth0Scheme: "https"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't hardcode your domain id here. Use a place holder in commited changes
| testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" | ||
| manifestPlaceholders = [auth0Domain: "test-domain", auth0Scheme: "test"] | ||
| manifestPlaceholders = [auth0Domain: "dev-z0xy0f8x5xj51m2q.us.auth0.com", auth0Scheme: "https"] | ||
| consumerProguardFiles '../proguard/proguard-gson.pro', '../proguard/proguard-okio.pro', '../proguard/proguard-jetpack.pro' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these from the Auth0.Android SDK ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The proguard-jetpack.pro file is a new requirement for v3.x of the native SDK because it now uses the AndroidX Biometric and Credentials libraries, and this file prevents their classes from being stripped.
| //noinspection GradleDynamicVersion | ||
| implementation 'com.auth0.android:auth0:2.11.0' | ||
| implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:$kotlin_version" | ||
| implementation 'com.auth0.android:auth0:3.9.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the latest version of 3.10.0
| implementation 'androidx.browser:browser:1.4.0' | ||
| implementation 'androidx.core:core-ktx:1.6.0' | ||
| implementation 'androidx.appcompat:appcompat:1.6.0' | ||
| implementation 'com.google.androidbrowserhelper:androidbrowserhelper:2.4.0' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need all these dependencies ? I don't think so
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need them because auth0-android:3.x changed its dependency declarations from api to implementation. This means its own dependencies (like appcompat for UI components and core-ktx for utilities) are no longer transitively exposed to our plugin.
| SharedPreferencesStorage(context, it) | ||
| } ?: SharedPreferencesStorage(context) | ||
| credentialsManager = | ||
| credentialsManager ?: SecureCredentialsManager(context, api, storage) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't remove this code. This is to ensure we don't create a new instance of CredentialsManager each time this method is called . Your current implementation creates a new instance of SecureCredentialsManager each time. Add this check in your current workflow
| } | ||
|
|
||
| @Test | ||
| fun `handler should result in 'notImplemented' if no handlers`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these two cases removed ?
| dependencies { | ||
| kover(project(":auth0_flutter")) | ||
| implementation "androidx.credentials:credentials-play-services-auth:1.3.0" | ||
| implementation "androidx.credentials:credentials:1.3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need these dependencies
| } | ||
|
|
||
| @Test | ||
| fun `handler should extract sharedPreferenceName correctly`() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
handler should extract sharedPreferenceName correctly why was this test case removed. This checks a completely different flow. I would suggest remove only relevant tests that are no longer valid. Don't remove tests which are testing entirely different things
📋 Changes
This pull request upgrades the native auth0-android SDK from version 2.11.0 to 3.9.0 and adapts the auth0-flutter plugin to the breaking changes introduced in the new version.
The primary goal is to ensure the Flutter plugin remains up-to-date with the latest features and platform requirements of the underlying native Android library.
📎 References
Jira Ticket: SDK-6921
🎯 Testing
This PR has been tested manually on an Android device to ensure all key flows are functional after the upgrade.
Also updated UT cases for the same.