-
Notifications
You must be signed in to change notification settings - Fork 913
Decoupling Matrix creation from Application #5185
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
Changes from 9 commits
2d80c6b
c160f5d
a63e79b
8dc8c20
0390994
5ff420f
c62b994
674aea9
fd2d928
2f7f86b
2eb417a
d05af1c
95df3e7
8e0f580
5b851f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Avoids using Matrix.getInstance() within the SyncService and instead relies on the SDK client to provide the matrix instance |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,12 +99,28 @@ 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 recommend to manage this instance as a singleton. | ||
| * To make use of the built in singleton use Matrix.initialise() and/or Matrix.getInstance(context) instead | ||
|
ouchadam marked this conversation as resolved.
Outdated
|
||
| **/ | ||
| 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) | ||
| */ | ||
| 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. | ||
| */ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| fun getInstance(context: Context): Matrix { | ||
| if (isInit.compareAndSet(false, true)) { | ||
| val appContext = context.applicationContext | ||
|
|
||
| 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) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
|
@@ -77,7 +75,6 @@ import androidx.work.Configuration as WorkConfiguration | |
| @HiltAndroidApp | ||
| class VectorApplication : | ||
| Application(), | ||
| MatrixConfiguration.Provider, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I defer to @ganfra :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. added deprecations 95df3e7 |
||
| WorkConfiguration.Provider { | ||
|
|
||
| lateinit var appContext: Context | ||
|
|
@@ -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 | ||
|
|
@@ -220,16 +218,9 @@ class VectorApplication : | |
| } | ||
| } | ||
|
|
||
| override fun providesMatrixConfiguration(): MatrixConfiguration { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the main downside to using |
||
| 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() | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that! |
||
| ) { | ||
| var inMultiWindowMode = false | ||
|
|
||
|
|
@@ -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) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.