Move pixel API to its own gradle lib module #1103

aitorvs · 2021-02-15T22:05:27Z

Task/Issue URL: https://app.asana.com/0/414730916066338/1199931182139284/f
Tech Design URL:
CC:

Description:
This PR moves the statistics package, that contains the ATB and Pixel APIs to its own gradle lib module. The PR:

Moved statistics package to its own module
Added dagger as implementation to use Inject annotations
Added common gradle module and moved WorkerInjectorPlugin to it
Moved AppUrl to common module and fix pixel service and entity
Moved Device.kt to common module and fix VERSION_NAME access
Moved exception package to common module
Removed default Build.VERSION_NAME from UncaughtExceptionEntity
Fixed the AppInstallationReferrerStateListener<>AtbInitializer dependency
Fixed tests

I have left the corresponding test in the app gradle module for now to be sure the are executed in CI (I was not sured whether CI was prepared to execute tests in other gradle lib modules).
Once that's done, we can move the tests as well

Steps to test this PR:

install/upgrade from this branch
open the app test basic features work, bookmarks, settings, fire proofing sites, tabs, etc.
verify pixels are sent normally

Internal references:

Software Engineering Expectations
Technical Design Template

aitorvs · 2021-02-15T22:12:31Z

app/src/androidTest/java/com/duckduckgo/app/statistics/api/RxPixelSenderTest.kt

+@Database(
+    entities = [PixelEntity::class],
+    version = 1
+)
+@TypeConverters(
+    QueryParamsTypeConverter::class
+)
+abstract class TestDatabase : RoomDatabase() {
+    abstract fun pixelDao(): PendingPixelDao
+}
+


Created the TestDatabase to break the circular dependency with the main app module. Also, we don't need the full DB, just the DAO to perform the test

as discussed, we can get rid of this

aitorvs · 2021-02-15T22:16:27Z

statistics/src/main/java/com/duckduckgo/app/statistics/AtbInitializer.kt

+interface AtbInitializerListener {
+
+    /**
+     * This method will be called before initializing the ATB
+     */
+    suspend fun beforeAtbInit()
+
+    /**
+     * @return the timeout in milliseconds after which [beforeAtbInit]
+     * will be stopped
+     */
+    fun beforeAtbInitTimeoutMillis(): Long
+}
+
 class AtbInitializer(
    private val statisticsDataStore: StatisticsDataStore,
    private val statisticsUpdater: StatisticsUpdater,
-    private val appReferrerStateListener: AppInstallationReferrerStateListener
+    private val listeners: Set<AtbInitializerListener>
 ) {


Creater AtbInitializerListener to break circular dependency with the main app module.
Also, the fact that the AtbInitializer depended on the AppInstallationReferrerStateListener was leaking the referrer logic into the AtbInitializer, which preferably should not happen.

The AtbInitializer now provides a listener that can be implemented by any other class in the app that is interested in executing some code before the AtbInitializer is initialized.
This way, the AtbInitializer does not need to know anything about these classes

aitorvs · 2021-02-15T22:17:34Z

common/src/main/java/com/duckduckgo/app/global/device/Device.kt

+    override val appVersion by lazy {
+        val info = context.packageManager.getPackageInfo(context.packageName, 0)
+        info.versionName.orEmpty()
+    }


BuildConfig.VERSION_NAME cannot be access from a lib gradle module. So we get the version name programatically

aitorvs · 2021-02-15T22:18:57Z

common/src/main/java/com/duckduckgo/app/global/exception/UncaughtExceptionEntity.kt

+    val version: String,
+    val timestamp: Long = System.currentTimeMillis()


BuildConfig.VERSION_NAME cannot be access from a lib gradle module, so I made version required so that the caller side can provide the proper version

aitorvs · 2021-02-15T22:21:45Z

common/src/main/java/com/duckduckgo/app/global/exception/UncaughtExceptionRepository.kt

+            val exceptionEntity = UncaughtExceptionEntity(
+                message = rootCause.extractExceptionCause(),
+                exceptionSource = exceptionSource,
+                version = deviceInfo.appVersion)


version is now required param, so get it from the DeviceInfo

aitorvs · 2021-02-15T22:25:09Z

versions.properties

 version.google.dagger=2.27
 version.io.reactivex.rxjava2..rxandroid=2.0.2
-version.io.reactivex.rxjava2..rxjava=2.1.10
+version.io.reactivex.rxjava2..rxjava=2.2.9


Bumped this dependency bc I realised that it was overriden by the AndroidX.work.rxJava2, which was bringing version 2.2.9 along, so we effectively had two rxjava versions in the APK, but we were using 2.2.9

…ency

malmstein

couple of changes on sdk versions and removing the unnecessary TestDatabase

malmstein · 2021-02-17T09:06:42Z

app/src/androidTest/java/com/duckduckgo/app/di/StubStatisticsModule.kt

        statisticsDataStore: StatisticsDataStore,
        statisticsUpdater: StatisticsUpdater,
-        appReferrerStateListener: AppInstallationReferrerStateListener
+        listeners: Set<@JvmSuppressWildcards AtbInitializerListener>


malmstein · 2021-02-17T09:12:28Z

app/src/androidTest/java/com/duckduckgo/app/statistics/api/RxPixelSenderTest.kt

+@Database(
+    entities = [PixelEntity::class],
+    version = 1
+)
+@TypeConverters(
+    QueryParamsTypeConverter::class
+)
+abstract class TestDatabase : RoomDatabase() {
+    abstract fun pixelDao(): PendingPixelDao
+}
+


as discussed, we can get rid of this

malmstein · 2021-02-17T09:21:33Z

common/build.gradle

+
+    defaultConfig {
+        minSdkVersion 21
+        targetSdkVersion 30


project's targetSdkVersion is 29, probably we should export it for everyone's to use.

malmstein · 2021-02-17T09:22:01Z

common/build.gradle

+}
+
+android {
+    compileSdkVersion 30


same here, this is 29

malmstein · 2021-02-17T09:24:07Z

common/build.gradle

+    testImplementation AndroidX.room.testing
+    androidTestImplementation AndroidX.room.testing
+
+    testImplementation 'junit:junit:4.+'


can you call ./gradleW refreshVersions so this is extracted?

malmstein · 2021-02-17T09:24:20Z

common/src/androidTest/java/com/duckduckgo/app/global/DummyTestForCI.kt

+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class DummyTestForCI {


malmstein · 2021-02-17T09:25:54Z

statistics/build.gradle

+}
+
+android {
+    compileSdkVersion 30


same here regarding version 29

malmstein · 2021-02-17T09:26:02Z

statistics/build.gradle

+
+    defaultConfig {
+        minSdkVersion 21
+        targetSdkVersion 30


malmstein · 2021-02-17T09:26:15Z

statistics/build.gradle

+    // Play Store referrer library
+    implementation("com.android.installreferrer:installreferrer:_")
+
+    testImplementation 'junit:junit:4.+'


same here for refreshVersions

…statistics and extracted common lib gradle config

malmstein

Thanks for the changes @aitorvs , this looks great!

malmstein · 2021-02-17T10:48:51Z

app/build.gradle

        applicationId "com.duckduckgo.mobile.android"
-        minSdkVersion 21
-        targetSdkVersion 29
+        minSdkVersion min_sdk


aitorvs commented Feb 15, 2021

View reviewed changes

aitorvs force-pushed the feature/aitor/pixel_module branch from 66a853e to a80752d Compare February 16, 2021 10:46

aitorvs requested a review from malmstein February 16, 2021 13:44

aitorvs added 11 commits February 16, 2021 13:59

Moved statistics package to its own module

edb8f62

Added dagger as implementation to use Inject annotations

834452f

Added common gradle module and moved WorkerInjectorPlugin to it

2b703f8

Moved AppUrl to common module and fix pixel service and entity

94b8441

Moved Device.kt to common module and fix VERSION_NAME access

ff43cc4

Moved exception package to common module

4ad8b0e

Removed default Build.VERSION_NAME from UncaughtExceptionEntity

718a96a

Fixed the AppInstallationReferrerStateListener<>AtbInitializer depend…

dd1409d

…ency

Fixed tests

24016e7

Added test runner to common and statistics manifest

522542d

Added dummy tests for CI

26709d0

aitorvs force-pushed the feature/aitor/pixel_module branch from 70451b8 to 26709d0 Compare February 16, 2021 14:00

malmstein self-assigned this Feb 16, 2021

malmstein requested changes Feb 17, 2021

View reviewed changes

malmstein and others added 3 commits February 17, 2021 10:34

added jUnit to refreshVersions

d38c6f9

Reverted change in RxPixelSenderTest as it stayed in the :app module

d89fb41

Changed compileSdkVersion and targetSdkVersion to 29 in :common and :…

c41dada

…statistics and extracted common lib gradle config

aitorvs requested a review from malmstein February 17, 2021 10:04

malmstein approved these changes Feb 17, 2021

View reviewed changes

aitorvs merged commit 088dd73 into develop Feb 17, 2021

aitorvs deleted the feature/aitor/pixel_module branch February 17, 2021 10:51

		val version: String,
		val timestamp: Long = System.currentTimeMillis()

Move pixel API to its own gradle lib module #1103

Move pixel API to its own gradle lib module #1103

Uh oh!

Conversation

aitorvs commented Feb 15, 2021

Internal references:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malmstein left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

malmstein left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants