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.

Sign up for GitHub

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

Fixed memory leaks in form-filling #6275

Merged
merged 2 commits into from
Jul 22, 2024
Merged

Fixed memory leaks in form-filling #6275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
184247b
Fixed passing the SavepointsRepositoryProvider to viewModelFactory
grzesiek2010 Jul 18, 2024
1dad86a
Fixed passing dependencies to BackgroundLocationHelper
grzesiek2010 Jul 18, 2024
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
8 changes: 4 additions & 4 deletions collect_app/src/main/java/org/odk/collect/android/activities/FormEntryViewModelFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ class FormEntryViewModelFactory(
fusedLocationClient,
BackgroundLocationHelper(
permissionsProvider,
settingsProvider.getUnprotectedSettings()
) {
formSessionRepository.get(sessionId).value?.formController
}
settingsProvider.getUnprotectedSettings(),
formSessionRepository,
sessionId
)
)

BackgroundLocationViewModel(locationManager)
Expand Down
2 changes: 1 addition & 1 deletion collect_app/src/main/java/org/odk/collect/android/activities/FormFillingActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ public void onCreate(Bundle savedInstanceState) {
autoSendSettingsProvider,
formsRepositoryProvider,
instancesRepositoryProvider,
new SavepointsRepositoryProvider(this, storagePathProvider),
savepointsRepositoryProvider,
new QRCodeCreatorImpl(),
new HtmlPrinter(),
instancesDataService
Expand Down
37 changes: 26 additions & 11 deletions ...n/java/org/odk/collect/android/formentry/backgroundlocation/BackgroundLocationHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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() {
Expand All @@ -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;
}

/**
Expand All @@ -62,7 +71,7 @@ boolean isCurrentFormSet() {
* Precondition: the global form controller has been initialized.
*/
boolean currentFormCollectsBackgroundLocation() {
return formControllerProvider.get().currentFormCollectsBackgroundLocation();
return getFormController().currentFormCollectsBackgroundLocation();
}

/**
Expand All @@ -72,7 +81,7 @@ boolean currentFormCollectsBackgroundLocation() {
* Precondition: the global form controller has been initialized.
*/
boolean currentFormAuditsLocation() {
return formControllerProvider.get().currentFormAuditsLocation();
return getFormController().currentFormAuditsLocation();
}

/**
Expand All @@ -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;
}

/**
Expand All @@ -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());
}

/**
Expand All @@ -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();
}
}
5 changes: 2 additions & 3 deletions collect_app/src/main/java/org/odk/collect/android/formhierarchy/FormHierarchyActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -232,7 +231,7 @@ public void onCreate(Bundle savedInstanceState) {
autoSendSettingsProvider,
formsRepositoryProvider,
instancesRepositoryProvider,
new SavepointsRepositoryProvider(this, storagePathProvider),
savepointsRepositoryProvider,
new QRCodeCreatorImpl(),
new HtmlPrinter(),
instancesDataService
Expand Down