Set up dokka to generate SDK documentation and cleanup the API #5744

bmarty · 2022-04-12T14:03:03Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Cleanup SDK api package
Some classes has been moved from internal package to api package
Some classes are now explicitly declared internal
Some code and classes have been deleted because they were not used: see 567f298
The CI will now check that the app does not import classes from the internal package 🙈
Changelog: WIP

Motivation and context

#5726 (this PR does not close it)

Screenshots / GIFs

Tests

Run the app: OK
Run the sanity test: OK

Tested devices

Physical
Emulator
OS version(s):

Checklist

Changes has been tested on an Android device or Android emulator with API 21
UI change has been tested on both light and dark themes
Accessibility has been taken into account. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#accessibility
Pull request is based on the develop branch
Pull request includes a new file under ./changelog.d. See https://github.com/vector-im/element-android/blob/develop/CONTRIBUTING.md#changelog
Pull request includes screenshots or videos if containing UI changes
Pull request includes a sign off
You've made a self review of your PR
If you have modified the screen flow, or added new screens to the application, you have updated the test UiAllScreensSanityTest.allScreensTest()

Run `./gradlew matrix-sdk-android:dokkaHtml` to generate the Html documentation of the Matrix Android SDK

Classes marked with `internal` will be excluded from Kdoc.

It is used by the app. Make the extensions internal

… api package

… to the app.

ouchadam · 2022-04-12T15:05:02Z

matrix-sdk-android/build.gradle

@@ -73,6 +75,10 @@ android {

    kotlinOptions {
        jvmTarget = "11"
+        freeCompilerArgs += [
+                // Disabled for now, there are too many errors. Could be handled in another dedicated PR


github-actions · 2022-04-12T15:12:21Z

Unit Test Results

114 files ±0 114 suites ±0 1m 18s ⏱️ -3s
201 tests ±0 201 ✔️ ±0 0 💤 ±0 0 ❌ ±0
674 runs ±0 674 ✔️ ±0 0 💤 ±0 0 ❌ ±0

Results for commit e30c68f. ± Comparison against base commit 047a45d.

♻️ This comment has been updated with latest results.

ouchadam · 2022-04-12T15:19:36Z

...src/main/java/org/matrix/android/sdk/api/session/crypto/keysbackup/KeysBackupVersionTrust.kt


 /**
 * Data model for response to [KeysBackup.getKeysBackupTrust()].
+ * TODO Members should be only val


could be handy to track these todos as github issues (if they aren't already)

Yes, you are right. Moar issues!

ouchadam · 2022-04-12T15:42:05Z

...dk-android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt

@@ -65,10 +63,8 @@ internal class SyncResponseHandler @Inject constructor(
        private val aggregatorHandler: SyncResponsePostTreatmentAggregatorHandler,
        private val cryptoService: DefaultCryptoService,
        private val tokenStore: SyncTokenStore,
-        private val lightweightSettingsStorage: LightweightSettingsStorage,


I guess these are unused?

ouchadam · 2022-04-12T15:42:58Z

.../src/main/java/org/matrix/android/sdk/internal/settings/DefaultLightweightSettingsStorage.kt

        context: Context,
        private val matrixConfiguration: MatrixConfiguration
-) {
+) : LightweightSettingsStorage {


ouchadam · 2022-04-12T15:44:10Z

.../src/main/java/org/matrix/android/sdk/internal/settings/DefaultLightweightSettingsStorage.kt

@@ -28,21 +29,20 @@ import javax.inject.Inject
 * on the sdk without using the database. This should be used just for sdk/user preferences and
 * not for large data sets
 */
-
-class LightweightSettingsStorage  @Inject constructor(
+internal class DefaultLightweightSettingsStorage @Inject constructor(


not for this PR but I wonder if these settings should be client side rather than in the SDK

They are meant to be SDK side, client side we have vectorPreferences. You can't read vectorPreferences from SDK side. It's mainly for dynamic settings changes, MatrixConfiguration cannot be changed dynamically.

The main problem to me, is that those settings are SDK scope and should be session scope.

ouchadam · 2022-04-12T15:45:40Z

tools/check/forbidden_strings_in_code_app.txt

+#
+
+### You should not use code from internal SDK package. Either move them to the API package, or add a proper API to access this code, and add internal keyword SDK side.
+import org.matrix.android.sdk.internal


ouchadam · 2022-04-12T15:59:39Z

vector/src/debug/java/im/vector/app/features/debug/DebugMenuActivity.kt

@@ -265,6 +264,8 @@ class DebugMenuActivity : VectorBaseActivity<ActivityDebugMenuBinding>() {
                    }
                }
            }
+            // Ensure developer will see that this cannot work anymore
+            error("toQrCodeData() is now internal")


is the qrscanning debug option still helpful if we can't log the toQrCodeData result?

Probably not, hence not used a lot nowadays. I guess if we want to test it we can uncomment the code and remove the internal keyword. Not sure what was the best approach to avoid to do that for that specific case.

ouchadam · 2022-04-12T16:07:15Z

vector/src/main/java/im/vector/app/features/crypto/recover/BackupToQuadSMigrationTask.kt

-                                    ))
-                                }
-                            })
+                            keysBackupService.computePrivateKey(


I'm assuming deriveKey and keysBackupService.computePrivateKey are equivalent

True, new API added for this specific purpose: 3735ac3

ouchadam

great clean up! 💯
some minor comments/questions but they could be handled in another PR to help get this through faster

ariskotsomitopoulos

Thanks for the PR, I only have one comment. There are a lot of changes here but most of the changes are straight forward. I also run the app & generated the dokka file I didn't observe any issue

ariskotsomitopoulos · 2022-04-13T10:26:54Z

build.gradle

@@ -22,6 +22,7 @@ buildscript {
        classpath 'com.google.android.gms:oss-licenses-plugin:0.10.5'
        classpath "com.likethesalad.android:stem-plugin:2.0.0"
        classpath 'org.owasp:dependency-check-gradle:7.0.4.1'
+        classpath "org.jetbrains.dokka:dokka-gradle-plugin:1.6.10"


We do not need this while it is already defined in the matrix-sdk-android/build.gradle. I also tested it and works fine without it

Ah, you are right. I will keep this one and remove the line in the matrix sdk build.gradle file, if ever we need to generate documentation for another module (such as matrix-sdk-android-flow for instance)

…ready declared in the main build.gradle file.

bmarty and others added 30 commits April 12, 2022 15:38

Configure Dokka plugin to generate Kdoc

4d49cca

Run `./gradlew matrix-sdk-android:dokkaHtml` to generate the Html documentation of the Matrix Android SDK

Add missing internal keyword.

dc092f8

Classes marked with `internal` will be excluded from Kdoc.

Move TextContent class to api package

83570dc

It is used by the app. Make the extensions internal

Internal + add some doc

b4dbb38

Move SpacePeekResult to api package, and cleanup

73d22d0

Move SignInvitationResult and RoomAliasDescription to api package

7711eb5

Remove unused code.

567f298

Move DeviceTrustLevel, DeviceTrustResult and UserTrustResult to…

24d044b

… api package

Move ElementToDecrypt to api package

1684ba1

Move SyncService to api package

3895ac8

Move PrivateKeysInfo to api package

a3575fb

Move SavedKeyBackupKeyInfo to api package

03dba3b

Move content data class to api package

0cb7703

Move md5() extension to api package

ccce552

Move awaitCallback() extension to api package

627a129

Move TermsResponse to api package

d4ff273

Move RecoveryKey functions to api package

c9bbf6c

Check usage of import org.matrix.android.sdk.internal app side

f4fba2d

Move CryptoDeviceInfo to api package

0720196

Move Fingerprint to api package

b1f4aa9

MoshiProvider is now internal. Create MatrixJsonParser to expose it…

d8eac14

… to the app.

Move Base64 facility to api package

1bc6bec

Move ImportRoomKeysResult to api package

4b74010

Move KeysVersionResult to api package

229547f

Move KeysBackupVersionTrust to api package

36fd570

Move KeysBackupLastVersionResult to api package

97c1929

Move KeysVersion to api package

e6fc723

Move IncomingRequestCancellation to api package

7644bd4

Move IncomingSecretShareRequest to api package

e9161dd

Move DeviceInfo to api package

af2825e

bmarty added 7 commits April 12, 2022 15:41

Move RoomKeyRequestBody to api package

89f20e1

Move SendToDeviceObject to api package

b6eafbe

Move UnsignedDeviceInfo to api package

150c643

Rename .java to .kt

7327047

Convert RuntimeJsonAdapterFactory to Kotlin

7514edb

Make RuntimeJsonAdapterFactory internal and cleanup

4e48c07

Global import optimization

23d2a29

bmarty force-pushed the feature/bma/dokka branch from 83134ab to 23d2a29 Compare April 12, 2022 14:06

bmarty added 3 commits April 12, 2022 16:09

Run ktlintFormat

bfbcef6

Add changelog files

a0eb4c1

Restore the correct copyright

38bbbd6

bmarty marked this pull request as ready for review April 12, 2022 14:43

bmarty requested review from a team and ariskotsomitopoulos and removed request for a team April 12, 2022 14:43

ouchadam reviewed Apr 12, 2022

View reviewed changes

ouchadam approved these changes Apr 12, 2022

View reviewed changes

ariskotsomitopoulos approved these changes Apr 13, 2022

View reviewed changes

bmarty added 2 commits April 13, 2022 13:42

Update issue hint

c8aefd5

Remove dokka dependencies classpath for this specific module, it's al…

e30c68f

…ready declared in the main build.gradle file.

bmarty merged commit 5075775 into develop Apr 13, 2022

bmarty deleted the feature/bma/dokka branch April 13, 2022 12:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Set up dokka to generate SDK documentation and cleanup the API #5744

Set up dokka to generate SDK documentation and cleanup the API #5744

bmarty commented Apr 12, 2022 •

edited

ouchadam Apr 12, 2022

github-actions bot commented Apr 12, 2022 •

edited

ouchadam Apr 12, 2022

bmarty Apr 13, 2022

ouchadam Apr 12, 2022

bmarty Apr 13, 2022

ouchadam Apr 12, 2022

ouchadam Apr 12, 2022

ariskotsomitopoulos Apr 13, 2022

bmarty Apr 13, 2022

ouchadam Apr 12, 2022

ouchadam Apr 12, 2022

bmarty Apr 13, 2022

ouchadam Apr 12, 2022

bmarty Apr 13, 2022

ouchadam left a comment

ariskotsomitopoulos left a comment

ariskotsomitopoulos Apr 13, 2022

bmarty Apr 13, 2022

Set up dokka to generate SDK documentation and cleanup the API #5744

Set up dokka to generate SDK documentation and cleanup the API #5744

Conversation

bmarty commented Apr 12, 2022 • edited

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

Checklist

Choose a reason for hiding this comment

github-actions bot commented Apr 12, 2022 • edited

Unit Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouchadam left a comment

Choose a reason for hiding this comment

ariskotsomitopoulos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmarty commented Apr 12, 2022 •

edited

github-actions bot commented Apr 12, 2022 •

edited