Make TestStorage use internal storage for output files when running as non system user.

Previously TestStorage always used the legacy external storage
directory to store both input and output files.
However there has been stability issues with creating directories
in this location in automotive platforms where tests always run as a secondary user.

This commit changes the logic to use the current app's local cache
directory when running as a secondary user.
The hope is this can be done without any corresponding host-side adjustment, as host side code paths use 'adb shell content read' commands to fetch output files when running as a secondary user - making it not-dependent on absolute storage location.

This commit also attempts to simplify the storage ContentProvider logic. It previously attempted to create directories in multiple locations, had hard coded assumptions around mounting locations etc.

PiperOrigin-RevId: 627060015

Author: brettchabot

services/CHANGELOG.md

@@ -6,6 +6,8 @@
 
 **Bug Fixes**
 
+* TestStorage: use local cache dir to store output files when running as non system user
+
 **New Features**
 
 **Breaking Changes**

services/storage/java/androidx/test/services/storage/TestStorage.java

@@ -125,7 +125,10 @@ public Uri getOutputFileUri(@NonNull String pathname) {
   @Override
   public boolean isTestStorageFilePath(@NonNull String pathname) {
     File onDevicePathRoot =
-        new File(HostedFile.getRootDirectory(), TestStorageConstants.ON_DEVICE_PATH_ROOT);
+        new File(
+            HostedFile.getOutputRootDirectory(
+                InstrumentationRegistry.getInstrumentation().getTargetContext()),
+            TestStorageConstants.ON_DEVICE_PATH_ROOT);
     // Append a trailing slash because ON_DEVICE_PATH_ROOT has a trailing slash. If pathname already
     // has a trailing slash or other suffix, this won't affect the startsWith() matching logic.
     pathname = pathname + "/";
@@ -318,6 +321,7 @@ public Map<String, Serializable> getOutputProperties() {
    * @return an InputStream to the given test file.
    * @hide
    */
+  // TODO(b/335660740): remove this unused method
   @RestrictTo(Scope.LIBRARY)
   @Override
   public InputStream openInternalInputFile(String pathname) throws FileNotFoundException {

services/storage/java/androidx/test/services/storage/file/HostedFile.java

@@ -15,9 +15,13 @@
  */
 package androidx.test.services.storage.file;
 
+import android.content.Context;
 import android.net.Uri;
+import android.os.Build.VERSION;
 import android.os.Environment;
+import android.os.UserManager;
 import android.provider.OpenableColumns;
+import android.util.Log;
 import androidx.test.annotation.ExperimentalTestApi;
 import androidx.test.services.storage.TestStorageConstants;
 import java.io.File;
@@ -26,6 +30,8 @@
 @ExperimentalTestApi
 public final class HostedFile {
 
+  private static final String TAG = "HostedFile";
+
   /** An enum of the columns returned by the hosted file service. */
   public enum HostedFileColumn {
     NAME("name", String.class, 3 /* Cursor.FIELD_TYPE_STRING since api 11 */, 0),
@@ -132,10 +138,40 @@ public static Uri buildUri(FileHost host, String fileName) {
         .build();
   }
 
-  public static File getRootDirectory() {
+  public static File getInputRootDirectory(Context context) {
+    // always use external storage dir for input
+    Log.i(
+        TAG,
+        "Choosing external storage as root dir for input: "
+            + Environment.getExternalStorageDirectory().getAbsolutePath());
     return Environment.getExternalStorageDirectory();
   }
 
+  public static File getOutputRootDirectory(Context context) {
+    UserManager userManager = (UserManager) context.getSystemService(Context.USER_SERVICE);
+    if (VERSION.SDK_INT < 23) {
+      Log.i(
+          TAG,
+          "Running on API < 23; Choosing external storage as output root dir: "
+              + Environment.getExternalStorageDirectory().getAbsolutePath());
+      return Environment.getExternalStorageDirectory();
+    } else if (userManager.isSystemUser()) {
+      Log.i(
+          TAG,
+          "System user detected. Choosing external storage as output root dir: "
+              + Environment.getExternalStorageDirectory().getAbsolutePath());
+      return Environment.getExternalStorageDirectory();
+    } else {
+      // using legacy external storage for output in automotive devices where tests run as
+      // a secondary user has been flaky. So use local storage instead.
+      Log.i(
+          TAG,
+          "Secondary user detected. Choosing local storage as output root dir: "
+              + context.getCacheDir().getAbsolutePath());
+      return context.getCacheDir();
+    }
+  }
+
   private static <T> T checkNotNull(T reference) {
     if (reference == null) {
       throw new NullPointerException();

services/storage/java/androidx/test/services/storage/provider/AbstractFileContentProvider.java

@@ -19,6 +19,7 @@
 
 import android.content.ContentProvider;
 import android.content.ContentValues;
+import android.content.Context;
 import android.database.Cursor;
 import android.database.MatrixCursor;
 import android.net.Uri;
@@ -43,52 +44,25 @@
 abstract class AbstractFileContentProvider extends ContentProvider {
   private static final String TAG = AbstractFileContentProvider.class.getSimpleName();
 
-  private final File hostedDirectory;
-  private final Access access;
+  private File hostedDirectory;
+  private Access access;
 
   enum Access {
     READ_ONLY,
     READ_WRITE
   }
 
-  /**
-   * Called during onCreate(). Subclasses should return true if they are ready to serve data and
-   * false if there is something wrong accessing their data. Such as the sdcard not being mounted.
-   */
-  protected abstract boolean onCreateHook();
+  /** Returns the root directory where this content provider should manage files from */
+  protected abstract File getHostedDirectory(Context context);
 
-  AbstractFileContentProvider(File hostedDirectory, Access access) {
-    super();
-    try {
-      this.hostedDirectory = checkNotNull(hostedDirectory).getCanonicalFile();
-    } catch (IOException ioe) {
-      throw new RuntimeException(ioe);
-    }
-    this.access = access;
-  }
+  /** Returns the access mode of this content provider */
+  protected abstract Access getAccess();
 
   @Override
   public boolean onCreate() {
-    if (onCreateHook()) {
-      if (!hostedDirectory.exists()) {
-        if (!hostedDirectory.mkdirs()) {
-          Log.e(TAG, "Cannot create hosted directory: " + hostedDirectory);
-          return false;
-        }
-      }
-      if (!hostedDirectory.isDirectory()) {
-        Log.e(TAG, "Hosted directory not a directory: " + hostedDirectory);
-        return false;
-      }
-      if ((Access.READ_WRITE == access) && !hostedDirectory.canWrite()) {
-        Log.e(TAG, "Hosted directory is not writable and write was requested: " + hostedDirectory);
-        return false;
-      }
-      return true;
-    } else {
-      Log.e(TAG, "Subclass claims hosted directory not ready: " + hostedDirectory);
-      return false;
-    }
+    hostedDirectory = getHostedDirectory(getContext());
+    access = getAccess();
+    return true;
   }
 
   @Override
@@ -107,10 +81,15 @@ public ParcelFileDescriptor openFile(Uri uri, String mode) throws FileNotFoundEx
       try {
         requestedFile.getParentFile().mkdirs();
         if (!requestedFile.getParentFile().exists()) {
+          Log.e(
+              TAG,
+              "Failed to create parent directory "
+                  + requestedFile.getParentFile().getAbsolutePath());
           throw new FileNotFoundException(String.format("No parent directory for '%s'", uri));
         }
 
         if (!requestedFile.createNewFile()) {
+          Log.e(TAG, "Failed to create file" + requestedFile.getAbsolutePath());
           throw new FileNotFoundException("Could not create file: " + uri);
         }
       } catch (IOException ioe) {

services/storage/java/androidx/test/services/storage/provider/ExportTestPropertiesContentProvider.java

@@ -20,9 +20,13 @@
 /** Hosts the properties file that are exported to the testing infrastructure for tests. */
 public final class ExportTestPropertiesContentProvider extends TestFileContentProvider {
 
-  public ExportTestPropertiesContentProvider() {
-    super(
-        TestStorageConstants.ON_DEVICE_PATH_TEST_PROPERTIES,
-        AbstractFileContentProvider.Access.READ_WRITE);
+  @Override
+  protected Access getAccess() {
+    return AbstractFileContentProvider.Access.READ_WRITE;
+  }
+
+  @Override
+  protected String getHostedRelativePath() {
+    return TestStorageConstants.ON_DEVICE_PATH_TEST_PROPERTIES;
   }
 }

services/storage/java/androidx/test/services/storage/provider/InternalUseOnlyFilesContentProvider.java

@@ -15,39 +15,23 @@
  */
 package androidx.test.services.storage.provider;
 
-import android.os.Environment;
-import android.util.Log;
+import android.content.Context;
 import androidx.test.services.storage.TestStorageConstants;
 import androidx.test.services.storage.file.HostedFile;
 import java.io.File;
 
 /** Hosts an SD Card directory for the test framework to read/write internal files to. */
 public final class InternalUseOnlyFilesContentProvider extends AbstractFileContentProvider {
-  private static final String TAG = "InternalUseOnlyFilesContentProvider";
 
-  private final File outputDirectory;
-
-  public InternalUseOnlyFilesContentProvider() {
-    super(
-        new File(HostedFile.getRootDirectory(), TestStorageConstants.ON_DEVICE_PATH_INTERNAL_USE),
-        AbstractFileContentProvider.Access.READ_WRITE);
-    outputDirectory =
-        new File(HostedFile.getRootDirectory(), TestStorageConstants.ON_DEVICE_PATH_INTERNAL_USE);
+  @Override
+  protected File getHostedDirectory(Context context) {
+    return new File(
+        HostedFile.getOutputRootDirectory(context),
+        TestStorageConstants.ON_DEVICE_PATH_INTERNAL_USE);
   }
 
   @Override
-  protected boolean onCreateHook() {
-    if (!Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
-      Log.e(TAG, "sdcard in bad state: " + Environment.getExternalStorageState());
-      return false;
-    } else {
-      if (!outputDirectory.exists()) {
-        if (!outputDirectory.mkdirs()) {
-          Log.e(TAG, String.format("'%s': could not create output dir! ", outputDirectory));
-          return false;
-        }
-      }
-      return true;
-    }
+  protected Access getAccess() {
+    return AbstractFileContentProvider.Access.READ_WRITE;
   }
 }

services/storage/java/androidx/test/services/storage/provider/TestArgsContentProvider.java

@@ -19,9 +19,11 @@
 
 import android.content.ContentProvider;
 import android.content.ContentValues;
+import android.content.Context;
 import android.database.Cursor;
 import android.database.MatrixCursor;
 import android.net.Uri;
+import android.util.Log;
 import androidx.test.services.storage.TestStorageConstants;
 import androidx.test.services.storage.TestStorageServiceProto.TestArgument;
 import androidx.test.services.storage.TestStorageServiceProto.TestArguments;
@@ -108,24 +110,25 @@ public Cursor query(
   private Map<String, String> buildArgMapFromFile() {
     Map<String, String> argMap = new HashMap<>();
 
-    for (TestArgument testArg : readProtoFromFile().getArgList()) {
+    for (TestArgument testArg : readProtoFromFile(this.getContext()).getArgList()) {
       String key = testArg.getName();
       String val = testArg.getValue();
       argMap.put(key, val);
     }
     return argMap;
   }
 
-  private static TestArguments readProtoFromFile() {
+  private static TestArguments readProtoFromFile(Context context) {
     File testArgsFile =
         new File(
-            HostedFile.getRootDirectory(),
+            HostedFile.getInputRootDirectory(context),
             TestStorageConstants.ON_DEVICE_PATH_INTERNAL_USE
                 + TestStorageConstants.TEST_ARGS_FILE_NAME);
     if (!testArgsFile.exists()) {
       return TestArguments.getDefaultInstance();
     }
     try {
+      Log.i(TAG, "Parsing test args from " + testArgsFile.getAbsolutePath());
       return TestArguments.parseFrom(new FileInputStream(testArgsFile));
     } catch (IOException e) {
       throw new RuntimeException("Not able to read from file: " + testArgsFile.getName(), e);

services/storage/java/androidx/test/services/storage/provider/TestDataContentProvider.java

@@ -15,29 +15,22 @@
  */
 package androidx.test.services.storage.provider;
 
-import android.os.Environment;
-import android.util.Log;
+import android.content.Context;
 import androidx.test.services.storage.TestStorageConstants;
 import androidx.test.services.storage.file.HostedFile;
 import java.io.File;
 
 /** Provides access to files in the test data section. */
 public final class TestDataContentProvider extends AbstractFileContentProvider {
-  private static final String TAG = TestDataContentProvider.class.getSimpleName();
 
-  public TestDataContentProvider() {
-    super(
-        new File(HostedFile.getRootDirectory(), TestStorageConstants.ON_DEVICE_TEST_RUNFILES),
-        AbstractFileContentProvider.Access.READ_ONLY);
+  @Override
+  protected File getHostedDirectory(Context context) {
+    return new File(
+        HostedFile.getInputRootDirectory(context), TestStorageConstants.ON_DEVICE_TEST_RUNFILES);
   }
 
   @Override
-  protected boolean onCreateHook() {
-    if (!Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
-      Log.e(TAG, "sdcard in bad state: " + Environment.getExternalStorageState());
-      return false;
-    } else {
-      return true;
-    }
+  protected Access getAccess() {
+    return Access.READ_ONLY;
   }
 }

services/storage/java/androidx/test/services/storage/provider/TestFileContentProvider.java

@@ -15,37 +15,19 @@
  */
 package androidx.test.services.storage.provider;
 
-import android.os.Environment;
-import android.util.Log;
+import android.content.Context;
 import androidx.test.services.storage.file.HostedFile;
 import java.io.File;
 
 /**
  * Content Provider that allows access to reading/writing files that were written to disk for tests.
  */
 abstract class TestFileContentProvider extends AbstractFileContentProvider {
-  private static final String TAG = TestFileContentProvider.class.getSimpleName();
 
-  private final File outputDirectory;
-
-  public TestFileContentProvider(String filePath, AbstractFileContentProvider.Access access) {
-    super(new File(HostedFile.getRootDirectory(), filePath), access);
-    outputDirectory = new File(HostedFile.getRootDirectory(), filePath);
-  }
+  protected abstract String getHostedRelativePath();
 
   @Override
-  protected boolean onCreateHook() {
-    if (!Environment.getExternalStorageState().equals(Environment.MEDIA_MOUNTED)) {
-      Log.e(TAG, "sdcard in bad state: " + Environment.getExternalStorageState());
-      return false;
-    } else {
-      if (!outputDirectory.exists()) {
-        if (!outputDirectory.mkdirs()) {
-          Log.e(TAG, String.format("'%s': could not create output dir! ", outputDirectory));
-          return false;
-        }
-      }
-      return true;
-    }
+  protected final File getHostedDirectory(Context context) {
+    return new File(HostedFile.getOutputRootDirectory(context), getHostedRelativePath());
   }
 }

services/storage/java/androidx/test/services/storage/provider/TestOutputFilesContentProvider.java

@@ -20,9 +20,13 @@
 /** Hosts the output directory for tests. */
 public final class TestOutputFilesContentProvider extends TestFileContentProvider {
 
-  public TestOutputFilesContentProvider() {
-    super(
-        TestStorageConstants.ON_DEVICE_PATH_TEST_OUTPUT,
-        AbstractFileContentProvider.Access.READ_WRITE);
+  @Override
+  protected Access getAccess() {
+    return AbstractFileContentProvider.Access.READ_WRITE;
+  }
+
+  @Override
+  protected String getHostedRelativePath() {
+    return TestStorageConstants.ON_DEVICE_PATH_TEST_OUTPUT;
   }
 }