Metro: last three sync plugins registered from a container, Dagger keeps ownership

Author: MilosKozak

app/src/main/kotlin/app/aaps/di/CoreObjectsModule.kt

@@ -116,6 +116,9 @@ import app.aaps.core.objects.wizard.BolusWizard
 import app.aaps.core.objects.wizard.QuickWizard
 import app.aaps.core.ui.search.SearchableProvider
 import app.aaps.core.utils.receivers.DataInbox
+import app.aaps.plugins.sync.wear.WearPlugin
+import app.aaps.plugins.sync.nsclientV3.NSClientV3Plugin
+import app.aaps.plugins.sync.smsCommunicator.SmsCommunicatorPlugin
 import app.aaps.implementation.plugin.PluginStore
 import app.aaps.di.metro.AapsLeaves
 import app.aaps.di.metro.MetroGraphs
@@ -454,6 +457,9 @@ class CoreObjectsModule {
         versionCheckerUtilsProvider: Provider<VersionCheckerUtils>,
         searchableProvidersProvider: Provider<Set<SearchableProvider>>,
         permissionProvidersProvider: Provider<Set<PermissionProvider>>,
+        smsCommunicatorPluginProvider: Provider<SmsCommunicatorPlugin>,
+        nsClientV3PluginProvider: Provider<NSClientV3Plugin>,
+        wearPluginProvider: Provider<WearPlugin>,
         bolusProgressDataProvider: Provider<BolusProgressData>,
         pumpEnactResultProvider: Provider<PumpEnactResult>,
         historyScopeProvider: Provider<HistoryScope>,
@@ -492,6 +498,9 @@ class CoreObjectsModule {
         versionCheckerUtilsProvider,
         searchableProvidersProvider,
         permissionProvidersProvider,
+        smsCommunicatorPluginProvider,
+        nsClientV3PluginProvider,
+        wearPluginProvider,
         bolusProgressDataProvider,
         pumpEnactResultProvider,
         historyScopeProvider,

app/src/main/kotlin/app/aaps/di/metro/AapsLeaves.kt

@@ -48,6 +48,9 @@ import app.aaps.core.keys.interfaces.Preferences
 import app.aaps.core.nssdk.interfaces.RunningConfiguration
 import app.aaps.core.ui.search.SearchableProvider
 import app.aaps.core.utils.receivers.DataInbox
+import app.aaps.plugins.sync.wear.WearPlugin
+import app.aaps.plugins.sync.nsclientV3.NSClientV3Plugin
+import app.aaps.plugins.sync.smsCommunicator.SmsCommunicatorPlugin
 import app.aaps.implementation.maintenance.cloud.CloudStorageManager
 import app.aaps.implementation.scenes.SceneExecutor
 import app.aaps.plugins.automation.services.LastLocationDataContainer
@@ -136,6 +139,9 @@ class AapsLeaves(
     // binding rather than re-declaring the multibinding on this side.
     private val searchableProvidersProvider: Provider<Set<SearchableProvider>>,
     private val permissionProvidersProvider: Provider<Set<PermissionProvider>>,
+    private val smsCommunicatorPluginProvider: Provider<SmsCommunicatorPlugin>,
+    private val nsClientV3PluginProvider: Provider<NSClientV3Plugin>,
+    private val wearPluginProvider: Provider<WearPlugin>,
     /**
      * The history browser scope.
      *
@@ -222,6 +228,13 @@ class AapsLeaves(
     // LocationServiceController, ReminderScheduler and BtConnectionSource, none of which the graph
     // reaches yet. PluginStore takes it through a lambda, so nothing is built until it is asked for.
     @Provides fun permissionProviders(): Set<PermissionProvider> = permissionProvidersProvider.get()
+
+    // Dagger owns these three, the same way it owns the pump drivers: AuthRequest, the nine
+    // @HiltWorker loaders under nsclientV3 and the wear data layer all inject the concrete class, so
+    // Dagger builds them and Metro borrows. See SyncPluginsBindings for how they reach the plugin map.
+    @Provides fun smsCommunicatorPlugin(): SmsCommunicatorPlugin = smsCommunicatorPluginProvider.get()
+    @Provides fun nsClientV3Plugin(): NSClientV3Plugin = nsClientV3PluginProvider.get()
+    @Provides fun wearPlugin(): WearPlugin = wearPluginProvider.get()
     @Provides fun overviewDataCache(): OverviewDataCache = overviewDataCacheProvider.get()
 
     /** Hilt's qualifier, read now that interop is on. Same Context as the unqualified binding. */

app/src/main/kotlin/app/aaps/di/metro/SyncPluginsBindings.kt

@@ -0,0 +1,68 @@
+package app.aaps.di.metro
+
+import app.aaps.core.interfaces.di.NotNSClient
+import app.aaps.core.interfaces.plugin.PluginBase
+import app.aaps.plugins.sync.nsclientV3.NSClientV3Plugin
+import app.aaps.plugins.sync.smsCommunicator.SmsCommunicatorPlugin
+import app.aaps.plugins.sync.wear.WearPlugin
+import dev.zacsweers.metro.AppScope
+import dev.zacsweers.metro.BindingContainer
+import dev.zacsweers.metro.ContributesTo
+import dev.zacsweers.metro.IntKey
+import dev.zacsweers.metro.IntoMap
+import dev.zacsweers.metro.Provides
+import dev.zacsweers.metro.SingleIn
+
+/**
+ * The last three `:plugins:sync` plugins, registered from here rather than from the class.
+ *
+ * ## Why not annotate the class, like the other four
+ *
+ * Dagger's KSP `InjectProcessingStep` runs over every class it has to construct, reads the annotations
+ * on it, and cannot resolve `dev.zacsweers.metro.IntKey`:
+ *
+ * ```
+ * InjectProcessingStep was unable to process 'NSClientV3Plugin(...)'
+ *   => annotation: @IntKey  => type (ERROR annotation type): error.NonExistentClass
+ * ```
+ *
+ * Tidepool, Xdrip, Tizen and Garmin have no Dagger-built consumer, so Dagger never processes them and
+ * they carry their own `@ContributesIntoMap`. These three do: `AuthRequest` injects
+ * [SmsCommunicatorPlugin], the nine `@HiltWorker` loaders inject [NSClientV3Plugin], and the wear data
+ * layer injects [WearPlugin]. Registering the entry here keeps the Metro annotations off the class.
+ *
+ * ## Who owns the instance
+ *
+ * **Dagger**, for the same reason it owns the pump drivers: the classes above are Dagger-built, so
+ * Dagger's copy is the live one. These take the plugin from `AapsLeaves`, which hands Dagger's singleton
+ * over, so the object in the plugin map is the object those consumers use. Giving Metro its own would be
+ * the split-brain that `PumpLeaves` documents - a plugin in the list that nothing ever writes to.
+ *
+ * `@SingleIn` here does not compete with that: it makes Metro hold **one reference to Dagger's object**
+ * rather than calling the leaf on every read. Without it `ContributedPluginsTest` fails with "plugin 350
+ * is rebuilt on every read", because a plugin has to be one object for its enabled state to mean
+ * anything.
+ */
+@ContributesTo(AppScope::class)
+@BindingContainer
+object SyncPluginsBindings {
+
+    @Provides
+    @SingleIn(AppScope::class)
+    @IntoMap
+    @NotNSClient
+    @IntKey(300)
+    fun smsCommunicatorPlugin(plugin: SmsCommunicatorPlugin): PluginBase = plugin
+
+    @Provides
+    @SingleIn(AppScope::class)
+    @IntoMap
+    @IntKey(310)
+    fun nsClientV3Plugin(plugin: NSClientV3Plugin): PluginBase = plugin
+
+    @Provides
+    @SingleIn(AppScope::class)
+    @IntoMap
+    @IntKey(350)
+    fun wearPlugin(plugin: WearPlugin): PluginBase = plugin
+}

app/src/test/kotlin/app/aaps/di/metro/ContributedPluginsTest.kt

@@ -12,6 +12,7 @@ import app.aaps.plugins.smoothing.UnscentedKalmanFilterPlugin
 import com.google.common.truth.Truth.assertThat
 import com.google.common.truth.Truth.assertWithMessage
 import org.junit.jupiter.api.Test
+import org.mockito.Mockito.mockingDetails
 
 /**
  * Plugins that register themselves with `@ContributesIntoMap` really reach the root graph.
@@ -42,9 +43,9 @@ class ContributedPluginsTest {
             // source - all sixteen. 400, 410 and 440 are also bound to an interface; Dagger delegates
             // to these instances in CoreObjectsModule rather than building its own.
             400, 410, 420, 430, 440, 450, 460, 470, 480, 490, 500, 510, 520, 530, 540, 550,
-            // sync - the every-build part of the 300 block. SmsCommunicator 300 and NSClientV3 310 are
-            // still Dagger, and Tidepool 320 is @NotNSClient, so only these three are here.
-            330, 360, 370,
+            // sync - the every-build part of the 300 block. SmsCommunicator 300 and Tidepool 320 are
+            // @NotNSClient; 340 (OpenHumans) comes from its own graph extension.
+            310, 330, 350, 360, 370,
             // smoothing
             600, 610, 620, 630,
             // calibration
@@ -68,6 +69,22 @@ class ContributedPluginsTest {
             .containsExactly(1010, 1020, 1030, 1040, 1050, 1060, 1080, 1090, 1100, 1120, 1130)
     }
 
+    @Test
+    fun `the three Dagger-owned sync plugins are borrowed, not rebuilt by Metro`() {
+        // Same rule as the pump drivers, and the same check: testRoot() mocks AapsLeaves, so a plugin
+        // handed over by a leaf comes back a mock while one Metro constructed comes back real. These
+        // three must stay Dagger's, because AuthRequest, the nine @HiltWorker loaders and the wear data
+        // layer all inject the concrete class - a second copy would sit in the plugin list unused.
+        val root = testRoot()
+        val borrowed = listOf(
+            300 to root.contributedNotNsClientPlugins[300],
+            310 to root.contributedPlugins[310],
+            350 to root.contributedPlugins[350]
+        )
+
+        assertThat(borrowed.filter { !mockingDetails(it.second).isMock }).isEmpty()
+    }
+
     @Test
     fun `the xdrip plugin and the XDripBroadcast binding are the same object`() {
         // XdripPlugin carries two contributions: into the plugin map, and as XDripBroadcast. Under
@@ -108,7 +125,7 @@ class ContributedPluginsTest {
     fun `only a non-follower build gets the version checker and Tidepool`() {
         // 340 (OpenHumans) is also @NotNSClient but comes from its own graph extension, so it joins
         // this bucket in MetroGraphs.notNsClientPlugins() rather than appearing here.
-        assertThat(testRoot().contributedNotNsClientPlugins.keys).containsExactly(320, 810)
+        assertThat(testRoot().contributedNotNsClientPlugins.keys).containsExactly(300, 320, 810)
     }
 
     @Test