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

Commit e037024f authored by zhidou's avatar zhidou
Browse files

Throw exceptions instead of returning null

This commit changes

1. load methods will throw FileSystemNotFoundException, if new storeage
   is not available on the device
1. load(container, package, storageFileProvider) will throw exception if
   container is null

Test: atest aconfig_storage_package aconfig_storage_file.test.java
Bug: 367765164
Flag: EXEMPT refactor
Change-Id: I81de851793f56691fb41caad03cba491fe7527c0
parent 52b9b468
Loading
Loading
Loading
Loading
+38 −41
Original line number Diff line number Diff line
@@ -19,8 +19,6 @@ package android.aconfig.storage;
import android.os.StrictMode;

import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;

/** @hide */
public class AconfigPackageImpl {
@@ -29,25 +27,29 @@ public class AconfigPackageImpl {
    private PackageTable.Node mPNode;

    /** @hide */
    public static AconfigPackageImpl load(String packageName, StorageFileProvider fileProvider) {
        AconfigPackageImpl aPackage = new AconfigPackageImpl();
        if (!aPackage.init(null, packageName, fileProvider)) {
            return null;
        }
        return aPackage;
    public static final int ERROR_NEW_STORAGE_SYSTEM_NOT_FOUND = 1;

    /** @hide */
    public static final int ERROR_PACKAGE_NOT_FOUND = 2;

    /** @hide */
    public static final int ERROR_CONTAINER_NOT_FOUND = 3;

    /** @hide */
    public AconfigPackageImpl() {}

    /** @hide */
    public int load(String packageName, StorageFileProvider fileProvider) {
        return init(null, packageName, fileProvider);
    }

    /** @hide */
    public static AconfigPackageImpl load(
            String container, String packageName, StorageFileProvider fileProvider) {
    public int load(String container, String packageName, StorageFileProvider fileProvider) {
        if (container == null) {
            return null;
            return ERROR_CONTAINER_NOT_FOUND;
        }
        AconfigPackageImpl aPackage = new AconfigPackageImpl();
        if (!aPackage.init(container, packageName, fileProvider)) {
            return null;
        }
        return aPackage;

        return init(container, packageName, fileProvider);
    }

    /** @hide */
@@ -74,61 +76,56 @@ public class AconfigPackageImpl {
        return mPNode.hasPackageFingerprint();
    }

    private boolean init(
            String containerName, String packageName, StorageFileProvider fileProvider) {
    private int init(String containerName, String packageName, StorageFileProvider fileProvider) {
        StrictMode.ThreadPolicy oldPolicy = StrictMode.allowThreadDiskReads();
        String container = containerName;
        try {
            // for devices don't have new storage directly return
            if (!fileProvider.containerFileExists(null)) {
                return false;
                return ERROR_NEW_STORAGE_SYSTEM_NOT_FOUND;
            }
            PackageTable.Node pNode = null;

            if (container == null) {
                PackageTable pTable = null;
                // check if device has flag files on the system partition
                // if the device has then search system partition first
                // Check if the device has flag files on the system partition.
                // If the device does, search the system partition first.
                container = "system";
                if (fileProvider.containerFileExists(container)) {
                    pTable = fileProvider.getPackageTable(container);
                    pNode = pTable.get(packageName);
                }
                List<Path> mapFiles = new ArrayList<>();
                if (pNode == null) {
                    mapFiles = fileProvider.listPackageMapFiles();
                    if (mapFiles.isEmpty()) return false;
                    pNode = fileProvider.getPackageTable(container).get(packageName);
                }

                for (Path p : mapFiles) {
                    pTable = StorageFileProvider.getPackageTable(p);
                if (pNode == null) {
                    // Search all package map files if not found in the system partition.
                    for (Path p : fileProvider.listPackageMapFiles()) {
                        PackageTable pTable = StorageFileProvider.getPackageTable(p);
                        pNode = pTable.get(packageName);
                        if (pNode != null) {
                            container = pTable.getHeader().getContainer();
                            break;
                        }
                    }
                }
            } else {
                if (!fileProvider.containerFileExists(container)) {
                    return ERROR_CONTAINER_NOT_FOUND;
                }
                pNode = fileProvider.getPackageTable(container).get(packageName);
            }

            if (pNode == null) {
                // for the case package is not found in all container, return instead of throwing
                // error
                return false;
                return ERROR_PACKAGE_NOT_FOUND;
            }

            mFlagTable = fileProvider.getFlagTable(container);
            mFlagValueList = fileProvider.getFlagValueList(container);
            mPNode = pNode;
        } catch (Exception e) {
            throw new AconfigStorageException(
                    String.format(
                            "cannot load package %s, from container %s", packageName, container),
                    e);
            throw e;
        } finally {
            StrictMode.setThreadPolicy(oldPolicy);
        }
        return true;
        return 0;
    }
}
+1 −1
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ public class StorageFileProvider {
    /** @hide */
    public boolean containerFileExists(String container) {
        if (container == null) {
            return Files.exists(Paths.get(DEFAULT_MAP_PATH));
            return Files.exists(Paths.get(mMapPath));
        }
        return Files.exists(Paths.get(mMapPath, container + PMAP_FILE_EXT));
    }
+47 −20
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@

package android.aconfig.storage.test;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
@@ -25,6 +26,7 @@ import android.aconfig.storage.AconfigPackageImpl;
import android.aconfig.storage.AconfigStorageException;
import android.aconfig.storage.StorageFileProvider;

import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -32,33 +34,62 @@ import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
public class AconfigPackageImplTest {

    private StorageFileProvider pr;

    @Before
    public void setup() {
        pr = new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
    }

    @Test
    public void testLoad_onlyPackageName() throws Exception {
        StorageFileProvider pr =
                new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
        AconfigPackageImpl p = AconfigPackageImpl.load("com.android.aconfig.storage.test_1", pr);
        AconfigPackageImpl p = new AconfigPackageImpl();
        p.load("com.android.aconfig.storage.test_1", pr);
        assertNotNull(p);
    }

    @Test
    public void testLoad_groupNameFingerprint() throws Exception {
        StorageFileProvider pr =
                new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
        AconfigPackageImpl p =
                AconfigPackageImpl.load("mockup", "com.android.aconfig.storage.test_1", pr);
        AconfigPackageImpl p = new AconfigPackageImpl();
        p.load("mockup", "com.android.aconfig.storage.test_1", pr);
        assertNotNull(p);
    }

    @Test
    public void testLoad_error() throws Exception {
        AconfigPackageImpl p = new AconfigPackageImpl();
        // cannot find package
        assertEquals(
                AconfigPackageImpl.ERROR_PACKAGE_NOT_FOUND,
                p.load("mockup", "com.android.aconfig.storage.test_10", pr));
        // cannot find package
        assertEquals(
                AconfigPackageImpl.ERROR_PACKAGE_NOT_FOUND,
                p.load("com.android.aconfig.storage.test_10", pr));
        // cannot find container
        assertEquals(
                AconfigPackageImpl.ERROR_CONTAINER_NOT_FOUND,
                p.load(null, "com.android.aconfig.storage.test_1", pr));
        assertEquals(
                AconfigPackageImpl.ERROR_CONTAINER_NOT_FOUND,
                p.load("test", "com.android.aconfig.storage.test_1", pr));

        // new storage doesn't exist
        pr = new StorageFileProvider("fake/path/", "fake/path/");
        assertEquals(
                AconfigPackageImpl.ERROR_NEW_STORAGE_SYSTEM_NOT_FOUND, p.load("fake_package", pr));

        // file read issue
        pr = new StorageFileProvider(TestDataUtils.TESTDATA_PATH, "fake/path/");
        assertThrows(
                AconfigStorageException.class,
                () -> AconfigPackageImpl.load("test", "com.android.aconfig.storage.test_1", pr));
                () -> p.load("mockup", "com.android.aconfig.storage.test_1", pr));
    }

    @Test
    public void testGetBooleanFlagValue_flagName() throws Exception {
        StorageFileProvider pr =
                new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
        AconfigPackageImpl p =
                AconfigPackageImpl.load("mockup", "com.android.aconfig.storage.test_1", pr);
        AconfigPackageImpl p = new AconfigPackageImpl();
        p.load("mockup", "com.android.aconfig.storage.test_1", pr);
        assertFalse(p.getBooleanFlagValue("disabled_rw", true));
        assertTrue(p.getBooleanFlagValue("enabled_ro", false));
        assertTrue(p.getBooleanFlagValue("enabled_rw", false));
@@ -67,10 +98,8 @@ public class AconfigPackageImplTest {

    @Test
    public void testGetBooleanFlagValue_index() throws Exception {
        StorageFileProvider pr =
                new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
        AconfigPackageImpl p =
                AconfigPackageImpl.load("mockup", "com.android.aconfig.storage.test_1", pr);
        AconfigPackageImpl p = new AconfigPackageImpl();
        p.load("mockup", "com.android.aconfig.storage.test_1", pr);
        assertFalse(p.getBooleanFlagValue(0));
        assertTrue(p.getBooleanFlagValue(1));
        assertTrue(p.getBooleanFlagValue(2));
@@ -78,10 +107,8 @@ public class AconfigPackageImplTest {

    @Test
    public void testHasPackageFingerprint() throws Exception {
        StorageFileProvider pr =
                new StorageFileProvider(TestDataUtils.TESTDATA_PATH, TestDataUtils.TESTDATA_PATH);
        AconfigPackageImpl p =
                AconfigPackageImpl.load("mockup", "com.android.aconfig.storage.test_1", pr);
        AconfigPackageImpl p = new AconfigPackageImpl();
        p.load("mockup", "com.android.aconfig.storage.test_1", pr);
        assertFalse(p.hasPackageFingerprint());
    }
}
+1 −1
Original line number Diff line number Diff line
@@ -7,7 +7,7 @@ rule android.aconfig.storage.SipHasher13 android.aconfig.storage.test.SipHasher1
rule android.aconfig.storage.FileType android.aconfig.storage.test.FileType
rule android.aconfig.storage.FlagValueList android.aconfig.storage.test.FlagValueList
rule android.aconfig.storage.TableUtils android.aconfig.storage.test.TableUtils
rule android.aconfig.storage.Package android.aconfig.storage.test.Package
rule android.aconfig.storage.AconfigPackageImpl android.aconfig.storage.test.AconfigPackageImpl
rule android.aconfig.storage.StorageFileProvider android.aconfig.storage.test.StorageFileProvider