Skip to content

prevent using jvm initializer in android and address violation of retrieving unsettable device id #67

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

Merged
merged 22 commits into from
Jan 11, 2022

Conversation

wenxi-zeng
Copy link
Contributor

Summary

  • prevent using jvm initializer in android
  • address violation of retrieving unsettable device id

Additional Changes

  • add cancel previous to github actions
  • updated sovran dependencies from jitpack to maven central
  • remove jitpack dependency
  • fix android snapshot depending on core release artifacts issue

@codecov-commenter
Copy link

codecov-commenter commented Jan 10, 2022

Codecov Report

Merging #67 (2adafea) into main (65d5419) will increase coverage by 0.02%.
The diff coverage is 48.71%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #67      +/-   ##
============================================
+ Coverage     57.71%   57.73%   +0.02%     
- Complexity      418      420       +2     
============================================
  Files            66       67       +1     
  Lines          7137     7174      +37     
  Branches        690      693       +3     
============================================
+ Hits           4119     4142      +23     
- Misses         2506     2519      +13     
- Partials        512      513       +1     
Impacted Files Coverage Δ
...ics/kotlin/android/AndroidContextCollectorTests.kt 0.00% <0.00%> (ø)
...tics/kotlin/android/AndroidLifecyclePluginTests.kt 0.00% <0.00%> (ø)
...ava/com/segment/analytics/kotlin/core/Analytics.kt 75.67% <28.57%> (-0.22%) ⬇️
...analytics/kotlin/android/AndroidAnalyticsKtTest.kt 37.50% <37.50%> (ø)
...om/segment/analytics/kotlin/core/AnalyticsTests.kt 78.92% <73.68%> (-0.39%) ⬇️
...tics/kotlin/core/platform/plugins/LogTargetTest.kt 72.04% <0.00%> (+1.07%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65d5419...2adafea. Read the comment docs.

…into wenxi/prevent-using-jvm-initializer-in-android

� Conflicts:
�	core/src/test/kotlin/com/segment/analytics/kotlin/core/AnalyticsTests.kt
Copy link
Contributor

@prayansh prayansh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. couple comments

*/
fun getUniqueID(): String? {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR2)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given that we support api 16+ we should have some fallback logic here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point. the getDeviceId method does a null check and returns random uuid as fallback.

internal class AndroidAnalyticsKtTest {
@Test
fun `jvm initializer in android platform should failed`() {
try {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the new junit has a way to expect exceptions instead of doing this pattern. if u wanna try that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me try that

@wenxi-zeng wenxi-zeng merged commit 9944986 into main Jan 11, 2022
@wenxi-zeng wenxi-zeng deleted the wenxi/prevent-using-jvm-initializer-in-android branch January 11, 2022 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants