From 96951fc50cd61289ed8dd56606085f33ce1d9c67 Mon Sep 17 00:00:00 2001 From: Julia Reynolds Date: Wed, 13 Nov 2019 16:04:33 -0500 Subject: [PATCH] Delete notif history files at the right time In addition to checking for expired files at boot, schedule jobs to delete files that expire while the device is running Test: atest Bug: 137396965 Change-Id: I006bdbc27638edbc6bc12ad7ff005776e92f1f7f --- .../NotificationHistoryDatabase.java | 117 +++++++++++++++--- .../NotificationHistoryDatabaseTest.java | 67 ++++++++-- 2 files changed, 155 insertions(+), 29 deletions(-) diff --git a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java index 99b1ef4a1d2e1..f05b2bf9711ff 100644 --- a/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java +++ b/services/core/java/com/android/server/notification/NotificationHistoryDatabase.java @@ -16,8 +16,15 @@ package com.android.server.notification; +import android.app.AlarmManager; import android.app.NotificationHistory; import android.app.NotificationHistory.HistoricalNotification; +import android.app.PendingIntent; +import android.content.BroadcastReceiver; +import android.content.Context; +import android.content.Intent; +import android.content.IntentFilter; +import android.net.Uri; import android.os.Handler; import android.util.AtomicFile; import android.util.Slog; @@ -33,11 +40,15 @@ import java.io.FileOutputStream; import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; +import java.nio.file.FileSystems; +import java.nio.file.Files; +import java.nio.file.attribute.BasicFileAttributes; import java.util.Arrays; import java.util.Calendar; import java.util.GregorianCalendar; import java.util.Iterator; import java.util.LinkedList; +import java.util.concurrent.TimeUnit; /** * Provides an interface to write and query for notification history data for a user from a Protocol @@ -52,32 +63,48 @@ public class NotificationHistoryDatabase { private static final String TAG = "NotiHistoryDatabase"; private static final boolean DEBUG = NotificationManagerService.DBG; private static final int HISTORY_RETENTION_DAYS = 2; + 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 String ACTION_HISTORY_DELETION = + NotificationHistoryDatabase.class.getSimpleName() + ".CLEANUP"; + private static final int REQUEST_CODE_DELETION = 1; + private static final String SCHEME_DELETION = "delete"; + private static final String EXTRA_KEY = "key"; + + private final Context mContext; + private final AlarmManager mAlarmManager; private final Object mLock = new Object(); private Handler mFileWriteHandler; @VisibleForTesting // List of files holding history information, sorted newest to oldest final LinkedList mHistoryFiles; - private final GregorianCalendar mCal; private final File mHistoryDir; private final File mVersionFile; // Current version of the database files schema private int mCurrentVersion; private final WriteBufferRunnable mWriteBufferRunnable; + private final FileAttrProvider mFileAttrProvider; // Object containing posted notifications that have not yet been written to disk @VisibleForTesting NotificationHistory mBuffer; - public NotificationHistoryDatabase(File dir) { + public NotificationHistoryDatabase(Context context, File dir, + FileAttrProvider fileAttrProvider) { + mContext = context; + mAlarmManager = context.getSystemService(AlarmManager.class); mCurrentVersion = DEFAULT_CURRENT_VERSION; mVersionFile = new File(dir, "version"); mHistoryDir = new File(dir, "history"); mHistoryFiles = new LinkedList<>(); - mCal = new GregorianCalendar(); mBuffer = new NotificationHistory(); mWriteBufferRunnable = new WriteBufferRunnable(); + mFileAttrProvider = fileAttrProvider; + + IntentFilter deletionFilter = new IntentFilter(ACTION_HISTORY_DELETION); + deletionFilter.addDataScheme(SCHEME_DELETION); + mContext.registerReceiver(mFileCleaupReceiver, deletionFilter); } public void init(Handler fileWriteHandler) { @@ -105,7 +132,8 @@ public class NotificationHistoryDatabase { } // Sort with newest files first - Arrays.sort(files, (lhs, rhs) -> Long.compare(rhs.lastModified(), lhs.lastModified())); + Arrays.sort(files, (lhs, rhs) -> Long.compare(mFileAttrProvider.getCreationTime(rhs), + mFileAttrProvider.getCreationTime(lhs))); for (File file : files) { mHistoryFiles.addLast(new AtomicFile(file)); @@ -197,31 +225,48 @@ public class NotificationHistoryDatabase { } /** - * Remove any files that are too old. + * Remove any files that are too old and schedule jobs to clean up the rest */ public void prune(final int retentionDays, final long currentTimeMillis) { synchronized (mLock) { - mCal.setTimeInMillis(currentTimeMillis); - mCal.add(Calendar.DATE, -1 * retentionDays); + GregorianCalendar retentionBoundary = new GregorianCalendar(); + retentionBoundary.setTimeInMillis(currentTimeMillis); + retentionBoundary.add(Calendar.DATE, -1 * retentionDays); - while (!mHistoryFiles.isEmpty()) { - final AtomicFile currentOldestFile = mHistoryFiles.getLast(); - final long age = currentTimeMillis - - currentOldestFile.getBaseFile().lastModified(); - if (age > mCal.getTimeInMillis()) { + for (int i = mHistoryFiles.size() - 1; i >= 0; i--) { + final AtomicFile currentOldestFile = mHistoryFiles.get(i); + final long creationTime = + mFileAttrProvider.getCreationTime(currentOldestFile.getBaseFile()); + if (creationTime <= retentionBoundary.getTimeInMillis()) { if (DEBUG) { Slog.d(TAG, "Removed " + currentOldestFile.getBaseFile().getName()); } currentOldestFile.delete(); mHistoryFiles.removeLast(); } else { - // all remaining files are newer than the cut off - return; + // all remaining files are newer than the cut off; schedule jobs to delete + final long deletionTime = creationTime + (retentionDays * HISTORY_RETENTION_MS); + scheduleDeletion(currentOldestFile.getBaseFile(), deletionTime); } } } } + void scheduleDeletion(File file, long deletionTime) { + if (DEBUG) { + Slog.d(TAG, "Scheduling deletion for " + file.getName() + " at " + deletionTime); + } + final PendingIntent pi = PendingIntent.getBroadcast(mContext, + REQUEST_CODE_DELETION, + new Intent(ACTION_HISTORY_DELETION) + .setData(new Uri.Builder().scheme(SCHEME_DELETION) + .appendPath(file.getAbsolutePath()).build()) + .addFlags(Intent.FLAG_RECEIVER_FOREGROUND) + .putExtra(EXTRA_KEY, file.getAbsolutePath()), + PendingIntent.FLAG_UPDATE_CURRENT); + mAlarmManager.setExactAndAllowWhileIdle(AlarmManager.RTC_WAKEUP, deletionTime, pi); + } + private void writeLocked(AtomicFile file, NotificationHistory notifications) throws IOException { FileOutputStream fos = file.startWrite(); @@ -245,6 +290,25 @@ public class NotificationHistoryDatabase { } } + private final BroadcastReceiver mFileCleaupReceiver = new BroadcastReceiver() { + @Override + public void onReceive(Context context, Intent intent) { + String action = intent.getAction(); + if (action == null) { + return; + } + if (ACTION_HISTORY_DELETION.equals(action)) { + try { + final String filePath = intent.getStringExtra(EXTRA_KEY); + AtomicFile fileToDelete = new AtomicFile(new File(filePath)); + fileToDelete.delete(); + } catch (Exception e) { + Slog.e(TAG, "Failed to delete notification history file", e); + } + } + } + }; + private final class WriteBufferRunnable implements Runnable { @Override public void run() { @@ -277,10 +341,7 @@ public class NotificationHistoryDatabase { // Remove packageName entries from pending history mBuffer.removeNotificationsFromWrite(mPkg); - // Remove packageName entries from files on disk, and rewrite them to disk - // Since we sort by modified date, we have to update the files oldest to newest to - // maintain the original ordering - Iterator historyFileItr = mHistoryFiles.descendingIterator(); + Iterator historyFileItr = mHistoryFiles.iterator(); while (historyFileItr.hasNext()) { final AtomicFile af = historyFileItr.next(); try { @@ -297,4 +358,24 @@ public class NotificationHistoryDatabase { } } } + + public static final class NotificationHistoryFileAttrProvider implements + NotificationHistoryDatabase.FileAttrProvider { + final static String TAG = "NotifHistoryFileDate"; + + public long getCreationTime(File file) { + try { + BasicFileAttributes attr = Files.readAttributes(FileSystems.getDefault().getPath( + file.getAbsolutePath()), BasicFileAttributes.class); + return attr.creationTime().to(TimeUnit.MILLISECONDS); + } catch (Exception e) { + Slog.w(TAG, "Cannot read creation data for file; using file name"); + return Long.valueOf(file.getName()); + } + } + } + + interface FileAttrProvider { + long getCreationTime(File file); + } } 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 bcff2f81f8056..608625f9fd10c 100644 --- a/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java +++ b/services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java @@ -18,6 +18,7 @@ package com.android.server.notification; import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -25,7 +26,9 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import android.app.AlarmManager; import android.app.NotificationHistory.HistoricalNotification; +import android.content.Context; import android.graphics.drawable.Icon; import android.os.Handler; import android.util.AtomicFile; @@ -42,8 +45,17 @@ import org.mockito.Mock; import org.mockito.MockitoAnnotations; import java.io.File; +import java.nio.file.FileSystem; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.FileTime; +import java.util.ArrayList; import java.util.Calendar; import java.util.GregorianCalendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; @RunWith(AndroidJUnit4.class) public class NotificationHistoryDatabaseTest extends UiServiceTestCase { @@ -51,6 +63,11 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { File mRootDir; @Mock Handler mFileWriteHandler; + @Mock + Context mContext; + @Mock + AlarmManager mAlarmManager; + TestFileAttrProvider mFileAttrProvider; NotificationHistoryDatabase mDataBase; @@ -85,36 +102,56 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { @Before public void setUp() { MockitoAnnotations.initMocks(this); + when(mContext.getSystemService(AlarmManager.class)).thenReturn(mAlarmManager); + when(mContext.getUser()).thenReturn(getContext().getUser()); + when(mContext.getPackageName()).thenReturn(getContext().getPackageName()); + mFileAttrProvider = new TestFileAttrProvider(); mRootDir = new File(mContext.getFilesDir(), "NotificationHistoryDatabaseTest"); - mDataBase = new NotificationHistoryDatabase(mRootDir); + mDataBase = new NotificationHistoryDatabase(mContext, mRootDir, mFileAttrProvider); mDataBase.init(mFileWriteHandler); } @Test - public void testPrune() { + public void testDeletionReceiver() { + verify(mContext, times(1)).registerReceiver(any(), any()); + } + + @Test + public void testPrune() throws Exception { + GregorianCalendar cal = new GregorianCalendar(); + cal.setTimeInMillis(10); int retainDays = 1; - for (long i = 10; i >= 5; i--) { + + List expectedFiles = new ArrayList<>(); + + // add 5 files with a creation date of "today" + for (long i = cal.getTimeInMillis(); i >= 5; i--) { File file = mock(File.class); - when(file.lastModified()).thenReturn(i); + mFileAttrProvider.creationDates.put(file, i); AtomicFile af = new AtomicFile(file); + expectedFiles.add(af); mDataBase.mHistoryFiles.addLast(af); } - GregorianCalendar cal = new GregorianCalendar(); - cal.setTimeInMillis(5); + cal.add(Calendar.DATE, -1 * retainDays); + // Add 5 more files more than retainDays old for (int i = 5; i >= 0; i--) { File file = mock(File.class); - when(file.lastModified()).thenReturn(cal.getTimeInMillis() - i); + mFileAttrProvider.creationDates.put(file, cal.getTimeInMillis() - i); AtomicFile af = new AtomicFile(file); mDataBase.mHistoryFiles.addLast(af); } - mDataBase.prune(retainDays, 10); - for (AtomicFile file : mDataBase.mHistoryFiles) { - assertThat(file.getBaseFile().lastModified() > 0); - } + // back to today; trim everything a day + old + cal.add(Calendar.DATE, 1 * retainDays); + mDataBase.prune(retainDays, cal.getTimeInMillis()); + + assertThat(mDataBase.mHistoryFiles).containsExactlyElementsIn(expectedFiles); + + verify(mAlarmManager, times(6)).setExactAndAllowWhileIdle(anyInt(), anyLong(), any()); + } @Test @@ -181,4 +218,12 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { verify(af2, never()).openRead(); } + private class TestFileAttrProvider implements NotificationHistoryDatabase.FileAttrProvider { + public Map creationDates = new HashMap<>(); + + @Override + public long getCreationTime(File file) { + return creationDates.get(file); + } + } }