Skip to content
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
1 change: 1 addition & 0 deletions changelog.d/5185.sdk
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecates Matrix.initialize and Matrix.getInstance in favour of the client providing its own singleton instance via Matrix.createInstance
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,31 @@ class Matrix private constructor(context: Context, matrixConfiguration: MatrixCo
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.
*/
Copy link
Member

Choose a reason for hiding this comment

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

Maybe update the comment at line 132 ("You should call Matrix.initialize or let your application implements MatrixConfiguration.Provider.") to inform developers that they can manage the singleton themselves by using createInstance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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
Expand All @@ -113,7 +132,8 @@ class Matrix private constructor(context: Context, matrixConfiguration: MatrixCo
instance = Matrix(appContext, matrixConfiguration)
} else {
throw IllegalStateException("Matrix is not initialized properly." +
" You should call Matrix.initialize or let your application implements MatrixConfiguration.Provider.")
" 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ data class MatrixConfiguration(
/**
* 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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,14 @@ abstract class SyncService : Service() {
}
}

abstract fun provideMatrix(): Matrix

private fun initialize(intent: Intent?): Boolean {
if (intent == null) {
Timber.d("## Sync: initialize intent is null")
return false
}
val matrix = Matrix.getInstance(applicationContext)
val matrix = provideMatrix()
val safeSessionId = intent.getStringExtra(EXTRA_SESSION_ID) ?: return false
syncTimeoutSeconds = intent.getIntExtra(EXTRA_TIMEOUT_SECONDS, getDefaultSyncTimeoutSeconds())
syncDelaySeconds = intent.getIntExtra(EXTRA_DELAY_SECONDS, getDefaultSyncDelaySeconds())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import androidx.test.platform.app.InstrumentationRegistry
import im.vector.app.core.utils.getMatrixInstance
import im.vector.app.features.MainActivity
import im.vector.app.features.crypto.recover.SetupMode
import im.vector.app.features.home.HomeActivity
Expand All @@ -47,7 +47,6 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.session.Session

@RunWith(AndroidJUnit4::class)
Expand All @@ -61,8 +60,7 @@ class SecurityBootstrapTest : VerificationTestBase() {

@Before
fun createSessionWithCrossSigning() {
val context = InstrumentationRegistry.getInstrumentation().targetContext
val matrix = Matrix.getInstance(context)
val matrix = getMatrixInstance()
val userName = "foobar_${System.currentTimeMillis()}"
existingSession = createAccountAndSync(matrix, userName, password, true)
stubAllExternalIntents()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,14 @@ import androidx.test.espresso.matcher.ViewMatchers.withText
import androidx.test.ext.junit.rules.ActivityScenarioRule
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import androidx.test.platform.app.InstrumentationRegistry
import im.vector.app.core.utils.getMatrixInstance
import im.vector.app.features.MainActivity
import im.vector.app.features.home.HomeActivity
import org.hamcrest.CoreMatchers.not
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.auth.UIABaseAuth
import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
import org.matrix.android.sdk.api.auth.UserPasswordAuth
Expand All @@ -66,8 +65,7 @@ class VerifySessionInteractiveTest : VerificationTestBase() {

@Before
fun createSessionWithCrossSigning() {
val context = InstrumentationRegistry.getInstrumentation().targetContext
val matrix = Matrix.getInstance(context)
val matrix = getMatrixInstance()
val userName = "foobar_${System.currentTimeMillis()}"
existingSession = createAccountAndSync(matrix, userName, password, true)
doSync<Unit> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.filters.LargeTest
import androidx.test.platform.app.InstrumentationRegistry
import im.vector.app.core.resources.StringProvider
import im.vector.app.core.utils.getMatrixInstance
import im.vector.app.features.MainActivity
import im.vector.app.features.crypto.quads.SharedSecureStorageActivity
import im.vector.app.features.crypto.recover.BootstrapCrossSigningTask
Expand All @@ -45,7 +46,6 @@ import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.auth.UIABaseAuth
import org.matrix.android.sdk.api.auth.UserInteractiveAuthInterceptor
import org.matrix.android.sdk.api.auth.UserPasswordAuth
Expand All @@ -67,7 +67,7 @@ class VerifySessionPassphraseTest : VerificationTestBase() {
@Before
fun createSessionWithCrossSigningAnd4S() {
val context = InstrumentationRegistry.getInstrumentation().targetContext
val matrix = Matrix.getInstance(context)
val matrix = getMatrixInstance()
val userName = "foobar_${System.currentTimeMillis()}"
existingSession = createAccountAndSync(matrix, userName, password, true)
doSync<Unit> {
Expand All @@ -90,7 +90,7 @@ class VerifySessionPassphraseTest : VerificationTestBase() {

runBlocking {
task.execute(Params(
userInteractiveAuthInterceptor = object : UserInteractiveAuthInterceptor {
userInteractiveAuthInterceptor = object : UserInteractiveAuthInterceptor {
override fun performStage(flowResponse: RegistrationFlowResponse, errCode: String?, promise: Continuation<UIABaseAuth>) {
promise.resume(
UserPasswordAuth(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/*
* Copyright (c) 2022 New Vector Ltd
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package im.vector.app.core.utils

import androidx.test.platform.app.InstrumentationRegistry
import im.vector.app.features.room.VectorRoomDisplayNameFallbackProvider
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.MatrixConfiguration

fun getMatrixInstance(): Matrix {
val context = InstrumentationRegistry.getInstrumentation().targetContext
val configuration = MatrixConfiguration(
roomDisplayNameFallbackProvider = VectorRoomDisplayNameFallbackProvider(context)
)
return Matrix.createInstance(context, configuration)
}
13 changes: 2 additions & 11 deletions vector/src/main/java/im/vector/app/VectorApplication.kt
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,13 @@ import im.vector.app.features.pin.PinLocker
import im.vector.app.features.popup.PopupAlertManager
import im.vector.app.features.rageshake.VectorFileLogger
import im.vector.app.features.rageshake.VectorUncaughtExceptionHandler
import im.vector.app.features.room.VectorRoomDisplayNameFallbackProvider
import im.vector.app.features.settings.VectorLocale
import im.vector.app.features.settings.VectorPreferences
import im.vector.app.features.themes.ThemeUtils
import im.vector.app.features.version.VersionProvider
import im.vector.app.push.fcm.FcmHelper
import org.jitsi.meet.sdk.log.JitsiMeetDefaultLogHandler
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.MatrixConfiguration
import org.matrix.android.sdk.api.auth.AuthenticationService
import org.matrix.android.sdk.api.legacy.LegacySessionImporter
import timber.log.Timber
Expand All @@ -77,7 +75,6 @@ import androidx.work.Configuration as WorkConfiguration
@HiltAndroidApp
class VectorApplication :
Application(),
MatrixConfiguration.Provider,
Copy link
Member

Choose a reason for hiding this comment

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

Maybe delete this interface? Or can it still be used by other application?

Copy link
Contributor Author

@ouchadam ouchadam Feb 14, 2022

Choose a reason for hiding this comment

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

it could be used by other clients of the SDK, if we agree that the Matrix singleton API should avoid querying the application then we could mark the interface as deprecated and schedule removing it? potentially with some migration steps~

what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I defer to @ganfra :)

Copy link
Member

Choose a reason for hiding this comment

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

Yes I think we should remove this and the inner singleton to avoid mistakes!
Regarding the process, we could indeed mark them as deprecated in a first place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added deprecations 95df3e7

WorkConfiguration.Provider {

lateinit var appContext: Context
Expand All @@ -100,6 +97,7 @@ class VectorApplication :
@Inject lateinit var autoRageShaker: AutoRageShaker
@Inject lateinit var vectorFileLogger: VectorFileLogger
@Inject lateinit var vectorAnalytics: VectorAnalytics
@Inject lateinit var matrix: Matrix

// font thread handler
private var fontThreadHandler: Handler? = null
Expand Down Expand Up @@ -220,16 +218,9 @@ class VectorApplication :
}
}

override fun providesMatrixConfiguration(): MatrixConfiguration {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the main downside to using MatrixConfiguration.Provider is that it occurs during super.onCreate which means the dependencies declared in the application haven't been injected yet, making the object creation lifecycle a bit misleading

return MatrixConfiguration(
applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION,
roomDisplayNameFallbackProvider = VectorRoomDisplayNameFallbackProvider(this)
)
}

override fun getWorkManagerConfiguration(): WorkConfiguration {
return WorkConfiguration.Builder()
.setWorkerFactory(Matrix.getInstance(this.appContext).workerFactory())
.setWorkerFactory(matrix.workerFactory())
.setExecutor(Executors.newCachedThreadPool())
.build()
}
Expand Down
16 changes: 14 additions & 2 deletions vector/src/main/java/im/vector/app/core/di/SingletonModule.kt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import dagger.Module
import dagger.Provides
import dagger.hilt.InstallIn
import dagger.hilt.components.SingletonComponent
import im.vector.app.BuildConfig
import im.vector.app.EmojiCompatWrapper
import im.vector.app.EmojiSpanify
import im.vector.app.core.dispatchers.CoroutineDispatchers
Expand All @@ -42,12 +43,14 @@ import im.vector.app.features.navigation.DefaultNavigator
import im.vector.app.features.navigation.Navigator
import im.vector.app.features.pin.PinCodeStore
import im.vector.app.features.pin.SharedPrefPinCodeStore
import im.vector.app.features.room.VectorRoomDisplayNameFallbackProvider
import im.vector.app.features.ui.SharedPreferencesUiStateRepository
import im.vector.app.features.ui.UiStateRepository
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.SupervisorJob
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.api.MatrixConfiguration
import org.matrix.android.sdk.api.auth.AuthenticationService
import org.matrix.android.sdk.api.auth.HomeServerHistoryService
import org.matrix.android.sdk.api.legacy.LegacySessionImporter
Expand Down Expand Up @@ -107,8 +110,17 @@ object VectorStaticModule {
}

@Provides
fun providesMatrix(context: Context): Matrix {
return Matrix.getInstance(context)
fun providesMatrixConfiguration(vectorRoomDisplayNameFallbackProvider: VectorRoomDisplayNameFallbackProvider): MatrixConfiguration {
return MatrixConfiguration(
applicationFlavor = BuildConfig.FLAVOR_DESCRIPTION,
roomDisplayNameFallbackProvider = vectorRoomDisplayNameFallbackProvider
)
}

@Provides
@Singleton
fun providesMatrix(context: Context, configuration: MatrixConfiguration): Matrix {
return Matrix.createInstance(context, configuration)
}

@Provides
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import im.vector.app.R
import im.vector.app.core.platform.PendingIntentCompat
import im.vector.app.features.notifications.NotificationUtils
import im.vector.app.features.settings.BackgroundSyncMode
import org.matrix.android.sdk.api.Matrix
import org.matrix.android.sdk.internal.session.sync.job.SyncService
import timber.log.Timber
import javax.inject.Inject
Expand Down Expand Up @@ -75,6 +76,9 @@ class VectorSyncService : SyncService() {
}

@Inject lateinit var notificationUtils: NotificationUtils
@Inject lateinit var matrix: Matrix

override fun provideMatrix() = matrix

override fun getDefaultSyncDelaySeconds() = BackgroundSyncMode.DEFAULT_SYNC_DELAY_SECONDS

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ class BugReporter @Inject constructor(
private val versionProvider: VersionProvider,
private val vectorPreferences: VectorPreferences,
private val vectorFileLogger: VectorFileLogger,
private val systemLocaleProvider: SystemLocaleProvider
private val systemLocaleProvider: SystemLocaleProvider,
private val matrix: Matrix
Copy link
Member

Choose a reason for hiding this comment

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

I like that!

) {
var inMultiWindowMode = false

Expand Down Expand Up @@ -265,7 +266,7 @@ class BugReporter @Inject constructor(
val builder = BugReporterMultipartBody.Builder()
.addFormDataPart("text", text)
.addFormDataPart("app", rageShakeAppNameForReport(reportType))
.addFormDataPart("user_agent", Matrix.getInstance(context).getUserAgent())
.addFormDataPart("user_agent", matrix.getUserAgent())
.addFormDataPart("user_id", userId)
.addFormDataPart("can_contact", canContact.toString())
.addFormDataPart("device_id", deviceId)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ package im.vector.app.features.room
import android.content.Context
import im.vector.app.R
import org.matrix.android.sdk.api.RoomDisplayNameFallbackProvider
import javax.inject.Inject

class VectorRoomDisplayNameFallbackProvider(
class VectorRoomDisplayNameFallbackProvider @Inject constructor(
private val context: Context
) : RoomDisplayNameFallbackProvider {

Expand Down