Merge pull request #6275 from grzesiek2010/COLLECT-5976

Fixed memory leaks in form-filling
getodk · Jul 22, 2024 · 8dcf4f9 · 8dcf4f9
2 parents a5dc0e2 + 1dad86a
commit 8dcf4f9
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 19 deletions.
diff --git a/collect_app/src/main/java/org/odk/collect/android/activities/FormEntryViewModelFactory.kt b/collect_app/src/main/java/org/odk/collect/android/activities/FormEntryViewModelFactory.kt
@@ -129,10 +129,10 @@ class FormEntryViewModelFactory(
                     fusedLocationClient,
                     BackgroundLocationHelper(
                         permissionsProvider,
-                        settingsProvider.getUnprotectedSettings()
-                    ) {
-                        formSessionRepository.get(sessionId).value?.formController
-                    }
+                        settingsProvider.getUnprotectedSettings(),
+                        formSessionRepository,
+                        sessionId
+                    )
                 )
 
                 BackgroundLocationViewModel(locationManager)

diff --git a/collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java b/collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java
@@ -429,7 +429,7 @@ public void onCreate(Bundle savedInstanceState) {
                 autoSendSettingsProvider,
                 formsRepositoryProvider,
                 instancesRepositoryProvider,
-                new SavepointsRepositoryProvider(this, storagePathProvider),
+                savepointsRepositoryProvider,
                 new QRCodeCreatorImpl(),
                 new HtmlPrinter(),
                 instancesDataService

diff --git a/...n/java/org/odk/collect/android/formentry/backgroundlocation/BackgroundLocationHelper.java b/...n/java/org/odk/collect/android/formentry/backgroundlocation/BackgroundLocationHelper.java
@@ -6,14 +6,16 @@
 
 import org.odk.collect.android.activities.FormFillingActivity;
 import org.odk.collect.android.application.Collect;
+import org.odk.collect.android.formentry.FormSession;
+import org.odk.collect.android.formentry.FormSessionRepository;
 import org.odk.collect.android.formentry.audit.AuditConfig;
 import org.odk.collect.android.formentry.audit.AuditEvent;
 import org.odk.collect.android.javarosawrapper.FormController;
 import org.odk.collect.androidshared.system.PlayServicesChecker;
 import org.odk.collect.permissions.PermissionsProvider;
 import org.odk.collect.shared.settings.Settings;
 
-import java.util.function.Supplier;
+import javax.annotation.Nullable;
 
 /**
  * Wrapper on resources needed by {@link BackgroundLocationManager} to make testing easier.
@@ -29,12 +31,19 @@ public class BackgroundLocationHelper {
 
     private final PermissionsProvider permissionsProvider;
     private final Settings generalSettings;
-    private final Supplier<FormController> formControllerProvider;
-
-    public BackgroundLocationHelper(PermissionsProvider permissionsProvider, Settings generalSettings, Supplier<FormController> formControllerProvider) {
+    private final FormSessionRepository formSessionRepository;
+    private final String sessionId;
+
+    public BackgroundLocationHelper(
+            PermissionsProvider permissionsProvider,
+            Settings generalSettings,
+            FormSessionRepository formSessionRepository,
+            String sessionId
+    ) {
         this.permissionsProvider = permissionsProvider;
         this.generalSettings = generalSettings;
-        this.formControllerProvider = formControllerProvider;
+        this.formSessionRepository = formSessionRepository;
+        this.sessionId = sessionId;
     }
 
     boolean isAndroidLocationPermissionGranted() {
@@ -53,7 +62,7 @@ boolean arePlayServicesAvailable() {
      * @return true if the global form controller has been initialized.
      */
     boolean isCurrentFormSet() {
-        return formControllerProvider.get() != null;
+        return getFormController() != null;
     }
 
     /**
@@ -62,7 +71,7 @@ boolean isCurrentFormSet() {
      * Precondition: the global form controller has been initialized.
      */
     boolean currentFormCollectsBackgroundLocation() {
-        return formControllerProvider.get().currentFormCollectsBackgroundLocation();
+        return getFormController().currentFormCollectsBackgroundLocation();
     }
 
     /**
@@ -72,7 +81,7 @@ boolean currentFormCollectsBackgroundLocation() {
      * Precondition: the global form controller has been initialized.
      */
     boolean currentFormAuditsLocation() {
-        return formControllerProvider.get().currentFormAuditsLocation();
+        return getFormController().currentFormAuditsLocation();
     }
 
     /**
@@ -81,7 +90,7 @@ boolean currentFormAuditsLocation() {
      * Precondition: the global form controller has been initialized.
      */
     AuditConfig getCurrentFormAuditConfig() {
-        return formControllerProvider.get().getSubmissionMetadata().auditConfig;
+        return getFormController().getSubmissionMetadata().auditConfig;
     }
 
     /**
@@ -90,7 +99,7 @@ AuditConfig getCurrentFormAuditConfig() {
      * Precondition: the global form controller has been initialized.
      */
     void logAuditEvent(AuditEvent.AuditEventType eventType) {
-        formControllerProvider.get().getAuditEventLogger().logEvent(eventType, false, System.currentTimeMillis());
+        getFormController().getAuditEventLogger().logEvent(eventType, false, System.currentTimeMillis());
     }
 
     /**
@@ -99,6 +108,12 @@ void logAuditEvent(AuditEvent.AuditEventType eventType) {
      * Precondition: the global form controller has been initialized.
      */
     void provideLocationToAuditLogger(Location location) {
-        formControllerProvider.get().getAuditEventLogger().addLocation(location);
+        getFormController().getAuditEventLogger().addLocation(location);
+    }
+
+    @Nullable
+    private FormController getFormController() {
+        FormSession formSession = formSessionRepository.get(sessionId).getValue();
+        return formSession == null ? null : formSession.getFormController();
     }
 }
diff --git a/collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyActivity.java b/collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyActivity.java
@@ -59,7 +59,6 @@
 import org.odk.collect.android.javarosawrapper.FormController;
 import org.odk.collect.android.javarosawrapper.JavaRosaFormController;
 import org.odk.collect.android.projects.ProjectsDataService;
-import org.odk.collect.android.storage.StoragePathProvider;
 import org.odk.collect.android.utilities.ApplicationConstants;
 import org.odk.collect.android.utilities.FormEntryPromptUtils;
 import org.odk.collect.android.utilities.FormsRepositoryProvider;
@@ -194,7 +193,7 @@ public class FormHierarchyActivity extends LocalizedActivity implements DeleteRe
     public FormsRepositoryProvider formsRepositoryProvider;
 
     @Inject
-    public StoragePathProvider storagePathProvider;
+    public SavepointsRepositoryProvider savepointsRepositoryProvider;
 
     @Inject
     public InstancesDataService instancesDataService;
@@ -232,7 +231,7 @@ public void onCreate(Bundle savedInstanceState) {
                 autoSendSettingsProvider,
                 formsRepositoryProvider,
                 instancesRepositoryProvider,
-                new SavepointsRepositoryProvider(this, storagePathProvider),
+                savepointsRepositoryProvider,
                 new QRCodeCreatorImpl(),
                 new HtmlPrinter(),
                 instancesDataService