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

4 files changed, 87 insertions, 63 deletions

diff --git a/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/AconfigPackageImpl.java b/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/AconfigPackageImpl.java
index 4d020cfcb9..5d8e7cbe2f 100644
--- a/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/AconfigPackageImpl.java
+++ b/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/AconfigPackageImpl.java
@@ -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;
-        }
-        AconfigPackageImpl aPackage = new AconfigPackageImpl();
-        if (!aPackage.init(container, packageName, fileProvider)) {
-            return null;
+            return ERROR_CONTAINER_NOT_FOUND;
         }
-        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);
-                    pNode = pTable.get(packageName);
-                    if (pNode != null) {
-                        container = pTable.getHeader().getContainer();
-                        break;
+                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;
     }
 }
diff --git a/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/StorageFileProvider.java b/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/StorageFileProvider.java
index f586150019..1f84a51b01 100644
--- a/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/StorageFileProvider.java
+++ b/tools/aconfig/aconfig_storage_read_api/srcs/android/aconfig/storage/StorageFileProvider.java
@@ -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));
     }
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/java/AconfigPackageImplTest.java b/tools/aconfig/aconfig_storage_read_api/tests/java/AconfigPackageImplTest.java
index 0f3be7ae39..8a72f0adf1 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/java/AconfigPackageImplTest.java
+++ b/tools/aconfig/aconfig_storage_read_api/tests/java/AconfigPackageImplTest.java
@@ -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());
     }
 }
diff --git a/tools/aconfig/aconfig_storage_read_api/tests/java/jarjar.txt b/tools/aconfig/aconfig_storage_read_api/tests/java/jarjar.txt
index 64ba09cf90..24952ecfdf 100644
--- a/tools/aconfig/aconfig_storage_read_api/tests/java/jarjar.txt
+++ b/tools/aconfig/aconfig_storage_read_api/tests/java/jarjar.txt
@@ -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