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

Commit 63989c98 authored by Sal Savage's avatar Sal Savage
Browse files

Make Cover Art storage more robust to filesystem issues 

We've being seeing conditions where external application storage fails
to come up. While that is a separate issue that needs to be fixed, the
way our storage handles those errors is our responsibility. Currently,
some things are crashing. This patch makes it so all file operations
check the path before proceeding. It also makes us be more aggressive in
clearing storage on stop and start so we can always try to start in a
good state if storage misbehaved previously.

Bug: b/151478590
Test: atest BluetoothInstrumentationTests
Change-Id: Iae0f29ef4b75d2c001642685c4a4b76da43bb7ed
parent a3920371

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtManager.java

+2 −0
Original line number Diff line number Diff line
@@ -77,6 +77,7 @@ public class AvrcpCoverArtManager { 
        mContext = context;

        mCoverArtStorage = new AvrcpCoverArtStorage(mContext);

        mCallback = callback;

        mCoverArtStorage.clear();

    }



    /**
@@ -123,6 +124,7 @@ public class AvrcpCoverArtManager { 
        for (BluetoothDevice device : mClients.keySet()) {

            disconnect(device);

        }

        mCoverArtStorage.clear();

    }



    /**

android/app/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtStorage.java

+49 −10
Original line number Diff line number Diff line
@@ -88,12 +88,17 @@ public class AvrcpCoverArtStorage { 


        String path = getImagePath(device, imageHandle);

        if (path == null) {

            debug("Cannot store image. Cannot provide a valid path to storage");

            error("Cannot store image. Cannot provide a valid path to storage");

            return null;

        }



        try {

            File deviceDirectory = new File(getDevicePath(device));

            String deviceDirectoryPath = getDevicePath(device);

            if (deviceDirectoryPath == null) {

                error("Cannot store image. Cannot get a valid path to per-device storage");

                return null;

            }

            File deviceDirectory = new File(deviceDirectoryPath);

            if (!deviceDirectory.exists()) {

                deviceDirectory.mkdirs();

            }
@@ -122,6 +127,10 @@ public class AvrcpCoverArtStorage { 
        debug("Removing image '" + imageHandle + "' from device " + device);

        if (device == null || imageHandle == null || "".equals(imageHandle)) return;

        String path = getImagePath(device, imageHandle);

        if (path == null) {

            error("Cannot remove image. Cannot get a valid path to storage");

            return;

        }

        File file = new File(path);

        if (!file.exists()) return;

        file.delete();
@@ -136,17 +145,26 @@ public class AvrcpCoverArtStorage { 
    public void removeImagesForDevice(BluetoothDevice device) {

        if (device == null) return;

        debug("Remove cover art for device " + device.getAddress());

        File deviceDirectory = new File(getDevicePath(device));

        File[] files = deviceDirectory.listFiles();

        if (files == null) {

            debug("No cover art files to delete");

        String deviceDirectoryPath = getDevicePath(device);

        if (deviceDirectoryPath == null) {

            error("Cannot remove images for device. Cannot get a valid path to storage");

            return;

        }

        for (int i = 0; i < files.length; i++) {

            debug("Deleted " + files[i].getAbsolutePath());

            files[i].delete();

        File deviceDirectory = new File(deviceDirectoryPath);

        deleteStorageDirectory(deviceDirectory);

    }



    /**

     * Clear the entirety of storage

     */

    public void clear() {

        String storageDirectoryPath = getStorageDirectory();

        if (storageDirectoryPath == null) {

            error("Cannot remove images, cannot get a valid path to storage. Is it mounted?");

            return;

        }

        deviceDirectory.delete();

        File storageDirectory = new File(storageDirectoryPath);

        deleteStorageDirectory(storageDirectory);

    }



    private String getStorageDirectory() {
@@ -171,6 +189,27 @@ public class AvrcpCoverArtStorage { 
        return deviceDir + "/" + imageHandle + ".png";

    }



    private void deleteStorageDirectory(File directory) {

        if (directory == null) {

            error("Cannot delete directory, file is null");

            return;

        }

        if (!directory.exists()) return;

        File[] files = directory.listFiles();

        if (files == null) {

            return;

        }

        for (int i = 0; i < files.length; i++) {

            debug("Deleting " + files[i].getAbsolutePath());

            if (files[i].isDirectory()) {

                deleteStorageDirectory(files[i]);

            } else {

                files[i].delete();

            }

        }

        directory.delete();

    }



    @Override

    public String toString() {

        String s = "CoverArtStorage:\n";

android/app/tests/unit/src/com/android/bluetooth/avrcpcontroller/AvrcpCoverArtStorageTest.java

+26 −0
Original line number Diff line number Diff line
@@ -308,4 +308,30 @@ public final class AvrcpCoverArtStorageTest { 
        Assert.assertTrue(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        Assert.assertTrue(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle2));

    }



    @Test

    public void clearStorageOneDevice_allImagesRemoved() {

        mAvrcpCoverArtStorage.addImage(mDevice1, mHandle1, mImage1);

        mAvrcpCoverArtStorage.addImage(mDevice1, mHandle2, mImage1);



        mAvrcpCoverArtStorage.clear();



        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle2));

    }



    @Test

    public void clearStorageManyDevices_allImagesRemoved() {

        mAvrcpCoverArtStorage.addImage(mDevice1, mHandle1, mImage1);

        mAvrcpCoverArtStorage.addImage(mDevice1, mHandle2, mImage1);

        mAvrcpCoverArtStorage.addImage(mDevice2, mHandle1, mImage1);

        mAvrcpCoverArtStorage.addImage(mDevice2, mHandle2, mImage1);



        mAvrcpCoverArtStorage.clear();



        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle1));

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice1, mHandle2));

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice2, mHandle1));

        Assert.assertFalse(mAvrcpCoverArtStorage.doesImageExist(mDevice2, mHandle2));

    }

}