Skip to content
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

Improve Matrix class #5887

Merged
merged 17 commits into from
May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .github/workflows/quality.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ jobs:
- name: Run code quality check suite
run: ./tools/check/check_code_quality.sh

# Knit for all the modules (https://github.com/Kotlin/kotlinx-knit)
knit:
name: Knit
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: Run knit
run: |
./gradlew knit

# ktlint for all the modules
ktlint:
name: Kotlin Linter
Expand Down
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ buildscript {
classpath "com.likethesalad.android:stem-plugin:2.0.0"
classpath 'org.owasp:dependency-check-gradle:7.1.0.1'
classpath "org.jetbrains.dokka:dokka-gradle-plugin:1.6.21"
classpath "org.jetbrains.kotlinx:kotlinx-knit:0.4.0"
// NOTE: Do not place your application dependencies here; they belong
// in the individual module build.gradle files
}
Expand Down
1 change: 1 addition & 0 deletions changelog.d/5887.sdk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Small change in the Matrix class: deprecated methods have been removed and the constructor is now public. Also the fun `workerFactory()` has been renamed to `getWorkerFactory()`
4 changes: 2 additions & 2 deletions docs/add_threePids.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ Wording: "We've sent you an email to verify your address. Please follow the inst
}
```

## User receive an e-mail
## User receives an e-mail

> [homeserver.org] Validate your email
> `homeserver.org` Validate your email
>
> A request to add an email address to your Matrix account has been received. If this was you, please click the link below to confirm adding this email:
https://homeserver.org/_matrix/client/unstable/add_threepid/email/submit_token?token=WUnEhQAmJrXupdEbXgdWvnVIKaGYZFsU&client_secret=TixzvOnw7nLEUdiQEmkHzkXKrY4HhiGh&sid=bxyDHuJKsdkjMlTJ
Expand Down
4 changes: 2 additions & 2 deletions docs/pull_request.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ Review such PR is the same recipe than for PR from Dependabot
##### Sync analytics plan

This tools imports any update in the analytics plan. See instruction in the PR itself to handle it.
More info can be found in the file [analytics.md]
More info can be found in the file [analytics.md](./analytics.md)

## Reviewing PR

Expand Down Expand Up @@ -247,4 +247,4 @@ Also "Resolve conversation" should probably be hit by the creator of the convers

PR submitter is responsible of the incoming change. PR reviewers who approved the PR take a part of responsibility on the code which will land to develop, and then be used by our users, and the user of our forks.

That said, bug may still be merged on `develop`, this is still acceptable of course. In this case, please make sure an issue is created and correctly labelled. Ideally, such issues should be fixed before the next release candidate, i.e. with a higher priority. But as we release the application every 10 working days, it can be hard to fix every bugs. That's why PR should be fully tested and reviewed before being merge and we should never comment code review remark with "will be handled later", or similar comments.
That said, bug may still be merged on `develop`, this is still acceptable of course. In this case, please make sure an issue is created and correctly labelled. Ideally, such issues should be fixed before the next release candidate, i.e. with a higher priority. But as we release the application every 10 working days, it can be hard to fix every bugs. That's why PR should be fully tested and reviewed before being merge and we should never comment code review remark with "will be handled later", or similar comments.
2 changes: 1 addition & 1 deletion matrix-sdk-android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dokkaHtml {
dokkaSourceSets {
configureEach {
// Emit warnings about not documented members.
reportUndocumented.set(true)
// reportUndocumented.set(true)
// Suppress legacy Riot's packages.
perPackageOption {
matchingRegex.set("org.matrix.android.sdk.internal.legacy.riot")
Expand Down
12 changes: 6 additions & 6 deletions matrix-sdk-android/docs/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ This pages list the complete API that this SDK is exposing to a client applicati

A few entry points:

- **Matrix**: The app will have to create and manage a Matrix object.
- From this **Matrix** object, you will be able to get various services, including the **AuthenticationService**.
- With this **AuthenticationService** you will be able to get an existing **Session**, or create one using a **LoginWizard** or a **RegistrationWizard**, which will finally give you a **Session**.
- From the **Session**, you will be able to retrieve many Services, including the **RoomService**.
- From the **RoomService**, you will be able to list the rooms, create a **Room**, and get a specific **Room**.
- And from a **Room**, you will be able to do many things, including get a **Timeline**, send messages, etc.
- **[Matrix](org.matrix.android.sdk.api.Matrix)**: The app will have to create and manage a **[Matrix](org.matrix.android.sdk.api.Matrix)** object.
- From this **[Matrix](org.matrix.android.sdk.api.Matrix)** object, you will be able to get various services, including the **[AuthenticationService](org.matrix.android.sdk.api.auth.AuthenticationService)**.
- With this **[AuthenticationService](org.matrix.android.sdk.api.auth.AuthenticationService)** you will be able to get an existing **[Session](org.matrix.android.sdk.api.session.Session)**, or create one using a **[LoginWizard](org.matrix.android.sdk.api.auth.login.LoginWizard)** or a **[RegistrationWizard](org.matrix.android.sdk.api.auth.registration.RegistrationWizard)**, which will finally give you a **[Session](org.matrix.android.sdk.api.session.Session)**.
- From the **[Session](org.matrix.android.sdk.api.session.Session)**, you will be able to retrieve many Services, including the **[RoomService](org.matrix.android.sdk.api.session.room.RoomService)**.
- From the **[RoomService](org.matrix.android.sdk.api.session.room.RoomService)**, you will be able to list the rooms, create a **[Room](org.matrix.android.sdk.api.session.room.Room)**, and get a specific **[Room](org.matrix.android.sdk.api.session.room.Room)**.
- And from a **[Room](org.matrix.android.sdk.api.session.room.Room)**, you will be able to do many things, including get a **[Timeline](org.matrix.android.sdk.api.session.room.timeline.Timeline)**, send messages, etc.

Please read the whole documentation to learn more!
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,17 @@ class CommonTestHelper(context: Context) {
fun getTestInterceptor(session: Session): MockOkHttpInterceptor? = TestModule.interceptorForSession(session.sessionId) as? MockOkHttpInterceptor

init {
var _matrix: TestMatrix? = null
UiThreadStatement.runOnUiThread {
TestMatrix.initialize(
_matrix = TestMatrix(
context,
MatrixConfiguration(
applicationFlavor = "TestFlavor",
roomDisplayNameFallbackProvider = TestRoomDisplayNameFallbackProvider()
)
)
}
matrix = TestMatrix.getInstance()
matrix = _matrix!!
}

fun createAccount(userNamePrefix: String, testParams: SessionTestParams): Session {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ import org.matrix.android.sdk.internal.util.BackgroundDetectionObserver
import org.matrix.android.sdk.internal.worker.MatrixWorkerFactory
import org.matrix.olm.OlmManager
import java.util.concurrent.Executors
import java.util.concurrent.atomic.AtomicBoolean
import javax.inject.Inject

/**
* This mimics the Matrix class but using TestMatrixComponent internally instead of regular MatrixComponent.
*/
internal class TestMatrix constructor(context: Context, matrixConfiguration: MatrixConfiguration) {
internal class TestMatrix(context: Context, matrixConfiguration: MatrixConfiguration) {

@Inject internal lateinit var legacySessionImporter: LegacySessionImporter
@Inject internal lateinit var authenticationService: AuthenticationService
Expand All @@ -60,13 +59,14 @@ internal class TestMatrix constructor(context: Context, matrixConfiguration: Mat
private val uiHandler = Handler(Looper.getMainLooper())

init {
Monarchy.init(context)
DaggerTestMatrixComponent.factory().create(context, matrixConfiguration).inject(this)
val appContext = context.applicationContext
Monarchy.init(appContext)
DaggerTestMatrixComponent.factory().create(appContext, matrixConfiguration).inject(this)
val configuration = Configuration.Builder()
.setExecutor(Executors.newCachedThreadPool())
.setWorkerFactory(matrixWorkerFactory)
.build()
WorkManager.initialize(context, configuration)
WorkManager.initialize(appContext, configuration)
uiHandler.post {
ProcessLifecycleOwner.get().lifecycle.addObserver(backgroundDetectionObserver)
}
Expand Down Expand Up @@ -95,23 +95,6 @@ internal class TestMatrix constructor(context: Context, matrixConfiguration: Mat
}

companion object {

private lateinit var instance: TestMatrix
private val isInit = AtomicBoolean(false)

fun initialize(context: Context, matrixConfiguration: MatrixConfiguration) {
if (isInit.compareAndSet(false, true)) {
instance = TestMatrix(context.applicationContext, matrixConfiguration)
}
}

fun getInstance(): TestMatrix {
if (isInit.compareAndSet(false, false)) {
throw IllegalStateException("Matrix is not initialized properly. You should call TestMatrix.initialize first")
}
return instance
}

fun getSdkVersion(): String {
return BuildConfig.SDK_VERSION + " (" + BuildConfig.GIT_SDK_REVISION + ")"
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,18 @@ import org.matrix.android.sdk.internal.util.BackgroundDetectionObserver
import org.matrix.android.sdk.internal.worker.MatrixWorkerFactory
import org.matrix.olm.OlmManager
import java.util.concurrent.Executors
import java.util.concurrent.atomic.AtomicBoolean
import javax.inject.Inject

/**
* This is the main entry point to the matrix sdk.
* <br/>
* See [Companion.createInstance] to create an instance. The app should create and manage the instance itself.
*
* The constructor creates a new instance of Matrix, it's recommended to manage this instance as a singleton.
*
* @param context the application context
* @param matrixConfiguration global configuration that will be used for every [org.matrix.android.sdk.api.session.Session]
*/
class Matrix private constructor(context: Context, matrixConfiguration: MatrixConfiguration) {
class Matrix(context: Context, matrixConfiguration: MatrixConfiguration) {

@Inject internal lateinit var legacySessionImporter: LegacySessionImporter
@Inject internal lateinit var authenticationService: AuthenticationService
Expand All @@ -61,89 +64,72 @@ class Matrix private constructor(context: Context, matrixConfiguration: MatrixCo
@Inject internal lateinit var lightweightSettingsStorage: LightweightSettingsStorage

init {
Monarchy.init(context)
DaggerMatrixComponent.factory().create(context, matrixConfiguration).inject(this)
if (context.applicationContext !is Configuration.Provider) {
val appContext = context.applicationContext
Monarchy.init(appContext)
DaggerMatrixComponent.factory().create(appContext, matrixConfiguration).inject(this)
if (appContext !is Configuration.Provider) {
val configuration = Configuration.Builder()
.setExecutor(Executors.newCachedThreadPool())
.setWorkerFactory(matrixWorkerFactory)
.build()
WorkManager.initialize(context, configuration)
WorkManager.initialize(appContext, configuration)
}
ProcessLifecycleOwner.get().lifecycle.addObserver(backgroundDetectionObserver)
}

/**
* Return the User Agent used for any request that the SDK is making to the homeserver.
* There is no way to change the user agent at the moment.
*/
fun getUserAgent() = userAgentHolder.userAgent

/**
* Return the AuthenticationService.
*/
fun authenticationService() = authenticationService
Copy link
Contributor

Choose a reason for hiding this comment

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

not for this PR, we have a mix of get${object} and ${object}, is it something we want to align on in the public api?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, we have a mix, the (un-official!) rule so far is to not use the get prefix for the SDK services, and I think this is the case for all services, to avoid having long name for the methods.

Happy to discuss this more, and/or to change the rule.

I have renamed workerFactory() because this is not a SDK service.


/**
* Return the RawService.
*/
fun rawService() = rawService

/**
* Return the LightweightSettingsStorage.
*/
fun lightweightSettingsStorage() = lightweightSettingsStorage

/**
* Return the HomeServerHistoryService.
*/
fun homeServerHistoryService() = homeServerHistoryService

/**
* Return the legacy session importer, useful if you want to migrate an app, which was using the legacy Matrix Android Sdk.
*/
fun legacySessionImporter() = legacySessionImporter

fun workerFactory(): WorkerFactory = matrixWorkerFactory
/**
* Get the worker factory. The returned value has to be provided to `WorkConfiguration.Builder()`.
*/
fun getWorkerFactory(): WorkerFactory = matrixWorkerFactory

/**
* Register an API interceptor, to be able to be notified when the specified API got a response.
*/
fun registerApiInterceptorListener(path: ApiPath, listener: ApiInterceptorListener) {
apiInterceptor.addListener(path, listener)
}

/**
* Un-register an API interceptor.
*/
fun unregisterApiInterceptorListener(path: ApiPath, listener: ApiInterceptorListener) {
apiInterceptor.removeListener(path, listener)
}

companion object {

private lateinit var instance: Matrix
private val isInit = AtomicBoolean(false)

/**
* Creates a new instance of Matrix, it's recommended to manage this instance as a singleton.
* To make use of the built in singleton use Matrix.initialize() and/or Matrix.getInstance(context) instead
**/
fun createInstance(context: Context, matrixConfiguration: MatrixConfiguration): Matrix {
return Matrix(context.applicationContext, matrixConfiguration)
}

/**
* Initializes a singleton instance of Matrix for the given MatrixConfiguration
* This instance will be returned by Matrix.getInstance(context)
*/
@Deprecated("Use Matrix.createInstance and manage the instance manually")
fun initialize(context: Context, matrixConfiguration: MatrixConfiguration) {
if (isInit.compareAndSet(false, true)) {
instance = Matrix(context.applicationContext, matrixConfiguration)
}
}

/**
* Either provides an already initialized singleton Matrix instance or queries the application context for a MatrixConfiguration.Provider
* to lazily create and store the instance.
*/
@Suppress("deprecation") // suppressing warning as this method is unused but is still provided for SDK clients
@Deprecated("Use Matrix.createInstance and manage the instance manually")
fun getInstance(context: Context): Matrix {
if (isInit.compareAndSet(false, true)) {
val appContext = context.applicationContext
if (appContext is MatrixConfiguration.Provider) {
val matrixConfiguration = (appContext as MatrixConfiguration.Provider).providesMatrixConfiguration()
instance = Matrix(appContext, matrixConfiguration)
} else {
throw IllegalStateException(
"Matrix is not initialized properly." +
" If you want to manage your own Matrix instance use Matrix.createInstance" +
" otherwise you should call Matrix.initialize or let your application implement MatrixConfiguration.Provider."
)
}
}
return instance
}

/**
* @return a String with details about the Matrix SDK version
* @return a String with details about the Matrix SDK version.
*/
fun getSdkVersion(): String {
return BuildConfig.SDK_VERSION + " (" + BuildConfig.GIT_SDK_REVISION + ")"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,4 @@ data class MatrixConfiguration(
* Thread messages default enable/disabled value
*/
val threadMessagesEnabledDefault: Boolean = false,
) {

/**
* Can be implemented by your Application class.
*/
@Deprecated("Use Matrix.createInstance and manage the instance manually instead of Matrix.getInstance")
interface Provider {
fun providesMatrixConfiguration(): MatrixConfiguration
}
}
)
13 changes: 13 additions & 0 deletions vector/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,24 @@ apply plugin: 'kotlin-parcelize'
apply plugin: 'kotlin-kapt'
apply plugin: 'com.likethesalad.stem'
apply plugin: 'dagger.hilt.android.plugin'
apply plugin: 'kotlinx-knit'

kapt {
correctErrorTypes = true
}

knit {
files = fileTree(project.rootDir) {
include '**/*.md'
include '**/*.kt'
include '**/*.kts'
exclude '**/build/**'
exclude '**/.gradle/**'
exclude '**/towncrier/template.md'
exclude '**/CHANGES.md'
}
}

// Note: 2 digits max for each value
ext.versionMajor = 1
ext.versionMinor = 4
Expand Down
4 changes: 4 additions & 0 deletions vector/lint.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<lint>

<!-- Ensure this file does not contain unknown Ids -->
<issue id="UnknownIssueId" severity="error" />

<!-- Modify some severity -->

<!-- Resource -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ fun getMatrixInstance(): Matrix {
val configuration = MatrixConfiguration(
roomDisplayNameFallbackProvider = VectorRoomDisplayNameFallbackProvider(context)
)
return Matrix.createInstance(context, configuration)
return Matrix(context, configuration)
}
2 changes: 1 addition & 1 deletion vector/src/main/java/im/vector/app/VectorApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ class VectorApplication :

override fun getWorkManagerConfiguration(): WorkConfiguration {
return WorkConfiguration.Builder()
.setWorkerFactory(matrix.workerFactory())
.setWorkerFactory(matrix.getWorkerFactory())
.setExecutor(Executors.newCachedThreadPool())
.build()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ object VectorStaticModule {
@Provides
@Singleton
fun providesMatrix(context: Context, configuration: MatrixConfiguration): Matrix {
return Matrix.createInstance(context, configuration)
return Matrix(context, configuration)
}

@Provides
Expand Down