From 7d0d156c5958f84d9fa4d1ed51416c1033e28bc2 Mon Sep 17 00:00:00 2001
From: Julia Reynolds <juliacr@google.com>
Date: Tue, 16 Jun 2020 13:06:31 -0400
Subject: [PATCH] Clean up temp AtomicFile files

Test: atest
Fixes: 152736887
Change-Id: Ibf0accbdf1771f6cd5fccc3d9e3a4dd99f5f5377
---
 .../NotificationHistoryDatabase.java          | 43 +++++++++++--------
 .../NotificationHistoryDatabaseTest.java      |  4 +-
 2 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java
index 0ec4b39d89608..18da33ce19e07 100644
--- a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java
+++ b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java
@@ -65,6 +65,7 @@ public class NotificationHistoryDatabase {
     private static final int HISTORY_RETENTION_DAYS = 1;
     private static final int HISTORY_RETENTION_MS = 24 * 60 * 60 * 1000;
     private static final long WRITE_BUFFER_INTERVAL_MS = 1000 * 60 * 20;
+    private static final long INVALID_FILE_TIME_MS = -1;
 
     private static final String ACTION_HISTORY_DELETION =
             NotificationHistoryDatabase.class.getSimpleName() + ".CLEANUP";
@@ -130,8 +131,8 @@ public class NotificationHistoryDatabase {
         }
 
         // Sort with newest files first
-        Arrays.sort(files, (lhs, rhs) -> Long.compare(Long.parseLong(rhs.getName()),
-                Long.parseLong(lhs.getName())));
+        Arrays.sort(files, (lhs, rhs) -> Long.compare(safeParseLong(rhs.getName()),
+                safeParseLong(lhs.getName())));
 
         for (File file : files) {
             mHistoryFiles.addLast(new AtomicFile(file));
@@ -252,22 +253,19 @@ public class NotificationHistoryDatabase {
 
             for (int i = mHistoryFiles.size() - 1; i >= 0; i--) {
                 final AtomicFile currentOldestFile = mHistoryFiles.get(i);
-                try {
-                    final long creationTime = Long.parseLong(
-                            currentOldestFile.getBaseFile().getName());
-                    if (DEBUG) {
-                        Slog.d(TAG, "File " + currentOldestFile.getBaseFile().getName()
-                                + " created on " + creationTime);
-                    }
-                    if (creationTime <= retentionBoundary.getTimeInMillis()) {
-                        deleteFile(currentOldestFile);
-                    } else {
-                        // all remaining files are newer than the cut off; schedule jobs to delete
-                        scheduleDeletion(
-                                currentOldestFile.getBaseFile(), creationTime, retentionDays);
-                    }
-                } catch (NumberFormatException e) {
+                final long creationTime = safeParseLong(
+                        currentOldestFile.getBaseFile().getName());
+                if (DEBUG) {
+                    Slog.d(TAG, "File " + currentOldestFile.getBaseFile().getName()
+                            + " created on " + creationTime);
+                }
+
+                if (creationTime <= retentionBoundary.getTimeInMillis()) {
                     deleteFile(currentOldestFile);
+                } else {
+                    // all remaining files are newer than the cut off; schedule jobs to delete
+                    scheduleDeletion(
+                            currentOldestFile.getBaseFile(), creationTime, retentionDays);
                 }
             }
         }
@@ -331,6 +329,17 @@ public class NotificationHistoryDatabase {
         }
     }
 
+    private static long safeParseLong(String fileName) {
+        // AtomicFile will create copies of the numeric files with ".new" and ".bak"
+        // over the course of its processing. If these files still exist on boot we need to clean
+        // them up
+        try {
+            return Long.parseLong(fileName);
+        } catch (NumberFormatException e) {
+            return INVALID_FILE_TIME_MS;
+        }
+    }
+
     private final BroadcastReceiver mFileCleaupReceiver = new BroadcastReceiver() {
         @Override
         public void onReceive(Context context, Intent intent) {
diff --git a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java
index 6991c18ceb82d..a2d987fb0a8df 100644
--- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java
+++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java
@@ -149,7 +149,7 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase {
     }
 
     @Test
-    public void testPrune_badFileName() {
+    public void testPrune_badFileName_noCrash() {
         GregorianCalendar cal = new GregorianCalendar();
         cal.setTimeInMillis(10);
         int retainDays = 1;
@@ -159,7 +159,7 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase {
         // add 5 files with a creation date of "today", but the file names are bad
         for (long i = cal.getTimeInMillis(); i >= 5; i--) {
             File file = mock(File.class);
-            when(file.getName()).thenReturn(i + ".txt");
+            when(file.getName()).thenReturn(i + ".bak");
             AtomicFile af = new AtomicFile(file);
             mDataBase.mHistoryFiles.addLast(af);
         }