Codebase review: translations, share/timeout fixes, Maps open, version 1.3.11

app/build.gradle

@@ -15,8 +15,8 @@ android {
         applicationId "com.doubleangels.redact"
         minSdk 31
         targetSdk 37
-        versionCode 20
-        versionName "1.3.10"
+        versionCode 21
+        versionName "1.3.11"
 
         def sentryRelease = "${applicationId}@${versionName}+${versionCode}"
         buildConfigField "String", "SENTRY_RELEASE", "\"${sentryRelease}\""
@@ -45,6 +45,12 @@ android {
     testOptions {
         unitTests {
             includeAndroidResources = true
+            all {
+                testLogging {
+                    events "failed"
+                    exceptionFormat "full"
+                }
+            }
         }
     }
 

app/src/main/AndroidManifest.xml

@@ -22,6 +22,7 @@
         android:name=".RedactApplication"
         android:allowBackup="false"
         android:dataExtractionRules="@xml/data_extraction_rules"
+        android:enableOnBackInvokedCallback="true"
         android:icon="@mipmap/ic_launcher"
         android:label="@string/app_name"
         android:localeConfig="@xml/locales_config"

app/src/main/java/com/doubleangels/redact/AppPreferences.java

@@ -29,6 +29,7 @@ public final class AppPreferences {
     private static final String KEY_STRICT_CLEAN = "strict_clean";
     private static final String KEY_VIDEO_FALLBACK_COPY = "video_fallback_copy";
     private static final String KEY_INITIAL_PERMISSIONS_PROMPTED = "initial_permissions_prompted";
+    private static final String KEY_MAPS_LOCATION_CONSENT_GIVEN = "maps_location_consent_given";
 
     /** High quality — matches legacy defaults. */
     public static final int QUALITY_PRESET_HIGH = 0;
@@ -250,4 +251,18 @@ public static boolean hasCompletedInitialPermissionsPrompt(@NonNull Context cont
     public static void setInitialPermissionsPromptCompleted(@NonNull Context context) {
         prefs(context).edit().putBoolean(KEY_INITIAL_PERMISSIONS_PROMPTED, true).apply();
     }
+
+    /**
+     * Whether the user has already agreed, once, that opening a Scan coordinate in the device's
+     * maps app is okay. That action hands the GPS coordinates to a third-party app, which may
+     * in turn send them off-device (tile/geocoding requests), unlike every other Scan action,
+     * which stays entirely on-device. False until the user explicitly consents.
+     */
+    public static boolean hasConsentedToOpenLocationInMaps(@NonNull Context context) {
+        return prefs(context).getBoolean(KEY_MAPS_LOCATION_CONSENT_GIVEN, false);
+    }
+
+    public static void setConsentedToOpenLocationInMaps(@NonNull Context context, boolean consented) {
+        prefs(context).edit().putBoolean(KEY_MAPS_LOCATION_CONSENT_GIVEN, consented).apply();
+    }
 }

app/src/main/java/com/doubleangels/redact/CacheCleanup.java

@@ -3,11 +3,13 @@
 import android.content.Context;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.VisibleForTesting;
 
 import com.doubleangels.redact.media.SecureDelete;
 
 import java.io.File;
 import java.util.Locale;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * Utilities for measuring and clearing temporary processing files in app cache.
@@ -17,6 +19,14 @@ public final class CacheCleanup {
     /** Default age for startup stale-temp cleanup (24 hours). */
     public static final long DEFAULT_STALE_TEMP_MAX_AGE_MS = 24L * 60L * 60L * 1000L;
 
+    /**
+     * Ensures {@link #scheduleAutoCleanupIfEnabled} runs at most once per process, however many
+     * entry points call it (the Application on cold start, MainActivity on open).
+     */
+    private static final AtomicBoolean autoCleanupScheduledThisProcess = new AtomicBoolean(false);
+
+    private static volatile Thread autoCleanupThread;
+
     private static final String PROCESSED_SUBDIR = "processed";
     private static final String[] TEMP_PREFIXES = {
             "temp_", "verify_", "vid_transform_", "vid_transmux_", "inbound_"
@@ -68,6 +78,55 @@ public static int clearAllTempFiles(@NonNull Context context) {
         return clearStaleTempFiles(context, MIN_DELETE_AGE_MS);
     }
 
+    /**
+     * Runs the stale-temp sweep in the background, at most once per process, as soon as any
+     * entry point reaches this call -- {@code RedactApplication.onCreate} on cold start, or
+     * {@code MainActivity.onCreate} on open, whichever comes first.
+     *
+     * <p>Previously this sweep only ran from {@code MainActivity}, so a user who only ever used
+     * the share-sheet entry point ({@code ShareHandlerActivity}) without opening the main app
+     * could leave unredacted source snapshots sitting in cache well past the 24-hour cutoff --
+     * bounded only by the next time they happened to launch the app, or uninstalled it.
+     */
+    public static void scheduleAutoCleanupIfEnabled(@NonNull Context context) {
+        if (!shouldRunAutoCleanup(context)) {
+            return;
+        }
+        Context appContext = context.getApplicationContext();
+        // Guarded to once per process, so a one-shot thread beats a never-released executor.
+        Thread thread = new Thread(() -> performAutoCleanup(appContext), "redact-auto-cleanup");
+        autoCleanupThread = thread;
+        thread.start();
+    }
+
+    @VisibleForTesting
+    static boolean shouldRunAutoCleanup(@NonNull Context context) {
+        if (!AppPreferences.isAutoClearTempFiles(context)
+                || ShareHandlerActivity.isShareProcessingActive()) {
+            return false;
+        }
+        return autoCleanupScheduledThisProcess.compareAndSet(false, true);
+    }
+
+    @VisibleForTesting
+    static void performAutoCleanup(@NonNull Context appContext) {
+        clearStaleTempFiles(appContext, DEFAULT_STALE_TEMP_MAX_AGE_MS);
+    }
+
+    @VisibleForTesting
+    public static void resetAutoCleanupStateForTests() {
+        // Let any sweep started by Application.onCreate finish so it can't race the test's files.
+        Thread thread = autoCleanupThread;
+        if (thread != null) {
+            try {
+                thread.join(5000);
+            } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+            }
+        }
+        autoCleanupScheduledThisProcess.set(false);
+    }
+
     @NonNull
     public static String formatSize(long bytes) {
         if (bytes < 1024) {

app/src/main/java/com/doubleangels/redact/MainActivity.java

@@ -33,17 +33,10 @@ public class MainActivity extends AppCompatActivity {
     private static final String TAG_SETTINGS = "settings";
     private static final String KEY_SELECTED_TAB = "selected_tab";
 
-    @Nullable
-    private java.util.concurrent.ExecutorService cacheClearExecutor;
-
     @Override
     protected void onDestroy() {
         com.doubleangels.redact.permission.PermissionManager.setInitialFlowCompletedCallback(null);
         com.doubleangels.redact.permission.PermissionManager.clearRuntimePermissionRequestOnDestroy();
-        if (cacheClearExecutor != null) {
-            cacheClearExecutor.shutdown();
-            cacheClearExecutor = null;
-        }
         super.onDestroy();
     }
 
@@ -67,14 +60,12 @@ protected void onCreate(Bundle savedInstanceState) {
 
             SentryManager.logEvent("lifecycle", "MainActivity created");
 
-            if (AppPreferences.isAutoClearTempFiles(this)
-                    && !ShareHandlerActivity.isShareProcessingActive()
-                    && !com.doubleangels.redact.ui.MainViewModel.isAnyProcessing(this)) {
-                cacheClearExecutor = java.util.concurrent.Executors.newSingleThreadExecutor();
-                cacheClearExecutor.execute(
-                        () -> com.doubleangels.redact.CacheCleanup.clearStaleTempFiles(
-                                getApplicationContext(),
-                                com.doubleangels.redact.CacheCleanup.DEFAULT_STALE_TEMP_MAX_AGE_MS));
+            // RedactApplication.onCreate already runs this sweep once per process; this call
+            // only matters when MainActivity is recreated without a fresh process (e.g. after a
+            // configuration change) and isAnyProcessing requires a live Activity to check, so it
+            // can't move into the Application-level call.
+            if (!com.doubleangels.redact.ui.MainViewModel.isAnyProcessing(this)) {
+                com.doubleangels.redact.CacheCleanup.scheduleAutoCleanupIfEnabled(this);
             }
 
             if (savedInstanceState == null) {

app/src/main/java/com/doubleangels/redact/RedactApplication.java

@@ -26,6 +26,12 @@ public void onCreate() {
         SentryManager.init(this);
         SentryInitializer.initializeIfNeeded(this);
         LocalNotifications.ensureChannels(this);
+
+        // Runs on every process start, including a cold start into ShareHandlerActivity via the
+        // share sheet -- not just when MainActivity happens to be opened -- so stale temp copies
+        // of source media (unredacted EXIF/GPS included) don't linger past the cutoff just
+        // because the user never opens the main app.
+        CacheCleanup.scheduleAutoCleanupIfEnabled(this);
     }
 
     @NonNull

app/src/main/java/com/doubleangels/redact/ScanFragment.java

@@ -1,8 +1,10 @@
 package com.doubleangels.redact;
 
+import android.content.ActivityNotFoundException;
 import android.content.ClipData;
 import android.content.ClipboardManager;
 import android.content.Context;
+import android.content.Intent;
 import android.net.Uri;
 import android.os.Bundle;
 import android.util.Pair;
@@ -34,6 +36,8 @@
 import com.doubleangels.redact.ui.ScanViewModel;
 import com.google.android.material.button.MaterialButton;
 import com.google.android.material.card.MaterialCardView;
+import com.google.android.material.dialog.MaterialAlertDialogBuilder;
+import com.google.android.material.divider.MaterialDividerItemDecoration;
 import com.doubleangels.redact.sentry.SentryManager;
 
 import java.util.ArrayList;
@@ -144,6 +148,13 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
         scanMetadataAdapter = new ScanMetadataAdapter();
         metadataItemsRecycler.setLayoutManager(new LinearLayoutManager(requireContext()));
         metadataItemsRecycler.setAdapter(scanMetadataAdapter);
+        MaterialDividerItemDecoration metadataDivider = new MaterialDividerItemDecoration(
+                requireContext(), LinearLayoutManager.VERTICAL);
+        int dividerInset = getResources().getDimensionPixelSize(R.dimen.scan_metadata_row_horizontal_padding);
+        metadataDivider.setDividerInsetStart(dividerInset);
+        metadataDivider.setDividerInsetEnd(dividerInset);
+        metadataDivider.setLastItemDecorated(false);
+        metadataItemsRecycler.addItemDecoration(metadataDivider);
         metadataItemsRecycler.setNestedScrollingEnabled(false);
         metadataFooter = view.findViewById(R.id.metadataFooter);
         metadataCard = view.findViewById(R.id.metadataCard);
@@ -498,9 +509,10 @@ private void updateScanActionCards(List<Pair<String, String>> allRows) {
         boolean hasCoords = !Double.isNaN(lastMapLatitude) && !Double.isNaN(lastMapLongitude)
                 && MetadataDisplayer.isUsableMapCoordinate(lastMapLatitude, lastMapLongitude);
         if (hasCoords) {
-            String coordsLabel = String.format(Locale.US, "%.7f, %.7f", lastMapLatitude, lastMapLongitude);
-            addScanActionCard(R.drawable.ic_map_24, getString(R.string.scan_copy_coordinates),
-                    v -> copyPlainTextToClipboard(coordsLabel));
+            double latForMaps = lastMapLatitude;
+            double lonForMaps = lastMapLongitude;
+            addScanActionCard(R.drawable.ic_map_24, getString(R.string.scan_open_coordinates_in_maps),
+                    v -> onOpenCoordinatesInMaps(latForMaps, lonForMaps));
             added = true;
         }
 
@@ -586,6 +598,38 @@ private void copyPlainTextToClipboard(@NonNull String text) {
         Toast.makeText(requireContext(), R.string.scan_copied_to_clipboard, Toast.LENGTH_SHORT).show();
     }
 
+    /**
+     * Opening a coordinate in the maps app hands the GPS location to a separate, third-party
+     * app, unlike every other Scan action, which stays entirely on-device, so the first use
+     * requires explicit consent. Once given, it's remembered and this goes straight to Maps.
+     */
+    private void onOpenCoordinatesInMaps(double latitude, double longitude) {
+        if (AppPreferences.hasConsentedToOpenLocationInMaps(requireContext())) {
+            openCoordinatesInMaps(latitude, longitude);
+            return;
+        }
+        new MaterialAlertDialogBuilder(requireContext())
+                .setTitle(R.string.scan_maps_consent_title)
+                .setMessage(R.string.scan_maps_consent_message)
+                .setPositiveButton(R.string.scan_maps_consent_continue, (dialog, which) -> {
+                    AppPreferences.setConsentedToOpenLocationInMaps(requireContext(), true);
+                    openCoordinatesInMaps(latitude, longitude);
+                })
+                .setNegativeButton(R.string.button_cancel, null)
+                .show();
+    }
+
+    private void openCoordinatesInMaps(double latitude, double longitude) {
+        String coords = String.format(Locale.US, "%.7f,%.7f", latitude, longitude);
+        Uri geoUri = Uri.parse("geo:" + coords + "?q=" + coords);
+        Intent intent = new Intent(Intent.ACTION_VIEW, geoUri);
+        try {
+            startActivity(intent);
+        } catch (ActivityNotFoundException e) {
+            Toast.makeText(requireContext(), R.string.scan_maps_not_available, Toast.LENGTH_SHORT).show();
+        }
+    }
+
     private void openInCleanTab(@NonNull MediaItem item) {
         MainViewModel viewModel = new ViewModelProvider(requireActivity()).get(MainViewModel.class);
         viewModel.setSelectedItems(List.of(item));