Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8b08e82d authored by Yuri Lin's avatar Yuri Lin
Browse files

Add method to remove file from history based on matching file path. 

NotificationHistoryDatabase sometimes attempts to read a file that doesn't exist; this happens because scheduled history file deletions (for files >24h old) weren't correctly removing AtomicFiles from the mHistoryFiles list due to using the built in LinkedList remove() method. The new method explictly removes the first file in history matching the file path deleted, rather than attempting to use AtomicFile equality.

Test: atest NotificationHistoryDatabaseTest; manually verified that NotiHistoryDatabase stopped erroring
Fixes: 185257041
Change-Id: I56c4a3293166a84e29920afe5549171becdaa496
parent a6d011d0

services/core/java/com/android/server/notification/NotificationHistoryDatabase.java

+27 −4
Original line number Diff line number Diff line
@@ -100,7 +100,7 @@ public class NotificationHistoryDatabase { 


        IntentFilter deletionFilter = new IntentFilter(ACTION_HISTORY_DELETION);

        deletionFilter.addDataScheme(SCHEME_DELETION);

        mContext.registerReceiver(mFileCleaupReceiver, deletionFilter);

        mContext.registerReceiver(mFileCleanupReceiver, deletionFilter);

    }



    public void init() {
@@ -273,13 +273,36 @@ public class NotificationHistoryDatabase { 
        }

    }



    /**

     * Remove the first entry from the list of history files whose file matches the given file path.

     *

     * This method is necessary for anything that only has an absolute file path rather than an

     * AtomicFile object from the list of history files.

     *

     * filePath should be an absolute path.

     */

    void removeFilePathFromHistory(String filePath) {

        if (filePath == null) {

            return;

        }



        Iterator<AtomicFile> historyFileItr = mHistoryFiles.iterator();

        while (historyFileItr.hasNext()) {

            final AtomicFile af = historyFileItr.next();

            if (af != null && filePath.equals(af.getBaseFile().getAbsolutePath())) {

                historyFileItr.remove();

                return;

            }

        }

    }



    private void deleteFile(AtomicFile file) {

        if (DEBUG) {

            Slog.d(TAG, "Removed " + file.getBaseFile().getName());

        }

        file.delete();

        // TODO: delete all relevant bitmaps, once they exist

        mHistoryFiles.remove(file);

        removeFilePathFromHistory(file.getBaseFile().getAbsolutePath());

    }



    private void scheduleDeletion(File file, long creationTime, int retentionDays) {
@@ -342,7 +365,7 @@ public class NotificationHistoryDatabase { 
        }

    }



    private final BroadcastReceiver mFileCleaupReceiver = new BroadcastReceiver() {

    private final BroadcastReceiver mFileCleanupReceiver = new BroadcastReceiver() {

        @Override

        public void onReceive(Context context, Intent intent) {

            String action = intent.getAction();
@@ -358,7 +381,7 @@ public class NotificationHistoryDatabase { 
                            Slog.d(TAG, "Removed " + fileToDelete.getBaseFile().getName());

                        }

                        fileToDelete.delete();

                        mHistoryFiles.remove(fileToDelete);

                        removeFilePathFromHistory(filePath);

                    }

                } catch (Exception e) {

                    Slog.e(TAG, "Failed to delete notification history file", e);

services/tests/uiservicestests/src/com/android/server/notification/NotificationHistoryDatabaseTest.java

+35 −0
Original line number Diff line number Diff line
@@ -123,6 +123,7 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { 
        for (long i = cal.getTimeInMillis(); i >= 5; i--) {

            File file = mock(File.class);

            when(file.getName()).thenReturn(String.valueOf(i));

            when(file.getAbsolutePath()).thenReturn(String.valueOf(i));

            AtomicFile af = new AtomicFile(file);

            expectedFiles.add(af);

            mDataBase.mHistoryFiles.addLast(af);
@@ -133,6 +134,7 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { 
        for (int i = 5; i >= 0; i--) {

            File file = mock(File.class);

            when(file.getName()).thenReturn(String.valueOf(cal.getTimeInMillis() - i));

            when(file.getAbsolutePath()).thenReturn(String.valueOf(cal.getTimeInMillis() - i));

            AtomicFile af = new AtomicFile(file);

            mDataBase.mHistoryFiles.addLast(af);

        }
@@ -158,6 +160,7 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { 
        for (long i = cal.getTimeInMillis(); i >= 5; i--) {

            File file = mock(File.class);

            when(file.getName()).thenReturn(i + ".bak");

            when(file.getAbsolutePath()).thenReturn(i + ".bak");

            AtomicFile af = new AtomicFile(file);

            mDataBase.mHistoryFiles.addLast(af);

        }
@@ -415,4 +418,36 @@ public class NotificationHistoryDatabaseTest extends UiServiceTestCase { 
        assertThat(mDataBase.mBuffer).isNotEqualTo(nh);

        verify(mAlarmManager, times(1)).setExactAndAllowWhileIdle(anyInt(), anyLong(), any());

    }



    @Test

    public void testRemoveFilePathFromHistory_hasMatch() throws Exception {

        for (int i = 0; i < 5; i++) {

            AtomicFile af = mock(AtomicFile.class);

            when(af.getBaseFile()).thenReturn(new File(mRootDir, "af" + i));

            mDataBase.mHistoryFiles.addLast(af);

        }

        // Baseline size of history files

        assertThat(mDataBase.mHistoryFiles.size()).isEqualTo(5);



        // Remove only file number 3

        String filePathToRemove = new File(mRootDir, "af3").getAbsolutePath();

        mDataBase.removeFilePathFromHistory(filePathToRemove);

        assertThat(mDataBase.mHistoryFiles.size()).isEqualTo(4);

    }



    @Test

    public void testRemoveFilePathFromHistory_noMatch() throws Exception {

        for (int i = 0; i < 5; i++) {

            AtomicFile af = mock(AtomicFile.class);

            when(af.getBaseFile()).thenReturn(new File(mRootDir, "af" + i));

            mDataBase.mHistoryFiles.addLast(af);

        }

        // Baseline size of history files

        assertThat(mDataBase.mHistoryFiles.size()).isEqualTo(5);



        // Attempt to remove a filename that doesn't exist, expect nothing to break or change

        String filePathToRemove = new File(mRootDir, "af.thisfileisfake").getAbsolutePath();

        mDataBase.removeFilePathFromHistory(filePathToRemove);

        assertThat(mDataBase.mHistoryFiles.size()).isEqualTo(5);

    }

}